| Message ID | 1456707071-15364-1-git-send-email-joel@jms.id.au |
|---|---|
| State | Accepted |
| Headers | show |
On 02/29/2016 06:21 AM, Joel Stanley wrote: > GCC 6 warns when we look at any stack frame other than our own, ie any > argument to __builtin_frame_address other than zero. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > core/stack.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/core/stack.c b/core/stack.c > index 17f89d491ba2..8b8842fc9d45 100644 > --- a/core/stack.c > +++ b/core/stack.c > @@ -31,10 +31,11 @@ extern uint32_t _stext, _etext; > void __nomcount __backtrace(struct bt_entry *entries, unsigned int *count) > { > unsigned int room = *count; > - unsigned long *fp = __builtin_frame_address(1); > + unsigned long *fp = __builtin_frame_address(0); > > *count = 0; > while(room) { > + fp = (unsigned long *)fp[0]; What if fp is NULL? -Vasant
On Mon, Feb 29, 2016 at 3:56 PM, Vasant Hegde <hegdevasant@linux.vnet.ibm.com> wrote: >> - unsigned long *fp = __builtin_frame_address(1); >> + unsigned long *fp = __builtin_frame_address(0); >> >> *count = 0; >> while(room) { >> + fp = (unsigned long *)fp[0]; > > > What if fp is NULL? We dereference a null pointer. Will we ever find ourselves in the situation where there is no frame pointer?
On 03/01/2016 08:51 AM, Joel Stanley wrote: > On Mon, Feb 29, 2016 at 3:56 PM, Vasant Hegde > <hegdevasant@linux.vnet.ibm.com> wrote: >>> - unsigned long *fp = __builtin_frame_address(1); >>> + unsigned long *fp = __builtin_frame_address(0); >>> >>> *count = 0; >>> while(room) { >>> + fp = (unsigned long *)fp[0]; >> >> >> What if fp is NULL? > > We dereference a null pointer. > > Will we ever find ourselves in the situation where there is no frame pointer? Never. But I doubt code analysis tools understand that and we may start getting warnings ;-) -Vasant
Joel Stanley <joel@jms.id.au> writes: > GCC 6 warns when we look at any stack frame other than our own, ie any > argument to __builtin_frame_address other than zero. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > core/stack.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Thanks! I think this all ends up being okay, we're already pretty bound to ABI here anyway, so I don't really think fp[0] here will be a problem. merged to master as of 793f6f5
diff --git a/core/stack.c b/core/stack.c index 17f89d491ba2..8b8842fc9d45 100644 --- a/core/stack.c +++ b/core/stack.c @@ -31,10 +31,11 @@ extern uint32_t _stext, _etext; void __nomcount __backtrace(struct bt_entry *entries, unsigned int *count) { unsigned int room = *count; - unsigned long *fp = __builtin_frame_address(1); + unsigned long *fp = __builtin_frame_address(0); *count = 0; while(room) { + fp = (unsigned long *)fp[0]; if (!fp || (unsigned long)fp > top_of_ram) break; entries->sp = (unsigned long)fp; @@ -42,7 +43,6 @@ void __nomcount __backtrace(struct bt_entry *entries, unsigned int *count) entries++; *count = (*count) + 1; room--; - fp = (unsigned long *)fp[0]; } }
GCC 6 warns when we look at any stack frame other than our own, ie any argument to __builtin_frame_address other than zero. Signed-off-by: Joel Stanley <joel@jms.id.au> --- core/stack.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)