Bug handling zero sized symbols in minsyms.c

Bug handling zero sized symbols in minsyms.c

[Robert Norton]

Hi,

In minsyms.c:lookup_minimal_symbol_by_pc_section() there is some code
which attempts to prefer symbols with sizes over those with zero size.
This is quite useful[1]. Unfortunately the present code will only work
if there is at most one zero-sized symbol. The fix is around line 503:

 if (MSYMBOL_SIZE (&msymbol[hi]) == 0
   && best_zero_sized == -1)
   {
   	best_zero_sized = hi;
      hi--;          
      continue;      
   }

SHOULD be:

if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
	{
	      if (best_zero_sized == -1)
	        best_zero_sized = hi;
	      hi--;
	      continue;
	}

We keep the highest zero-sized symbol as the best but continue to
iterate backwards until we hit a non-zero-sized symbol or run out of
symbols. It's pretty clear that this is what was originally intended.

I can get copyright assigment for this if required although it seems
pretty trivial...

Cheers,

Robert

[1] In particular it is useful when debugging assembly functions which
have internal labels for loops etc. Without this fix we sometimes get
back a minsym corresponding to an internal label (e.g. a loop) when
really what we wanted was the function symbol. This messes up prologue
analysis and some other things. For example in our GDB port the assembly
file:

.global main
main:
        nop
test2:
        nop
test3:
        nop
.size main,.-main
.type main,@function

results in:

(gdb) info sym test2
main + 8 in section .text
(gdb) info sym test3
test3 in section .text   <----------------- !!!
(gdb) disas main
Dump of assembler code for function main:
0x00000270 <main+0>:     NOP
0x00000278 <main+8>:     NOP
0x00000280 <test3+0>:    NOP    <----------------- !!!
End of assembler dump.

and in a patched version:

(gdb) info sym test2
main + 8 in section .text
(gdb) info sym test3
main + 16 in section .text
(gdb) disas main
Dump of assembler code for function main:
0x00000270 <main+0>:     NOP
0x00000278 <main+8>:     NOP
0x00000280 <main+16>:    NOP
End of assembler dump.

RCS file: /cvs/dev/tools/src/binutils/gdb/minsyms.c,v
retrieving revision 1.3
diff -u -r1.3 minsyms.c
--- minsyms.c   4 Jan 2008 18:33:25 -0000       1.3
+++ minsyms.c   2 Jul 2008 16:36:02 -0000
@@ -503,10 +503,10 @@
                     symbol isn't an object or function (e.g. a
                     label), or it may just mean that the size was not
                     specified.  */
-                 if (MSYMBOL_SIZE (&msymbol[hi]) == 0
-                     && best_zero_sized == -1)
+                 if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
                    {
-                     best_zero_sized = hi;
+                     if (best_zero_sized == -1)
+                       best_zero_sized = hi;
                      hi--;
                      continue;
                    }

Re: Bug handling zero sized symbols in minsyms.c

[Michael Snyder]

On Wed, 2008-07-02 at 09:58 -0700, Robert Norton wrote:
> Hi,
> 
> In minsyms.c:lookup_minimal_symbol_by_pc_section() there is some code
> which attempts to prefer symbols with sizes over those with zero size.
> This is quite useful[1]. Unfortunately the present code will only work
> if there is at most one zero-sized symbol. The fix is around line 503:
> 
>  if (MSYMBOL_SIZE (&msymbol[hi]) == 0
>    && best_zero_sized == -1)
>    {
>    	best_zero_sized = hi;
>       hi--;          
>       continue;      
>    }
> 
> SHOULD be:
> 
> if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
> 	{
> 	      if (best_zero_sized == -1)
> 	        best_zero_sized = hi;
> 	      hi--;
> 	      continue;
> 	}
> 
> We keep the highest zero-sized symbol as the best but continue to
> iterate backwards until we hit a non-zero-sized symbol or run out of
> symbols. It's pretty clear that this is what was originally intended.

Your change is good, thanks.

> I can get copyright assigment for this if required although it seems
> pretty trivial...

Not necessary, but feel free to if you'd like.
I think this is at least the second change you've submitted.

Could you give us a change log entry please?

RE: Bug handling zero sized symbols in minsyms.c

[Robert Norton]

> -----Original Message-----
> From: Michael Snyder [mailto:msnyder@specifix.com] 
> Sent: 02 July 2008 18:45
> To: Robert Norton
> Cc: gdb@sourceware.org
> Subject: Re: Bug handling zero sized symbols in minsyms.c
> 
> On Wed, 2008-07-02 at 09:58 -0700, Robert Norton wrote:
> > Hi,
> > 
> > In minsyms.c:lookup_minimal_symbol_by_pc_section() there is 
> some code
> > which attempts to prefer symbols with sizes over those with 
> zero size.
> > This is quite useful[1]. Unfortunately the present code 
> will only work
> > if there is at most one zero-sized symbol. The fix is 
> around line 503:
> > 
> >  if (MSYMBOL_SIZE (&msymbol[hi]) == 0
> >    && best_zero_sized == -1)
> >    {
> >    	best_zero_sized = hi;
> >       hi--;          
> >       continue;      
> >    }
> > 
> > SHOULD be:
> > 
> > if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
> > 	{
> > 	      if (best_zero_sized == -1)
> > 	        best_zero_sized = hi;
> > 	      hi--;
> > 	      continue;
> > 	}
> > 
> > We keep the highest zero-sized symbol as the best but continue to
> > iterate backwards until we hit a non-zero-sized symbol or run out of
> > symbols. It's pretty clear that this is what was originally 
> intended.
> 
> Your change is good, thanks.
> 
> > I can get copyright assigment for this if required although it seems
> > pretty trivial...
> 
> Not necessary, but feel free to if you'd like.
> I think this is at least the second change you've submitted.

It turns out that we (Broadcom) already have one on file, so no problems
there.

> Could you give us a change log entry please?

How about:

2008-07-03 Robert Norton (rnorton@broadcom.com)

   * minsyms.c: Fix a bug with ignoring zero-sized symbols when looking
up the minsym for a PC.

Cheers,

Robert

RE: Bug handling zero sized symbols in minsyms.c

[Robert Norton]

> -----Original Message-----
> From: gdb-owner@sourceware.org 
> [mailto:gdb-owner@sourceware.org] On Behalf Of Robert Norton
> Sent: 03 July 2008 11:17
> To: Michael Snyder
> Cc: gdb@sourceware.org
> Subject: RE: Bug handling zero sized symbols in minsyms.c
> 
> > -----Original Message-----
> > From: Michael Snyder [mailto:msnyder@specifix.com] 
> > Sent: 02 July 2008 18:45
> > To: Robert Norton
> > Cc: gdb@sourceware.org
> > Subject: Re: Bug handling zero sized symbols in minsyms.c
> > 
> > On Wed, 2008-07-02 at 09:58 -0700, Robert Norton wrote:
> > > Hi,
> > > 
> > > In minsyms.c:lookup_minimal_symbol_by_pc_section() there is 
> > some code
> > > which attempts to prefer symbols with sizes over those with 
> > zero size.
> > > This is quite useful[1]. Unfortunately the present code 
> > will only work
> > > if there is at most one zero-sized symbol. The fix is 
> > around line 503:
> > > 
> > >  if (MSYMBOL_SIZE (&msymbol[hi]) == 0
> > >    && best_zero_sized == -1)
> > >    {
> > >    	best_zero_sized = hi;
> > >       hi--;          
> > >       continue;      
> > >    }
> > > 
> > > SHOULD be:
> > > 
> > > if (MSYMBOL_SIZE (&msymbol[hi]) == 0)
> > > 	{
> > > 	      if (best_zero_sized == -1)
> > > 	        best_zero_sized = hi;
> > > 	      hi--;
> > > 	      continue;
> > > 	}
> > > 
> > > We keep the highest zero-sized symbol as the best but continue to
> > > iterate backwards until we hit a non-zero-sized symbol or 
> run out of
> > > symbols. It's pretty clear that this is what was originally 
> > intended.
> > 
> > Your change is good, thanks.
> > 
> > > I can get copyright assigment for this if required 
> although it seems
> > > pretty trivial...
> > 
> > Not necessary, but feel free to if you'd like.
> > I think this is at least the second change you've submitted.
> 
> It turns out that we (Broadcom) already have one on file, so 
> no problems
> there.
> 
> > Could you give us a change log entry please?
> 
> How about:
> 
> 2008-07-03 Robert Norton (rnorton@broadcom.com)
> 
>    * minsyms.c: Fix a bug with ignoring zero-sized symbols 
> when looking
> up the minsym for a PC.

What is the status of this patch?

Robert