[RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Joel Brobecker]

Our testsuite noticed a problem on ia64-hpux when trying to compute
a backtrace inside one of our Ada programs. In one of the frames,
it said that the function name was `uwx_get_reg' instead of the
name of our main subprogram (in our case, `_ada_test_protected').

Exploring further, I found that the code address for the frame was
correct, and that the problem was the reverse lookup of the symbol
at that address. It turns out:

  1. When searching the symtabs, it found the correct symbol;
  2. Regardless of the above, it still searched the minsyms, and
     found a different symbol (uwx_get_reg) whose address is closer
     to our PC;
  3. Based on the above, it decided that the correct answer is
     the latter choice.

Unfortunately, this is not correct in our case, simply because the
entry in the symbol table seems bogus. `uwx_get_reg' is provided
by the libunwind shared library, and the entry in the symbol table
looks like this:

    40000000000182e0       F *UND*  0000000000000000 uwx_get_reg

Normally, such entries either have a null value, or their value
corresponds to an address in the PLT section.  But, in this case,
it actually points to some real (user) code in the .text section,
right in the middle of our main subprogram. This is what we have
in the symbol table for our main subprogram:

    4000000000018060 g     F .text  0000000000000570 _ada_test_protected

The debug info probably says exactly the same thing in terms of
the code boundaries.  I've also verified by visual inspection that
the instructions at those addresses correspond to the instructions
I found in the assembly file.

So the undefined symbol entry for uwx_get_reg seems bogus.  We use
the system linker, so we cannot fix the problem there.  So I tried
to find a way to work around this in GDB. Normally, we ignore entries
with a null value. Here, the value is not null, so we expect it to
be a reference to the corresponding PLT.  The check I am adding is
to verify that it seems plausible by checking the name of the section
we think this address corresponds to.  It bets on the fact that it
usually is ".plt".  And to survive the theoretical situation where
the symbol is a valid reference to a valid PLT which lives in
a section not named ".plt", I added a check that makes sure that
we only discard such symbol if there is a ".plt" section. In other
words, if the symbol points to a section not named ".plt", and
there is no section named ".plt", then we retain it.

gdb/ChangeLog:

        * elfread.c (elf_symtab_read): Ignore undefined symbols with
        nonzero addresses if they do not correspond to a .plt section
        when one is available in the objfile.

Tested on ia64-hpux. No regressions.

Thoughts? Worth a try?

---
 gdb/elfread.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/gdb/elfread.c b/gdb/elfread.c
index a309a2c..866226d 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -303,6 +303,23 @@ elf_symtab_read (struct objfile *objfile, int type,
 	  if (!sect)
 	    continue;
 
+	  /* On ia64-hpux, we have discovered that the system linker
+	     adds undefined symbols with nonzero addresses that cannot
+	     be right (their address points inside the code of another
+	     function in the .text section).  This creates problems
+	     when trying to determine which symbol corresponds to
+	     a given address.
+
+	     We try to detect those buggy symbols by checking which
+	     section we think they correspond to.  Normally, PLT symbols
+	     are stored inside their own section, and the typical name
+	     for that section is ".plt".  So, if there is a ".plt"
+	     section, and yet the section name of our symbol does not
+	     start with ".plt", we ignore that symbol.  */
+	  if (strncmp (sect->name, ".plt", 4) != 0
+	      && bfd_get_section_by_name (abfd, ".plt") != NULL)
+	    continue;
+
 	  symaddr += ANOFFSET (objfile->section_offsets, sect->index);
 
 	  msym = record_minimal_symbol
-- 
1.7.1

Re: [RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Jan Kratochvil]

On Wed, 19 Oct 2011 02:23:37 +0200, Joel Brobecker wrote:
> Normally, such entries either have a null value, or their value
> corresponds to an address in the PLT section.

Do you have some such file with undefinde symbol and non-zero value?


> +	  if (strncmp (sect->name, ".plt", 4) != 0
> +	      && bfd_get_section_by_name (abfd, ".plt") != NULL)

At least the bfd_get_section_by_name result should be cached.


This is not a review.


Thanks,
Jan

Re: [RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Joel Brobecker]

> > Normally, such entries either have a null value, or their value
> > corresponds to an address in the PLT section.
> 
> Do you have some such file with undefinde symbol and non-zero value?

Do you mean, other than on ia64-hpux? My comment is just repeating
what the comment in that file says, so I assume it's true somewhere.

> > +	  if (strncmp (sect->name, ".plt", 4) != 0
> > +	      && bfd_get_section_by_name (abfd, ".plt") != NULL)
> 
> At least the bfd_get_section_by_name result should be cached.

I didn't think it was going to make much of a difference, since
this only happens for symbols that point outside of the .plt
section, which I am hoping would be seldom.  But if others agree
with the caching (or even if it does not really matter to them),
it's no problem.

As a datapoint, my executable has 96 undefined symbols for a total
of roughly 1150 symbols in total. Total number of sections is
almost 50 with .plt at index 30. That indeed suggest that we can
get a good improvement in terms of performance, although I don't
think it be noticeable as far as the user is concerned.

I'll wait for an official review before making any adjustment.
Thanks for your comments,
-- 
Joel

Re: [RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Jan Kratochvil]

On Fri, 21 Oct 2011 19:29:58 +0200, Joel Brobecker wrote:
> > Do you have some such file with undefinde symbol and non-zero value?
> 
> Do you mean, other than on ia64-hpux?

You suggest the non-zero values are really random while I thought there could
be some coincidence what the value means.  But it is true you would figure it
out yourself so we have to consider it as a random value.


> > > +	  if (strncmp (sect->name, ".plt", 4) != 0
> > > +	      && bfd_get_section_by_name (abfd, ".plt") != NULL)
> > 
> > At least the bfd_get_section_by_name result should be cached.
> 
> I didn't think it was going to make much of a difference,

OK, I agree it is better this way as the conditional is evaluated only for
very few ELF symbols, not for all ELF symbols.


Thanks,
Jan

Re: [RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Joel Brobecker]

On Tue, Oct 18, 2011 at 05:23:37PM -0700, Joel Brobecker wrote:
> gdb/ChangeLog:
> 
>         * elfread.c (elf_symtab_read): Ignore undefined symbols with
>         nonzero addresses if they do not correspond to a .plt section
>         when one is available in the objfile.

No real comment on this except from Jan. I think the bottom line is
that we're not really sure it might not have any unwanted side-effect.
But on the other hand, we've had it in AdaCore's  tree for a while now,
with no visible side-effect, while it does catch a real problem.

URL for the discussion:
http://www.sourceware.org/ml/gdb-patches/2011-10/msg00517.html

I suggest we check it in after the 7.4 branch is created.

> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index a309a2c..866226d 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -303,6 +303,23 @@ elf_symtab_read (struct objfile *objfile, int type,
>  	  if (!sect)
>  	    continue;
>  
> +	  /* On ia64-hpux, we have discovered that the system linker
> +	     adds undefined symbols with nonzero addresses that cannot
> +	     be right (their address points inside the code of another
> +	     function in the .text section).  This creates problems
> +	     when trying to determine which symbol corresponds to
> +	     a given address.
> +
> +	     We try to detect those buggy symbols by checking which
> +	     section we think they correspond to.  Normally, PLT symbols
> +	     are stored inside their own section, and the typical name
> +	     for that section is ".plt".  So, if there is a ".plt"
> +	     section, and yet the section name of our symbol does not
> +	     start with ".plt", we ignore that symbol.  */
> +	  if (strncmp (sect->name, ".plt", 4) != 0
> +	      && bfd_get_section_by_name (abfd, ".plt") != NULL)
> +	    continue;
> +
>  	  symaddr += ANOFFSET (objfile->section_offsets, sect->index);
>  
>  	  msym = record_minimal_symbol

-- 
Joel

Re: [RFA/RFC] try ignoring bad PLT entries in ELF symbol tables

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * elfread.c (elf_symtab_read): Ignore undefined symbols with
>         nonzero addresses if they do not correspond to a .plt section
>         when one is available in the objfile.

This patch is now in HEAD.

-- 
Joel