[patch] stack.c, check return value of lookup_symbol

[patch] stack.c, check return value of lookup_symbol

[msnyder]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

Since it's known that lookup_symbol can return NULL, my first impulse
was to call gdb_assert.  That still might be the right thing to do,
since it indicates some sort of internal fault -- but it seems to
me that it isn't necessarily fatal, and simply doing nothing is an option...


[-- Attachment #2: nsym2.txt --]
[-- Type: text/plain, Size: 1207 bytes --]

2007-07-05  Michael Snyder  <msnyder@access-company.com>

	* stack.c (print_frame_args): Check return value of lookup_symbol
	(Coverity).

Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.149
diff -p -r1.149 stack.c
*** stack.c	18 Jun 2007 18:28:29 -0000	1.149
--- stack.c	5 Jul 2007 23:04:35 -0000
*************** print_frame_args (struct symbol *func, s
*** 247,253 ****
  	      struct symbol *nsym;
  	      nsym = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
  				    b, VAR_DOMAIN, NULL, NULL);
! 	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
  		{
  		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means
  		     that it was passed on the stack and loaded into a
--- 247,257 ----
  	      struct symbol *nsym;
  	      nsym = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
  				    b, VAR_DOMAIN, NULL, NULL);
! 	      if (nsym == NULL)
! 		/* Not sure what to do here, but doing nothing 
! 		   should be safe...  */
! 		;
! 	      else if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
  		{
  		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means
  		     that it was passed on the stack and loaded into a

Re: [patch] stack.c, check return value of lookup_symbol

[Jim Blandy]

msnyder@sonic.net writes:
> Since it's known that lookup_symbol can return NULL, my first impulse
> was to call gdb_assert.  That still might be the right thing to do,
> since it indicates some sort of internal fault -- but it seems to
> me that it isn't necessarily fatal, and simply doing nothing is an
> option...

gdb_assert calls internal_error, so it's not going to sweep GDB out
from under the user.  And the situation we're talking about here would
be one where we found a symbol in a block, and then looked up that
name in that block and didn't find the symbol.  I definitely want to
see an internal error if that happens.

Re: [patch] stack.c, check return value of lookup_symbol

[msnyder]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

>
> msnyder@sonic.net writes:
>> Since it's known that lookup_symbol can return NULL, my first impulse
>> was to call gdb_assert.  That still might be the right thing to do,
>> since it indicates some sort of internal fault -- but it seems to
>> me that it isn't necessarily fatal, and simply doing nothing is an
>> option...
>
> gdb_assert calls internal_error, so it's not going to sweep GDB out
> from under the user.  And the situation we're talking about here would
> be one where we found a symbol in a block, and then looked up that
> name in that block and didn't find the symbol.  I definitely want to
> see an internal error if that happens.

OK then -- how's this?


[-- Attachment #2: nsym.txt --]
[-- Type: text/plain, Size: 754 bytes --]

2007-07-05  Michael Snyder  <msnyder@access-company.com>

	* stack.c (print_frame_args): Check return value of lookup_symbol
	(Coverity).

Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.149
diff -p -r1.149 stack.c
*** stack.c	18 Jun 2007 18:28:29 -0000	1.149
--- stack.c	5 Jul 2007 22:57:59 -0000
*************** print_frame_args (struct symbol *func, s
*** 247,252 ****
--- 247,253 ----
  	      struct symbol *nsym;
  	      nsym = lookup_symbol (DEPRECATED_SYMBOL_NAME (sym),
  				    b, VAR_DOMAIN, NULL, NULL);
+ 	      gdb_assert (nsym);
  	      if (SYMBOL_CLASS (nsym) == LOC_REGISTER)
  		{
  		  /* There is a LOC_ARG/LOC_REGISTER pair.  This means

Re: [patch] stack.c, check return value of lookup_symbol

[Jim Blandy]

msnyder@sonic.net writes:
>> msnyder@sonic.net writes:
>>> Since it's known that lookup_symbol can return NULL, my first impulse
>>> was to call gdb_assert.  That still might be the right thing to do,
>>> since it indicates some sort of internal fault -- but it seems to
>>> me that it isn't necessarily fatal, and simply doing nothing is an
>>> option...
>>
>> gdb_assert calls internal_error, so it's not going to sweep GDB out
>> from under the user.  And the situation we're talking about here would
>> be one where we found a symbol in a block, and then looked up that
>> name in that block and didn't find the symbol.  I definitely want to
>> see an internal error if that happens.
>
> OK then -- how's this?

That's what I'd do. :)