RFA: don't use minsym name when searching block

RFA: don't use minsym name when searching block

[Jim Blandy]

I'd appreciate it if the C++ folks could check this out.  I don't have
a test case yet, but I'll try to put one together tomorrow.

2002-08-21  Jim Blandy  <jimb@redhat.com>

	* symtab.c (lookup_symbol_aux): In the case where we find a
	minimal symbol of an appropriate name and use its address to
	select a symtab to read and search, use `name' (as passed to us)
	as the demangled name when searching the symtab's global and
	static blocks, not the minsym's name.  The minsym's name might be
	mangled.

Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.68
diff -c -r1.68 symtab.c
*** gdb/symtab.c	1 Aug 2002 17:18:33 -0000	1.68
--- gdb/symtab.c	22 Aug 2002 04:38:54 -0000
***************
*** 860,874 ****
  	      /* This is a function which has a symtab for its address.  */
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					 mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					     mangled_name, namespace);
  		}
  
  	      /* sym == 0 if symbol was found in the minimal symbol table
--- 860,873 ----
  	      /* This is a function which has a symtab for its address.  */
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, name,
!                                              mangled_name, namespace);
  		}
  
  	      /* sym == 0 if symbol was found in the minimal symbol table

Re: RFA: don't use minsym name when searching block

[Daniel Jacobowitz]

On Thu, Aug 22, 2002 at 12:26:51AM -0500, Jim Blandy wrote:
> 
> I'd appreciate it if the C++ folks could check this out.  I don't have
> a test case yet, but I'll try to put one together tomorrow.

Could you describe the problem?  Also... "the minsym's name might be
mangled" definitely seems like a problem; they should either be
consistently mangled or demangled...

> 2002-08-21  Jim Blandy  <jimb@redhat.com>
> 
> 	* symtab.c (lookup_symbol_aux): In the case where we find a
> 	minimal symbol of an appropriate name and use its address to
> 	select a symtab to read and search, use `name' (as passed to us)
> 	as the demangled name when searching the symtab's global and
> 	static blocks, not the minsym's name.  The minsym's name might be
> 	mangled.

Style nit - a comment like this belongs in the code, not in the
changelog.

> 
> Index: gdb/symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.68
> diff -c -r1.68 symtab.c
> *** gdb/symtab.c	1 Aug 2002 17:18:33 -0000	1.68
> --- gdb/symtab.c	22 Aug 2002 04:38:54 -0000
> ***************
> *** 860,874 ****
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					 mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					     mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> --- 860,873 ----
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, name,
> !                                              mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: don't use minsym name when searching block

[Daniel Jacobowitz]

On Thu, Aug 22, 2002 at 12:26:51AM -0500, Jim Blandy wrote:
> 
> I'd appreciate it if the C++ folks could check this out.  I don't have
> a test case yet, but I'll try to put one together tomorrow.

Could you describe the problem?  Also... "the minsym's name might be
mangled" definitely seems like a problem; they should either be
consistently mangled or demangled...

> 2002-08-21  Jim Blandy  <jimb@redhat.com>
> 
> 	* symtab.c (lookup_symbol_aux): In the case where we find a
> 	minimal symbol of an appropriate name and use its address to
> 	select a symtab to read and search, use `name' (as passed to us)
> 	as the demangled name when searching the symtab's global and
> 	static blocks, not the minsym's name.  The minsym's name might be
> 	mangled.

Style nit - a comment like this belongs in the code, not in the
changelog.

> 
> Index: gdb/symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.68
> diff -c -r1.68 symtab.c
> *** gdb/symtab.c	1 Aug 2002 17:18:33 -0000	1.68
> --- gdb/symtab.c	22 Aug 2002 04:38:54 -0000
> ***************
> *** 860,874 ****
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					 mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					     mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> --- 860,873 ----
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, name,
> !                                              mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: don't use minsym name when searching block

[Jim Blandy]

Daniel Jacobowitz <drow@mvista.com> writes:

> On Thu, Aug 22, 2002 at 12:26:51AM -0500, Jim Blandy wrote:
> > 
> > I'd appreciate it if the C++ folks could check this out.  I don't have
> > a test case yet, but I'll try to put one together tomorrow.
> 
> Could you describe the problem?  Also... "the minsym's name might be
> mangled" definitely seems like a problem; they should either be
> consistently mangled or demangled...

Minimal symbol names are consistently mangled, which makes sense:
they're linker symbols, after all.  You have to use
SYMBOL_DEMANGLED_NAME to get a minsym's demangled name.  But the names
of real symbols (struct symbol) are always demangled.  So when we pass
SYMBOL_NAME (msymbol) to lookup_block_symbol, we're passing a name
guaranteed to be mangled to a function that expects a demangled name.

I'm working on a test case for this.  The symptom is that, when you
try to set a breakpoint on a method, say foo::bar, you get an error
message, but if you then repeat the exact same command, it works the
second time.

What's going on is that, the first time we call lookup_symbol_aux,
which is passed both the mangled name and the demangled name, we hit
the case being patched: we find a minsym, and read the symtab whose
address range contains the minsym's value.  However, since we then
search the global and static blocks using the mangled name, we fail.

The second time we try to lookup foo::bar, the symtab has been read,
and the ALL_SYMTABS loop finds the symbol.


> > 2002-08-21  Jim Blandy  <jimb@redhat.com>
> > 
> > 	* symtab.c (lookup_symbol_aux): In the case where we find a
> > 	minimal symbol of an appropriate name and use its address to
> > 	select a symtab to read and search, use `name' (as passed to us)
> > 	as the demangled name when searching the symtab's global and
> > 	static blocks, not the minsym's name.  The minsym's name might be
> > 	mangled.
> 
> Style nit - a comment like this belongs in the code, not in the
> changelog.

Yes, you're right.

Once I saw the problem, the code seemed so obviously wrong to me that
I thought a comment would be redundant --- minsym names are mangled,
and lookup_block_symbol expects a demangled name.  But you're right:
before I had worked through the problem, it wasn't obviously wrong.

Re: RFA: don't use minsym name when searching block

[Daniel Jacobowitz]

On Thu, Aug 29, 2002 at 04:24:16PM -0500, Jim Blandy wrote:
> Daniel Jacobowitz <drow@mvista.com> writes:
> 
> > On Thu, Aug 22, 2002 at 12:26:51AM -0500, Jim Blandy wrote:
> > > 
> > > I'd appreciate it if the C++ folks could check this out.  I don't have
> > > a test case yet, but I'll try to put one together tomorrow.
> > 
> > Could you describe the problem?  Also... "the minsym's name might be
> > mangled" definitely seems like a problem; they should either be
> > consistently mangled or demangled...
> 
> Minimal symbol names are consistently mangled, which makes sense:
> they're linker symbols, after all.  You have to use
> SYMBOL_DEMANGLED_NAME to get a minsym's demangled name.  But the names
> of real symbols (struct symbol) are always demangled.  So when we pass
> SYMBOL_NAME (msymbol) to lookup_block_symbol, we're passing a name
> guaranteed to be mangled to a function that expects a demangled name.
> 
> I'm working on a test case for this.  The symptom is that, when you
> try to set a breakpoint on a method, say foo::bar, you get an error
> message, but if you then repeat the exact same command, it works the
> second time.

Oh!  In that case, I think I have a test case for it tucked away in a
bug report; I'll look for it.  That makes much more sense now.

> What's going on is that, the first time we call lookup_symbol_aux,
> which is passed both the mangled name and the demangled name, we hit
> the case being patched: we find a minsym, and read the symtab whose
> address range contains the minsym's value.  However, since we then
> search the global and static blocks using the mangled name, we fail.
> 
> The second time we try to lookup foo::bar, the symtab has been read,
> and the ALL_SYMTABS loop finds the symbol.

Thanks for the explanation.  Shouldn't the code that does the same
thing in the (#&@ HPUXHPPA block further down have the same fix
applied?  Other than that, it looks good to me.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: RFA: don't use minsym name when searching block

[Jim Blandy]

I've committed the following, which differs from my original patch:
1) it fixes the analogous uses in the HPPAHPUX block, and
2) it includes comments explaining why using the minsym name is
   inappropriate. 

2002-08-29  Jim Blandy  <jimb@redhat.com>

	* symtab.c (lookup_symbol_aux): In the cases where we find a
	minimal symbol of an appropriate name and use its address to
	select a symtab to read and search, use `name' (as passed to us)
	as the demangled name when searching the symtab's global and
	static blocks, not the minsym's name.

Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.68
diff -c -r1.68 symtab.c
*** gdb/symtab.c	1 Aug 2002 17:18:33 -0000	1.68
--- gdb/symtab.c	30 Aug 2002 02:18:57 -0000
***************
*** 860,874 ****
  	      /* This is a function which has a symtab for its address.  */
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					 mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					     mangled_name, namespace);
  		}
  
  	      /* sym == 0 if symbol was found in the minimal symbol table
--- 860,879 ----
  	      /* This is a function which has a symtab for its address.  */
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
! 
!               /* This call used to pass `SYMBOL_NAME (msymbol)' as the
!                  `name' argument to lookup_block_symbol.  But the name
!                  of a minimal symbol is always mangled, so that seems
!                  to be clearly the wrong thing to pass as the
!                  unmangled name.  */
! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, name,
!                                              mangled_name, namespace);
  		}
  
  	      /* sym == 0 if symbol was found in the minimal symbol table
***************
*** 1027,1041 ****
  	    {
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					 mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
! 					     mangled_name, namespace);
  		}
  	      /* If we found one, return it */
  	      if (sym)
--- 1032,1050 ----
  	    {
  	      bv = BLOCKVECTOR (s);
  	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
!               /* This call used to pass `SYMBOL_NAME (msymbol)' as the
!                  `name' argument to lookup_block_symbol.  But the name
!                  of a minimal symbol is always mangled, so that seems
!                  to be clearly the wrong thing to pass as the
!                  unmangled name.  */
! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
  	      /* We kept static functions in minimal symbol table as well as
  	         in static scope. We want to find them in the symbol table. */
  	      if (!sym)
  		{
  		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
! 		  sym = lookup_block_symbol (block, name,
!                                              mangled_name, namespace);
  		}
  	      /* If we found one, return it */
  	      if (sym)

Re: RFA: don't use minsym name when searching block

[Peter.Schauer]

I proposed a very similar patch almost two years ago, but it got turned
down and I was tired of arguing. Please note that there are two more
calls to lookup_block_symbol in the #ifdef HPUXHPPA block, which I think
have to be adjusted as well.

See http://sources.redhat.com/ml/gdb-patches/2000-10/msg00250.html

> I'd appreciate it if the C++ folks could check this out.  I don't have
> a test case yet, but I'll try to put one together tomorrow.
> 
> 2002-08-21  Jim Blandy  <jimb@redhat.com>
> 
> 	* symtab.c (lookup_symbol_aux): In the case where we find a
> 	minimal symbol of an appropriate name and use its address to
> 	select a symtab to read and search, use `name' (as passed to us)
> 	as the demangled name when searching the symtab's global and
> 	static blocks, not the minsym's name.  The minsym's name might be
> 	mangled.
> 
> Index: gdb/symtab.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symtab.c,v
> retrieving revision 1.68
> diff -c -r1.68 symtab.c
> *** gdb/symtab.c	1 Aug 2002 17:18:33 -0000	1.68
> --- gdb/symtab.c	22 Aug 2002 04:38:54 -0000
> ***************
> *** 860,874 ****
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					 mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, SYMBOL_NAME (msymbol),
> ! 					     mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> --- 860,873 ----
>   	      /* This is a function which has a symtab for its address.  */
>   	      bv = BLOCKVECTOR (s);
>   	      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
> ! 	      sym = lookup_block_symbol (block, name, mangled_name, namespace);
>   	      /* We kept static functions in minimal symbol table as well as
>   	         in static scope. We want to find them in the symbol table. */
>   	      if (!sym)
>   		{
>   		  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
> ! 		  sym = lookup_block_symbol (block, name,
> !                                              mangled_name, namespace);
>   		}
>   
>   	      /* sym == 0 if symbol was found in the minimal symbol table
> 
> 


-- 
Peter Schauer			pes@regent.e-technik.tu-muenchen.de

Re: RFA: don't use minsym name when searching block

[Jim Blandy]

"Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de> writes:
> I proposed a very similar patch almost two years ago, but it got turned
> down and I was tired of arguing. Please note that there are two more
> calls to lookup_block_symbol in the #ifdef HPUXHPPA block, which I think
> have to be adjusted as well.

Okay, thanks for pointing that out.

> See http://sources.redhat.com/ml/gdb-patches/2000-10/msg00250.html

Do you think it's really best to use SYMBOL_SOURCE_NAME, instead of
just `name'?  I had the impression that the full symbol names were
always demangled, so the name to use shouldn't depend on the setting
of the `demangle' flag, as SYMBOL_SOURCE_NAME does.

Re: RFA: don't use minsym name when searching block

[Daniel Jacobowitz]

On Thu, Aug 29, 2002 at 04:28:07PM -0500, Jim Blandy wrote:
> 
> "Peter.Schauer" <Peter.Schauer@regent.e-technik.tu-muenchen.de> writes:
> > I proposed a very similar patch almost two years ago, but it got turned
> > down and I was tired of arguing. Please note that there are two more
> > calls to lookup_block_symbol in the #ifdef HPUXHPPA block, which I think
> > have to be adjusted as well.
> 
> Okay, thanks for pointing that out.
> 
> > See http://sources.redhat.com/ml/gdb-patches/2000-10/msg00250.html
> 
> Do you think it's really best to use SYMBOL_SOURCE_NAME, instead of
> just `name'?  I had the impression that the full symbol names were
> always demangled, so the name to use shouldn't depend on the setting
> of the `demangle' flag, as SYMBOL_SOURCE_NAME does.

You are absolutely correct.  SYMBOL_SOURCE_NAME should be (is?) only
for display.  I fixed a couple bugs in this area recently, although I
don't remember whether I posted the patch... I'll check.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer