[rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[Ulrich Weigand]

Hello,

we're still seeing problems on ppc when stepping over shared library
function calls, due to the issue previously discussed e.g. in:
http://sourceware.org/ml/gdb-patches/2006-06/msg00368.html

Now, at the time the discussion primarily focussed on ways how to
identify PLT stubs via e.g. code reading or BFD synthetic symbols.

However, looking at the existing code, it occurs to me that the
current method seems to be a valid approach, it's just that the
implementation is not quite correct.

The basic idea is to make use of a hint the linker adds to 
undefined dynamic symbols.  When the main executable imports
a function from a shared library, it will have an undefined
symbol refering to that function in its dynamic symbol table.

Now, for undefined symbols, the "value" is generally meaningless.
However, in this specific case, the linker actually may place a
significant piece of information into the value field: the address
of the PLT stub used to call the imported function.

It is my understanding that this information is generally reliable,
as it is implemented e.g. to determine the value of a function
pointer constant refering to that symbol (due to the special ABI
rule that the "address" of a function imported into the main 
executable, for function pointer comparison purpuses, is the
address of the PLT stub in the main executable).

Thus, the current elf_symtab_read code attempts to use that hint
to construct a minimal symbol associating that address with the
name of the function.  This is generally the correct thing to do,
as the code found at that address really is the stub used to call
that function.

However, this place in elf_symtab_read uses the section information
from the original undefined symbol as the section that synthetic
minimal symbol resides in .  This seems not particularly useful to me,
as that section is of course always the "undefined" section.

This causes the minimal symbol to be defined in the "undefined"
section, which later on leads to it being ignored ...


Note that this is not a problem with minimal symbols in general,
nor even with mst_solib_trampoline symbols in general, only those
generated using this particular sort of hints in elf_symtab_read.

Thus my suggestion would be to fix this problem by creating those
symbols with a proper section in the first place.  The patch I'm
proposing below attempts to do that by searching for the section
covering the VMA identified as PLT stub address.  If it cannot 
find any, the hint is ignored.


This patch fixes the ppc32 step-over-shared-library-call problem,
fixing about two dozen FAILs in the test suite.


Any comments on that approach?  Am I missing something here?

Bye,
Ulrich


ChangeLog:

	* elfread.c (elf_symtab_read): When constructing a solib trampoline
	minimal symbol from an undefined dynamic symbol, use proper section.

diff -urNp gdb-orig/gdb/elfread.c gdb-head/gdb/elfread.c
--- gdb-orig/gdb/elfread.c	2007-10-21 01:24:25.000000000 +0200
+++ gdb-head/gdb/elfread.c	2007-10-21 14:39:09.702154927 +0200
@@ -240,6 +240,8 @@ elf_symtab_read (struct objfile *objfile
 	  && (sym->flags & BSF_FUNCTION))
 	{
 	  struct minimal_symbol *msym;
+	  bfd *abfd = objfile->obfd;
+	  asection *sect; 
 
 	  /* Symbol is a reference to a function defined in
 	     a shared library.
@@ -252,10 +254,28 @@ elf_symtab_read (struct objfile *objfile
 	  symaddr = sym->value;
 	  if (symaddr == 0)
 	    continue;
-	  symaddr += offset;
+
+	  /* sym->section is the undefined section.  However, we want to
+	     record the section where the PLT stub resides with the
+	     minimal symbol.  Search the section table for the one that
+	     covers the stub's address.  */
+	  for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	    {
+	      if ((bfd_get_section_flags (abfd, sect) & SEC_ALLOC) == 0)
+		continue;
+
+	      if (symaddr >= bfd_get_section_vma (abfd, sect)
+		  && symaddr < bfd_get_section_vma (abfd, sect)
+			       + bfd_get_section_size (sect))
+		break;
+	    }
+	  if (!sect)
+	    continue;
+
+	  symaddr += ANOFFSET (objfile->section_offsets, sect->index);
+
 	  msym = record_minimal_symbol
-	    ((char *) sym->name, symaddr,
-	     mst_solib_trampoline, sym->section, objfile);
+	    ((char *) sym->name, symaddr, mst_solib_trampoline, sect, objfile);
 	  if (msym != NULL)
 	    msym->filename = filesymname;
 	  continue;
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[Daniel Jacobowitz]

On Wed, Oct 31, 2007 at 02:36:39AM +0100, Ulrich Weigand wrote:
> Hello,
> 
> we're still seeing problems on ppc when stepping over shared library
> function calls, due to the issue previously discussed e.g. in:
> http://sourceware.org/ml/gdb-patches/2006-06/msg00368.html
> 
> Now, at the time the discussion primarily focussed on ways how to
> identify PLT stubs via e.g. code reading or BFD synthetic symbols.

Which are otherwise useful too; I really recommend making a synthetic
symtab.  I thought PowerPC BFD already did... oh, 64-bit only.  Right.

> Now, for undefined symbols, the "value" is generally meaningless.
> However, in this specific case, the linker actually may place a
> significant piece of information into the value field: the address
> of the PLT stub used to call the imported function.
> 
> It is my understanding that this information is generally reliable,
> as it is implemented e.g. to determine the value of a function
> pointer constant refering to that symbol (due to the special ABI
> rule that the "address" of a function imported into the main 
> executable, for function pointer comparison purpuses, is the
> address of the PLT stub in the main executable).

Yes.  But if the address of the function is not taken somewhere in the
executable, then the address of the undefined symbol will generally be
left at zero.  That should be true most of the time.  So while I don't
see anything wrong with your approach here, I'm surprised it fixed the
testsuite failures; why do they have non-zero addresses?  If we're
taking their addresses in all cases, we need some new tests.

The patch itself looks fine.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[Ulrich Weigand]

Daniel Jacobowitz:

> Yes.  But if the address of the function is not taken somewhere in the
> executable, then the address of the undefined symbol will generally be
> left at zero.  That should be true most of the time.  So while I don't
> see anything wrong with your approach here, I'm surprised it fixed the
> testsuite failures; why do they have non-zero addresses?  If we're
> taking their addresses in all cases, we need some new tests.

I think this is because there are function calls from the main
executable to those functions, and the main executable is not
compiled with -fPIC.  I understand those references will also 
cause the undefined symbol to carry the address of the main
PLT call stub ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[Daniel Jacobowitz]

On Wed, Oct 31, 2007 at 03:29:53PM +0100, Ulrich Weigand wrote:
> Daniel Jacobowitz:
> 
> > Yes.  But if the address of the function is not taken somewhere in the
> > executable, then the address of the undefined symbol will generally be
> > left at zero.  That should be true most of the time.  So while I don't
> > see anything wrong with your approach here, I'm surprised it fixed the
> > testsuite failures; why do they have non-zero addresses?  If we're
> > taking their addresses in all cases, we need some new tests.
> 
> I think this is because there are function calls from the main
> executable to those functions, and the main executable is not
> compiled with -fPIC.  I understand those references will also 
> cause the undefined symbol to carry the address of the main
> PLT call stub ...

Interesting.  On most platforms, that's not necessary: LD only needs
to set a canonical address in the executable's symtab if the
executable takes the function's address, so this should only happen if
those non-PIC calls were indistinguishable from taking the address.

That's not implemented for PowerPC.  I don't know if it's an ABI
requirement or just a missing optimization.  Search for "st_value = 0"
in elf32-ppc.c versus elf32-i386.c.

So, this won't help everywhere or fix the root cause, but it does seem
like it would work for PowerPC.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [rfc] Fix mst_solib_trampoline symbol sections for PLT stubs

[Ulrich Weigand]

Daniel Jacobowitz wrote:

> Interesting.  On most platforms, that's not necessary: LD only needs
> to set a canonical address in the executable's symtab if the
> executable takes the function's address, so this should only happen if
> those non-PIC calls were indistinguishable from taking the address.
> 
> That's not implemented for PowerPC.  I don't know if it's an ABI
> requirement or just a missing optimization.  Search for "st_value = 0"
> in elf32-ppc.c versus elf32-i386.c.

Interesting indeed; on s390 it looks yet again different.  I don't know
why this is handled so differently across platforms ...

> So, this won't help everywhere or fix the root cause, but it does seem
> like it would work for PowerPC.

OK, I've committed the patch now.  Whenever we do have the canonical
address hint in the undef symbol, we should use it (and the patch
fixes that in any case).  On platforms where we don't have those hints,
some other method needs to work (e.g. synthetic symtab).

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com