[rfc] Fix ELF synthetic symbol size bug (affects PPC64)

[rfc] Fix ELF synthetic symbol size bug (affects PPC64)

[Ulrich Weigand]

Hello,

when debugging PPC64 test case problems, I noticed that symbol sizes 
were completely broken.  This turns out to be caused by elf_symtab_read
accessing the ELF-private part of the BFD symbols it processes in order
to find out the symbol size:

   unsigned long size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
   MSYMBOL_SIZE(msym) = size;

Now, this is not a problem when processing the regular or the dynamic 
symbol tables, because the "asymbol" entries in those tables can in fact
be cast to elf_symbol_type.  However, the same logic is used to process
the "synthetic" symbol table, and *those* are actually pure asymbol data
structures, and the above line will access some random memory.

The patch below fixes this by treating the synthetic symbol table 
differently; this means that we do not get incorrect sizes, but it
unfortunately also means we do not get any sizes at all.

This is particularly problematic because on PPC64, the "regular"
symbol for a function points to the function descriptor, and the
actual code of the function is only identified by a synthetic
symbol.  This means we basically do not get any size information
for functions at all.

Interestingly enough, BFD seems to actually have the size of the
function code available; it's just not possible right now to pass
that information through the "synthetic symbol" interface ...

Tested on powerpc64-linux, fixes about 20 FAILs.

I'd appreciate any comments, in particular whether I'm overlooking
a way to get at the sizes after all.

Bye,
Ulrich


ChangeLog:

	* elfread.c (ST_REGULAR, ST_DYNAMIC, ST_SYNTHETIC): New defines.
	(elf_symtab_read): Rename DYNAMIC argument to TYPE.  Do not access
	ELF-private symbol data when processing synthetic symbols.
	(elf_symfile_read): Pass TYPE argument to elf_symtab_read.

diff -urNp gdb-orig/gdb/elfread.c gdb-head/gdb/elfread.c
--- gdb-orig/gdb/elfread.c	2007-11-01 14:50:26.000000000 +0100
+++ gdb-head/gdb/elfread.c	2007-11-05 20:04:50.915454504 +0100
@@ -180,14 +180,14 @@ record_minimal_symbol (char *name, CORE_
 
    SYNOPSIS
 
-   void elf_symtab_read (struct objfile *objfile, int dynamic,
+   void elf_symtab_read (struct objfile *objfile, int type,
 			 long number_of_symbols, asymbol **symbol_table)
 
    DESCRIPTION
 
    Given an objfile, a symbol table, and a flag indicating whether the
-   symbol table contains dynamic symbols, add all the global function
-   and data symbols to the minimal symbol table.
+   symbol table contains regular, dynamic, or synthetic symbols, add all
+   the global function and data symbols to the minimal symbol table.
 
    In stabs-in-ELF, as implemented by Sun, there are some local symbols
    defined in the ELF symbol table, which can be used to locate
@@ -197,8 +197,12 @@ record_minimal_symbol (char *name, CORE_
 
  */
 
+#define ST_REGULAR 0
+#define ST_DYNAMIC 1
+#define ST_SYNTHETIC 2
+
 static void
-elf_symtab_read (struct objfile *objfile, int dynamic,
+elf_symtab_read (struct objfile *objfile, int type,
 		 long number_of_symbols, asymbol **symbol_table)
 {
   long storage_needed;
@@ -235,7 +239,7 @@ elf_symtab_read (struct objfile *objfile
 	continue;
 
       offset = ANOFFSET (objfile->section_offsets, sym->section->index);
-      if (dynamic
+      if (type == ST_DYNAMIC
 	  && sym->section == &bfd_und_section
 	  && (sym->flags & BSF_FUNCTION))
 	{
@@ -284,7 +288,7 @@ elf_symtab_read (struct objfile *objfile
       /* If it is a nonstripped executable, do not enter dynamic
 	 symbols, as the dynamic symbol table is usually a subset
 	 of the main symbol table.  */
-      if (dynamic && !stripped)
+      if (type == ST_DYNAMIC && !stripped)
 	continue;
       if (sym->flags & BSF_FILE)
 	{
@@ -324,8 +328,11 @@ elf_symtab_read (struct objfile *objfile
 	    {
 	      /* This is a hack to get the minimal symbol type
 		 right for Irix 5, which has absolute addresses
-		 with special section indices for dynamic symbols. */
-	      unsigned short shndx =
+		 with special section indices for dynamic symbols.
+
+		 NOTE: uweigand-20071105: Synthetic symbols do not
+		 have an ELF-private part, so do not touch those.  */
+	      unsigned short shndx = type == ST_SYNTHETIC ? 0 : 
 		((elf_symbol_type *) sym)->internal_elf_sym.st_shndx;
 
 	      switch (shndx)
@@ -484,9 +491,14 @@ elf_symtab_read (struct objfile *objfile
 	  msym = record_minimal_symbol
 	    ((char *) sym->name, symaddr,
 	     ms_type, sym->section, objfile);
-	  if (msym)
+
+	  /* Pass symbol size field in via BFD.  FIXME!!!
+
+	     FIXME: uweigand-20071105: This unfortunately does not work
+	     for synthetic symbols, as they do not have an ELF-private part.
+	     This means we will not get sizes for ppc64 functions. */
+	  if (msym && type != ST_SYNTHETIC)
 	    {
-	      /* Pass symbol size field in via BFD.  FIXME!!!  */
 	      unsigned long size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
 	      MSYMBOL_SIZE(msym) = size;
 	    }
@@ -569,7 +581,7 @@ elf_symfile_read (struct objfile *objfil
 	error (_("Can't read symbols from %s: %s"), bfd_get_filename (objfile->obfd),
 	       bfd_errmsg (bfd_get_error ()));
 
-      elf_symtab_read (objfile, 0, symcount, symbol_table);
+      elf_symtab_read (objfile, ST_REGULAR, symcount, symbol_table);
     }
 
   /* Add the dynamic symbols.  */
@@ -587,7 +599,7 @@ elf_symfile_read (struct objfile *objfil
 	error (_("Can't read symbols from %s: %s"), bfd_get_filename (objfile->obfd),
 	       bfd_errmsg (bfd_get_error ()));
 
-      elf_symtab_read (objfile, 1, dynsymcount, dyn_symbol_table);
+      elf_symtab_read (objfile, ST_DYNAMIC, dynsymcount, dyn_symbol_table);
     }
 
   /* Add synthetic symbols - for instance, names for any PLT entries.  */
@@ -605,7 +617,7 @@ elf_symfile_read (struct objfile *objfil
       for (i = 0; i < synthcount; i++)
 	synth_symbol_table[i] = synthsyms + i;
       make_cleanup (xfree, synth_symbol_table);
-      elf_symtab_read (objfile, 0, synthcount, synth_symbol_table);
+      elf_symtab_read (objfile, ST_SYNTHETIC, synthcount, synth_symbol_table);
     }
 
   /* Install any minimal symbols that have been collected as the current
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [rfc] Fix ELF synthetic symbol size bug (affects PPC64)

[Daniel Jacobowitz]

On Mon, Nov 05, 2007 at 09:14:44PM +0100, Ulrich Weigand wrote:
> I'd appreciate any comments, in particular whether I'm overlooking
> a way to get at the sizes after all.

Seems OK to me; I could not find another way to get the size out of
BFD either, which is a shame.

-- 
Daniel Jacobowitz
CodeSourcery

[rfc, v2] Fix ELF synthetic symbol size bug (affects PPC64)

[Ulrich Weigand]

Hello,

in http://sourceware.org/ml/gdb-patches/2007-11/msg00080.html I wrote:

> This is particularly problematic because on PPC64, the "regular"
> symbol for a function points to the function descriptor, and the
> actual code of the function is only identified by a synthetic
> symbol.  This means we basically do not get any size information
> for functions at all.
> 
> Interestingly enough, BFD seems to actually have the size of the
> function code available; it's just not possible right now to pass
> that information through the "synthetic symbol" interface ...

Alan suggested a way to pass that information: have the udata.p
member of the synthetic symbol point back to the full ELF symbol
structure of the original symbol used to derive the synthetic
symbol from.   The patch below implements this.

Tested (GDB) on powerpc64-linux, fixes another 12 FAILs on top
of the ones already fixed by the original patch.

Is the bfd part OK?

Bye,
Ulrich


bfd/ChangeLog:

	* elf.c (_bfd_elf_get_synthetic_symtab): Set udata.p of synthetic
	symbol to point back to the ELF symbol it was derived from.
	* elf64-ppc.c (ppc64_elf_get_synthetic_symtab): Likewise.


gdb/ChangeLog:

        * elfread.c (ST_REGULAR, ST_DYNAMIC, ST_SYNTHETIC): New defines.
        (elf_symtab_read): Rename DYNAMIC argument to TYPE.  Do not access
        ELF-private symbol data when processing synthetic symbols.
        (elf_symfile_read): Pass TYPE argument to elf_symtab_read.


diff -urNp gdb-orig/bfd/elf64-ppc.c gdb-head/bfd/elf64-ppc.c
--- gdb-orig/bfd/elf64-ppc.c	2007-11-06 20:31:59.661739207 +0100
+++ gdb-head/bfd/elf64-ppc.c	2007-11-06 20:17:17.399685878 +0100
@@ -2959,6 +2959,9 @@ ppc64_elf_get_synthetic_symtab (bfd *abf
 	      len = strlen (syms[i]->name);
 	      memcpy (names, syms[i]->name, len + 1);
 	      names += len + 1;
+	      /* Have udata.p point back to the original symbol this
+		 synthetic symbol was derived from.  */
+	      s->udata.p = syms[i];
 	      s++;
 	    }
 	}
@@ -3046,6 +3049,9 @@ ppc64_elf_get_synthetic_symtab (bfd *abf
 	      len = strlen (syms[i]->name);
 	      memcpy (names, syms[i]->name, len + 1);
 	      names += len + 1;
+	      /* Have udata.p point back to the original symbol this
+		 synthetic symbol was derived from.  */
+	      s->udata.p = syms[i];
 	      s++;
 	    }
 	}
diff -urNp gdb-orig/bfd/elf.c gdb-head/bfd/elf.c
--- gdb-orig/bfd/elf.c	2007-11-06 20:31:59.687735465 +0100
+++ gdb-head/bfd/elf.c	2007-11-06 20:17:56.179085383 +0100
@@ -8700,6 +8700,9 @@ _bfd_elf_get_synthetic_symtab (bfd *abfd
       names += len;
       memcpy (names, "@plt", sizeof ("@plt"));
       names += sizeof ("@plt");
+      /* Have udata.p point back to the original symbol this
+	 synthetic symbol was derived from.  */
+      s->udata.p = *p->sym_ptr_ptr;
       ++n;
     }
 
diff -urNp gdb-orig/gdb/elfread.c gdb-head/gdb/elfread.c
--- gdb-orig/gdb/elfread.c	2007-11-06 21:10:11.292646086 +0100
+++ gdb-head/gdb/elfread.c	2007-11-06 20:37:46.532472318 +0100
@@ -180,14 +180,14 @@ record_minimal_symbol (char *name, CORE_
 
    SYNOPSIS
 
-   void elf_symtab_read (struct objfile *objfile, int dynamic,
+   void elf_symtab_read (struct objfile *objfile, int type,
 			 long number_of_symbols, asymbol **symbol_table)
 
    DESCRIPTION
 
    Given an objfile, a symbol table, and a flag indicating whether the
-   symbol table contains dynamic symbols, add all the global function
-   and data symbols to the minimal symbol table.
+   symbol table contains regular, dynamic, or synthetic symbols, add all
+   the global function and data symbols to the minimal symbol table.
 
    In stabs-in-ELF, as implemented by Sun, there are some local symbols
    defined in the ELF symbol table, which can be used to locate
@@ -197,8 +197,12 @@ record_minimal_symbol (char *name, CORE_
 
  */
 
+#define ST_REGULAR 0
+#define ST_DYNAMIC 1
+#define ST_SYNTHETIC 2
+
 static void
-elf_symtab_read (struct objfile *objfile, int dynamic,
+elf_symtab_read (struct objfile *objfile, int type,
 		 long number_of_symbols, asymbol **symbol_table)
 {
   long storage_needed;
@@ -235,7 +239,7 @@ elf_symtab_read (struct objfile *objfile
 	continue;
 
       offset = ANOFFSET (objfile->section_offsets, sym->section->index);
-      if (dynamic
+      if (type == ST_DYNAMIC
 	  && sym->section == &bfd_und_section
 	  && (sym->flags & BSF_FUNCTION))
 	{
@@ -284,7 +288,7 @@ elf_symtab_read (struct objfile *objfile
       /* If it is a nonstripped executable, do not enter dynamic
 	 symbols, as the dynamic symbol table is usually a subset
 	 of the main symbol table.  */
-      if (dynamic && !stripped)
+      if (type == ST_DYNAMIC && !stripped)
 	continue;
       if (sym->flags & BSF_FILE)
 	{
@@ -324,8 +328,11 @@ elf_symtab_read (struct objfile *objfile
 	    {
 	      /* This is a hack to get the minimal symbol type
 		 right for Irix 5, which has absolute addresses
-		 with special section indices for dynamic symbols. */
-	      unsigned short shndx =
+		 with special section indices for dynamic symbols.
+
+		 NOTE: uweigand/20071105: Synthetic symbols do not
+		 have an ELF-private part, so do not touch those.  */
+	      unsigned short shndx = type == ST_SYNTHETIC ? 0 : 
 		((elf_symbol_type *) sym)->internal_elf_sym.st_shndx;
 
 	      switch (shndx)
@@ -484,11 +491,22 @@ elf_symtab_read (struct objfile *objfile
 	  msym = record_minimal_symbol
 	    ((char *) sym->name, symaddr,
 	     ms_type, sym->section, objfile);
+
 	  if (msym)
 	    {
 	      /* Pass symbol size field in via BFD.  FIXME!!!  */
-	      unsigned long size = ((elf_symbol_type *) sym)->internal_elf_sym.st_size;
-	      MSYMBOL_SIZE(msym) = size;
+	      elf_symbol_type *elf_sym;
+
+	      /* NOTE: uweigand/20071105: A synthetic symbol does not have an
+		 ELF-private part.  However, its udata.p entry points back to
+		 the original ELF symbol it was derived from.  Get the size
+		 from that symbol.  */ 
+	      if (type != ST_SYNTHETIC)
+		elf_sym = (elf_symbol_type *) sym;
+	      else
+		elf_sym = (elf_symbol_type *) sym->udata.p;
+
+	      MSYMBOL_SIZE(msym) = elf_sym->internal_elf_sym.st_size;
 	    }
 	  if (msym != NULL)
 	    msym->filename = filesymname;
@@ -569,7 +587,7 @@ elf_symfile_read (struct objfile *objfil
 	error (_("Can't read symbols from %s: %s"), bfd_get_filename (objfile->obfd),
 	       bfd_errmsg (bfd_get_error ()));
 
-      elf_symtab_read (objfile, 0, symcount, symbol_table);
+      elf_symtab_read (objfile, ST_REGULAR, symcount, symbol_table);
     }
 
   /* Add the dynamic symbols.  */
@@ -587,7 +605,7 @@ elf_symfile_read (struct objfile *objfil
 	error (_("Can't read symbols from %s: %s"), bfd_get_filename (objfile->obfd),
 	       bfd_errmsg (bfd_get_error ()));
 
-      elf_symtab_read (objfile, 1, dynsymcount, dyn_symbol_table);
+      elf_symtab_read (objfile, ST_DYNAMIC, dynsymcount, dyn_symbol_table);
     }
 
   /* Add synthetic symbols - for instance, names for any PLT entries.  */
@@ -605,7 +623,7 @@ elf_symfile_read (struct objfile *objfil
       for (i = 0; i < synthcount; i++)
 	synth_symbol_table[i] = synthsyms + i;
       make_cleanup (xfree, synth_symbol_table);
-      elf_symtab_read (objfile, 0, synthcount, synth_symbol_table);
+      elf_symtab_read (objfile, ST_SYNTHETIC, synthcount, synth_symbol_table);
     }
 
   /* Install any minimal symbols that have been collected as the current

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

Re: [rfc, v2] Fix ELF synthetic symbol size bug (affects PPC64)

[Daniel Jacobowitz]

On Tue, Nov 06, 2007 at 10:33:04PM +0100, Ulrich Weigand wrote:
> Alan suggested a way to pass that information: have the udata.p
> member of the synthetic symbol point back to the full ELF symbol
> structure of the original symbol used to derive the synthetic
> symbol from.   The patch below implements this.
> 
> Tested (GDB) on powerpc64-linux, fixes another 12 FAILs on top
> of the ones already fixed by the original patch.
> 
> Is the bfd part OK?

The elf.c part of this does not make sense to me.  Why should we have
the same size as that symbol?  The @plt entry is only a couple of
instructions.  To figure out its size you need to ask the backend
plt_sym_val routine.

-- 
Daniel Jacobowitz
CodeSourcery