SHN_XINDEX support is broken

SHN_XINDEX support is broken

[H. J. Lu]

Hi Alan,

SHN_XINDEX support doesn't work. I have a testcase to show it. The
problem is that binutils assumes the full symbol table is available
before all sections are processed. It only works without SHN_XINDEX
since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
affects both bfd and readelf. I will see what I can do.


H.J.

PATCH: SHN_XINDEX support is broken

[H. J. Lu]

On Fri, Feb 04, 2005 at 03:43:28PM -0800, H. J. Lu wrote:
> Hi Alan,
> 
> SHN_XINDEX support doesn't work. I have a testcase to show it. The
> problem is that binutils assumes the full symbol table is available
> before all sections are processed. It only works without SHN_XINDEX
> since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> affects both bfd and readelf. I will see what I can do.
> 
> 

This patch fixes bfd. I will fix readelf later.


H.J.
----
2005-02-04  H.J. Lu  <hongjiu.lu@intel.com>

	* elfcode.h (elf_object_p): Read in SHT_SYMTAB and
	SHT_SYMTAB_SHNDX sections first.

--- bfd/elfcode.h.ext	2005-01-30 09:33:38.000000000 -0800
+++ bfd/elfcode.h	2005-02-04 16:20:50.585023878 -0800
@@ -725,20 +725,51 @@ elf_object_p (bfd *abfd)
 
   if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff)
     {
-      unsigned int num_sec;
+      unsigned int num_sec, sec_count;
 
       shstrtab = bfd_elf_get_str_section (abfd, i_ehdrp->e_shstrndx);
       if (!shstrtab)
 	goto got_no_match;
 
+      /* We need to read in SHT_SYMTAB and SHT_SYMTAB_SHNDX sections
+	 first since bfd_section_from_shdr may use them.  */        
+      num_sec = elf_numsections (abfd);
+      sec_count = (num_sec < SHN_LORESERVE) ? 1 : 0;
+      for (shindex = 1; sec_count < 2 && shindex < num_sec; shindex++)
+	{
+	  Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
+	  switch (hdr->sh_type)
+	    {
+	    case SHT_SYMTAB:
+	    case SHT_SYMTAB_SHNDX:
+	      if (! bfd_section_from_shdr (abfd, shindex))
+		goto got_no_match;
+	      sec_count++;
+	      break;
+	    default:
+	      break;
+	    }
+	  if (shindex == SHN_LORESERVE - 1)
+	    shindex += SHN_HIRESERVE + 1 - SHN_LORESERVE;
+	}
+
       /* Once all of the section headers have been read and converted, we
 	 can start processing them.  Note that the first section header is
 	 a dummy placeholder entry, so we ignore it.  */
       num_sec = elf_numsections (abfd);
       for (shindex = 1; shindex < num_sec; shindex++)
 	{
-	  if (! bfd_section_from_shdr (abfd, shindex))
-	    goto got_no_match;
+	  Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
+	  switch (hdr->sh_type)
+	    {
+	    case SHT_SYMTAB:
+	    case SHT_SYMTAB_SHNDX:
+	      break;
+	    default:
+	      if (! bfd_section_from_shdr (abfd, shindex))
+		goto got_no_match;
+	      break;
+	    }
 	  if (shindex == SHN_LORESERVE - 1)
 	    shindex += SHN_HIRESERVE + 1 - SHN_LORESERVE;
 	}

PATCH: SHN_XINDEX support is broken

[H. J. Lu]

On Fri, Feb 04, 2005 at 03:43:28PM -0800, H. J. Lu wrote:
> Hi Alan,
> 
> SHN_XINDEX support doesn't work. I have a testcase to show it. The
> problem is that binutils assumes the full symbol table is available
> before all sections are processed. It only works without SHN_XINDEX
> since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> affects both bfd and readelf. I will see what I can do.
> 
> 

This patch fixes bfd. I will fix readelf later.


H.J.
----
2005-02-04  H.J. Lu  <hongjiu.lu@intel.com>

	* elfcode.h (elf_object_p): Read in SHT_SYMTAB and
	SHT_SYMTAB_SHNDX sections first.

--- bfd/elfcode.h.ext	2005-01-30 09:33:38.000000000 -0800
+++ bfd/elfcode.h	2005-02-04 16:20:50.585023878 -0800
@@ -725,20 +725,51 @@ elf_object_p (bfd *abfd)
 
   if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff)
     {
-      unsigned int num_sec;
+      unsigned int num_sec, sec_count;
 
       shstrtab = bfd_elf_get_str_section (abfd, i_ehdrp->e_shstrndx);
       if (!shstrtab)
 	goto got_no_match;
 
+      /* We need to read in SHT_SYMTAB and SHT_SYMTAB_SHNDX sections
+	 first since bfd_section_from_shdr may use them.  */        
+      num_sec = elf_numsections (abfd);
+      sec_count = (num_sec < SHN_LORESERVE) ? 1 : 0;
+      for (shindex = 1; sec_count < 2 && shindex < num_sec; shindex++)
+	{
+	  Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
+	  switch (hdr->sh_type)
+	    {
+	    case SHT_SYMTAB:
+	    case SHT_SYMTAB_SHNDX:
+	      if (! bfd_section_from_shdr (abfd, shindex))
+		goto got_no_match;
+	      sec_count++;
+	      break;
+	    default:
+	      break;
+	    }
+	  if (shindex == SHN_LORESERVE - 1)
+	    shindex += SHN_HIRESERVE + 1 - SHN_LORESERVE;
+	}
+
       /* Once all of the section headers have been read and converted, we
 	 can start processing them.  Note that the first section header is
 	 a dummy placeholder entry, so we ignore it.  */
       num_sec = elf_numsections (abfd);
       for (shindex = 1; shindex < num_sec; shindex++)
 	{
-	  if (! bfd_section_from_shdr (abfd, shindex))
-	    goto got_no_match;
+	  Elf_Internal_Shdr *hdr = elf_elfsections (abfd)[shindex];
+	  switch (hdr->sh_type)
+	    {
+	    case SHT_SYMTAB:
+	    case SHT_SYMTAB_SHNDX:
+	      break;
+	    default:
+	      if (! bfd_section_from_shdr (abfd, shindex))
+		goto got_no_match;
+	      break;
+	    }
 	  if (shindex == SHN_LORESERVE - 1)
 	    shindex += SHN_HIRESERVE + 1 - SHN_LORESERVE;
 	}

Re: PATCH: SHN_XINDEX support is broken

[Alan Modra]

On Fri, Feb 04, 2005 at 04:24:12PM -0800, H. J. Lu wrote:
> On Fri, Feb 04, 2005 at 03:43:28PM -0800, H. J. Lu wrote:
> > Hi Alan,
> > 
> > SHN_XINDEX support doesn't work. I have a testcase to show it. The
> > problem is that binutils assumes the full symbol table is available
> > before all sections are processed.

HJ, you really ought to describe in a little more detail why you need to
make changes.  Since I know the ELF linker code fairly well, I can guess
that this change is needed to read a group signature symbol.  Correct?

If you have done the work to debug the problem, a description like the
following should be easy to write, and would make review of your patches
simpler.

"SHT_GROUP sections are typically ordered before SHT_SYMTAB and
SHT_SYMTAB_SHNDX in the ELF section headers, and thus are loaded by
elf_object_p before the symbol table sections.  When loading a group
section, bfd_section_from_shdr calls group_signature to read a symbol
name associated with the group.  group_signature ensures that the
SHT_SYMTAB section is loaded, but doesn't load SHT_SYMTAB_SHNDX.  This
can lead to an abort if the symbol st_shndx is SHN_XINDEX."

> > It only works without SHN_XINDEX
> > since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> > affects both bfd and readelf. I will see what I can do.
> > 
> 
> 2005-02-04  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	* elfcode.h (elf_object_p): Read in SHT_SYMTAB and
> 	SHT_SYMTAB_SHNDX sections first.

I don't think this is the best place to fix this problem.  One reason is
that elfcode.h is compiled twice, once for 32-bit support and once for
64-bit, so we should avoid adding code to elfcode.h if at all possible.
Another reason is that bfd_section_from_shdr already handles a number of
similar section dependecies, so that's where I would add code to load
SHT_SYMTAB_SHNDX when handling SHT_SYMTAB.

Like this.  Plus a few cleanups and minor optimizations.  I'll commit in
the morning assuming my overnight tests look good.

	* elf-bfd.h (elf_string_from_elf_strtab): Delete macro.
	* elf.c (bfd_elf_string_from_elf_section): Expand occurrence of
	elf_string_from_elf_strtab.
	(_bfd_elf_setup_group_pointers, bfd_section_from_shdr): Likewise.
	(bfd_section_from_shdr): For SHT_SYMTAB, load SHT_SYMTAB_SHNDX too
	if it exists.  Don't do the reverse for SHT_SYMTAB_SHNDX.  For
	SHT_STRTAB, check whether the strtab is for symtab or dynsymtab by
	looking at cached symtab info first, before iterating over headers.
	For SHT_REL and SHT_RELA, load dynsymtab if needed.
	* elfcode.h (elf_object_p): Don't load section header stringtab
	specially.

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.170
diff -u -p -r1.170 elf-bfd.h
--- bfd/elf-bfd.h	31 Jan 2005 22:53:24 -0000	1.170
+++ bfd/elf-bfd.h	6 Feb 2005 13:16:46 -0000
@@ -1378,10 +1378,6 @@ extern bfd_boolean _bfd_elf_print_privat
 extern void bfd_elf_print_symbol
   (bfd *, void *, asymbol *, bfd_print_symbol_type);
 
-#define elf_string_from_elf_strtab(abfd, strindex) \
-  bfd_elf_string_from_elf_section (abfd, elf_elfheader(abfd)->e_shstrndx, \
-				   strindex)
-
 extern void _bfd_elf_sprintf_vma
   (bfd *, char *, bfd_vma);
 extern void _bfd_elf_fprintf_vma
Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.263
diff -u -p -r1.263 elf.c
--- bfd/elf.c	31 Jan 2005 23:13:18 -0000	1.263
+++ bfd/elf.c	6 Feb 2005 13:16:47 -0000
@@ -291,13 +291,13 @@ bfd_elf_string_from_elf_section (bfd *ab
 
   if (strindex >= hdr->sh_size)
     {
+      unsigned int shstrndx = elf_elfheader(abfd)->e_shstrndx;
       (*_bfd_error_handler)
 	(_("%B: invalid string offset %u >= %lu for section `%s'"),
 	 abfd, strindex, (unsigned long) hdr->sh_size,
-	 ((shindex == elf_elfheader(abfd)->e_shstrndx
-	   && strindex == hdr->sh_name)
+	 (shindex == shstrndx && strindex == hdr->sh_name
 	  ? ".shstrtab"
-	  : elf_string_from_elf_strtab (abfd, hdr->sh_name)));
+	  : bfd_elf_string_from_elf_section (abfd, shstrndx, hdr->sh_name)));
       return "";
     }
 
@@ -650,7 +650,10 @@ _bfd_elf_setup_group_pointers (bfd *abfd
 	      (_("%B: unknown [%d] section `%s' in group [%s]"),
 	       abfd,
 	       (unsigned int) idx->shdr->sh_type,
-	       elf_string_from_elf_strtab (abfd, idx->shdr->sh_name),
+	       bfd_elf_string_from_elf_section (abfd,
+						(elf_elfheader (abfd)
+						 ->e_shstrndx),
+						idx->shdr->sh_name),
 	       shdr->bfd_section->name);
 	    result = FALSE;
 	  }
@@ -1706,7 +1709,9 @@ bfd_section_from_shdr (bfd *abfd, unsign
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
   const char *name;
 
-  name = elf_string_from_elf_strtab (abfd, hdr->sh_name);
+  name = bfd_elf_string_from_elf_section (abfd,
+					  elf_elfheader (abfd)->e_shstrndx,
+					  hdr->sh_name);
 
   switch (hdr->sh_type)
     {
@@ -1779,6 +1784,32 @@ bfd_section_from_shdr (bfd *abfd, unsign
 	  && ! _bfd_elf_make_section_from_shdr (abfd, hdr, name))
 	return FALSE;
 
+      /* Go looking for SHT_SYMTAB_SHNDX too, since if there is one we
+	 can't read symbols without that section loaded as well.  It
+	 is most likely specified by the next section header.  */
+      if (elf_elfsections (abfd)[elf_symtab_shndx (abfd)]->sh_link != shindex)
+	{
+	  unsigned int i, num_sec;
+
+	  num_sec = elf_numsections (abfd);
+	  for (i = shindex + 1; i < num_sec; i++)
+	    {
+	      Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+	      if (hdr2->sh_type == SHT_SYMTAB_SHNDX
+		  && hdr2->sh_link == shindex)
+		break;
+	    }
+	  if (i == num_sec)
+	    for (i = 1; i < shindex; i++)
+	      {
+		Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+		if (hdr2->sh_type == SHT_SYMTAB_SHNDX
+		    && hdr2->sh_link == shindex)
+		  break;
+	      }
+	  if (i != shindex)
+	    return bfd_section_from_shdr (abfd, i);
+	}
       return TRUE;
 
     case SHT_DYNSYM:		/* A dynamic symbol table */
@@ -1800,11 +1831,7 @@ bfd_section_from_shdr (bfd *abfd, unsign
       if (elf_symtab_shndx (abfd) == shindex)
 	return TRUE;
 
-      /* Get the associated symbol table.  */
-      if (! bfd_section_from_shdr (abfd, hdr->sh_link)
-	  || hdr->sh_link != elf_onesymtab (abfd))
-	return FALSE;
-
+      BFD_ASSERT (elf_symtab_shndx (abfd) == 0);
       elf_symtab_shndx (abfd) = shindex;
       elf_tdata (abfd)->symtab_shndx_hdr = *hdr;
       elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->symtab_shndx_hdr;
@@ -1819,37 +1846,46 @@ bfd_section_from_shdr (bfd *abfd, unsign
 	  elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->shstrtab_hdr;
 	  return TRUE;
 	}
-      {
-	unsigned int i, num_sec;
+      if (elf_elfsections (abfd)[elf_onesymtab (abfd)]->sh_link == shindex)
+	{
+	symtab_strtab:
+	  elf_tdata (abfd)->strtab_hdr = *hdr;
+	  elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->strtab_hdr;
+	  return TRUE;
+	}
+      if (elf_elfsections (abfd)[elf_dynsymtab (abfd)]->sh_link == shindex)
+	{
+	dynsymtab_strtab:
+	  elf_tdata (abfd)->dynstrtab_hdr = *hdr;
+	  hdr = &elf_tdata (abfd)->dynstrtab_hdr;
+	  elf_elfsections (abfd)[shindex] = hdr;
+	  /* We also treat this as a regular section, so that objcopy
+	     can handle it.  */
+	  return _bfd_elf_make_section_from_shdr (abfd, hdr, name);
+	}
 
-	num_sec = elf_numsections (abfd);
-	for (i = 1; i < num_sec; i++)
-	  {
-	    Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
-	    if (hdr2->sh_link == shindex)
-	      {
-		if (! bfd_section_from_shdr (abfd, i))
-		  return FALSE;
-		if (elf_onesymtab (abfd) == i)
-		  {
-		    elf_tdata (abfd)->strtab_hdr = *hdr;
-		    elf_elfsections (abfd)[shindex] =
-		      &elf_tdata (abfd)->strtab_hdr;
-		    return TRUE;
-		  }
-		if (elf_dynsymtab (abfd) == i)
-		  {
-		    elf_tdata (abfd)->dynstrtab_hdr = *hdr;
-		    elf_elfsections (abfd)[shindex] = hdr =
-		      &elf_tdata (abfd)->dynstrtab_hdr;
-		    /* We also treat this as a regular section, so
-		       that objcopy can handle it.  */
-		    break;
-		  }
-	      }
-	  }
-      }
+      /* If the string table isn't one of the above, then treat it as a
+	 regular section.  We need to scan all the headers to be sure,
+	 just in case this strtab section appeared before the above.  */
+      if (elf_onesymtab (abfd) == 0 || elf_dynsymtab (abfd) == 0)
+	{
+	  unsigned int i, num_sec;
 
+	  num_sec = elf_numsections (abfd);
+	  for (i = 1; i < num_sec; i++)
+	    {
+	      Elf_Internal_Shdr *hdr2 = elf_elfsections (abfd)[i];
+	      if (hdr2->sh_link == shindex)
+		{
+		  if (! bfd_section_from_shdr (abfd, i))
+		    return FALSE;
+		  if (elf_onesymtab (abfd) == i)
+		    goto symtab_strtab;
+		  if (elf_dynsymtab (abfd) == i)
+		    goto dynsymtab_strtab;
+		}
+	    }
+	}
       return _bfd_elf_make_section_from_shdr (abfd, hdr, name);
 
     case SHT_REL:
@@ -1902,7 +1938,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
 	  }
 
 	/* Get the symbol table.  */
-	if (elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
+	if ((elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
+	     || elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_DYNSYM)
 	    && ! bfd_section_from_shdr (abfd, hdr->sh_link))
 	  return FALSE;
 
@@ -2001,10 +2038,8 @@ bfd_section_from_shdr (bfd *abfd, unsign
 
     default:
       /* Check for any processor-specific section types.  */
-      {
-	if (bed->elf_backend_section_from_shdr)
-	  (*bed->elf_backend_section_from_shdr) (abfd, hdr, name);
-      }
+      if (bed->elf_backend_section_from_shdr)
+	(*bed->elf_backend_section_from_shdr) (abfd, hdr, name);
       break;
     }
 
Index: bfd/elfcode.h
===================================================================
RCS file: /cvs/src/src/bfd/elfcode.h,v
retrieving revision 1.62
diff -u -p -r1.62 elfcode.h
--- bfd/elfcode.h	28 Jan 2005 17:58:24 -0000	1.62
+++ bfd/elfcode.h	6 Feb 2005 13:16:48 -0000
@@ -475,7 +475,6 @@ elf_object_p (bfd *abfd)
   Elf_Internal_Shdr i_shdr;
   Elf_Internal_Shdr *i_shdrp;	/* Section header table, internal form */
   unsigned int shindex;
-  char *shstrtab;		/* Internal copy of section header stringtab */
   const struct elf_backend_data *ebd;
   struct bfd_preserve preserve;
   asection *s;
@@ -686,12 +685,6 @@ elf_object_p (bfd *abfd)
 	}
     }
 
-  if (i_ehdrp->e_shstrndx && i_ehdrp->e_shoff)
-    {
-      if (! bfd_section_from_shdr (abfd, i_ehdrp->e_shstrndx))
-	goto got_no_match;
-    }
-
   /* Read in the program headers.  */
   if (i_ehdrp->e_phnum == 0)
     elf_tdata (abfd)->phdr = NULL;
@@ -717,20 +710,10 @@ elf_object_p (bfd *abfd)
 	}
     }
 
-  /* Read in the string table containing the names of the sections.  We
-     will need the base pointer to this table later.  */
-  /* We read this inline now, so that we don't have to go through
-     bfd_section_from_shdr with it (since this particular strtab is
-     used to find all of the ELF section names.) */
-
-  if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff)
+  if (i_ehdrp->e_shstrndx != 0 && i_ehdrp->e_shoff != 0)
     {
       unsigned int num_sec;
 
-      shstrtab = bfd_elf_get_str_section (abfd, i_ehdrp->e_shstrndx);
-      if (!shstrtab)
-	goto got_no_match;
-
       /* Once all of the section headers have been read and converted, we
 	 can start processing them.  Note that the first section header is
 	 a dummy placeholder entry, so we ignore it.  */


-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: SHN_XINDEX support is broken

[H. J. Lu]

On Sun, Feb 06, 2005 at 11:48:10PM +1030, Alan Modra wrote:
> On Fri, Feb 04, 2005 at 04:24:12PM -0800, H. J. Lu wrote:
> > On Fri, Feb 04, 2005 at 03:43:28PM -0800, H. J. Lu wrote:
> > > Hi Alan,
> > > 
> > > SHN_XINDEX support doesn't work. I have a testcase to show it. The
> > > problem is that binutils assumes the full symbol table is available
> > > before all sections are processed.
> 
> HJ, you really ought to describe in a little more detail why you need to
> make changes.  Since I know the ELF linker code fairly well, I can guess
> that this change is needed to read a group signature symbol.  Correct?
> 
> If you have done the work to debug the problem, a description like the
> following should be easy to write, and would make review of your patches
> simpler.

I will remember that before I came up with a smal testcase next time.
BTW, I couldn't create a small testcase.

> 
> "SHT_GROUP sections are typically ordered before SHT_SYMTAB and
> SHT_SYMTAB_SHNDX in the ELF section headers, and thus are loaded by
> elf_object_p before the symbol table sections.  When loading a group
> section, bfd_section_from_shdr calls group_signature to read a symbol
> name associated with the group.  group_signature ensures that the
> SHT_SYMTAB section is loaded, but doesn't load SHT_SYMTAB_SHNDX.  This
> can lead to an abort if the symbol st_shndx is SHN_XINDEX."

> 
> > > It only works without SHN_XINDEX
> > > since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> > > affects both bfd and readelf. I will see what I can do.
> > > 
> > 
> > 2005-02-04  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> > 	* elfcode.h (elf_object_p): Read in SHT_SYMTAB and
> > 	SHT_SYMTAB_SHNDX sections first.
> 
> I don't think this is the best place to fix this problem.  One reason is
> that elfcode.h is compiled twice, once for 32-bit support and once for
> 64-bit, so we should avoid adding code to elfcode.h if at all possible.
> Another reason is that bfd_section_from_shdr already handles a number of
> similar section dependecies, so that's where I would add code to load
> SHT_SYMTAB_SHNDX when handling SHT_SYMTAB.
> 
> Like this.  Plus a few cleanups and minor optimizations.  I'll commit in
> the morning assuming my overnight tests look good.
> 

It seems to work with my 20MB testcase.

Thanks.

H.J.

Re: PATCH: SHN_XINDEX support is broken

[H. J. Lu]

On Sun, Feb 06, 2005 at 11:48:10PM +1030, Alan Modra wrote:
> 
> "SHT_GROUP sections are typically ordered before SHT_SYMTAB and
> SHT_SYMTAB_SHNDX in the ELF section headers, and thus are loaded by
> elf_object_p before the symbol table sections.  When loading a group
> section, bfd_section_from_shdr calls group_signature to read a symbol
> name associated with the group.  group_signature ensures that the
> SHT_SYMTAB section is loaded, but doesn't load SHT_SYMTAB_SHNDX.  This
> can lead to an abort if the symbol st_shndx is SHN_XINDEX."
> 
> > > It only works without SHN_XINDEX
> > > since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
> > > affects both bfd and readelf. I will see what I can do.
> > > 
> > 
> > 2005-02-04  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> > 	* elfcode.h (elf_object_p): Read in SHT_SYMTAB and
> > 	SHT_SYMTAB_SHNDX sections first.
> 
> I don't think this is the best place to fix this problem.  One reason is
> that elfcode.h is compiled twice, once for 32-bit support and once for
> 64-bit, so we should avoid adding code to elfcode.h if at all possible.
> Another reason is that bfd_section_from_shdr already handles a number of
> similar section dependecies, so that's where I would add code to load
> SHT_SYMTAB_SHNDX when handling SHT_SYMTAB.
> 
> Like this.  Plus a few cleanups and minor optimizations.  I'll commit in
> the morning assuming my overnight tests look good.
> 
> 	* elf-bfd.h (elf_string_from_elf_strtab): Delete macro.
> 	* elf.c (bfd_elf_string_from_elf_section): Expand occurrence of
> 	elf_string_from_elf_strtab.
> 	(_bfd_elf_setup_group_pointers, bfd_section_from_shdr): Likewise.
> 	(bfd_section_from_shdr): For SHT_SYMTAB, load SHT_SYMTAB_SHNDX too
> 	if it exists.  Don't do the reverse for SHT_SYMTAB_SHNDX.  For
> 	SHT_STRTAB, check whether the strtab is for symtab or dynsymtab by
> 	looking at cached symtab info first, before iterating over headers.
> 	For SHT_REL and SHT_RELA, load dynsymtab if needed.
> 	* elfcode.h (elf_object_p): Don't load section header stringtab
> 	specially.
> 

I have been looking at the change. I am not 100% sure if it will be
always correct. Will the symbol table be read in by SHT_REL/SHT_RELA?
What if a file doesn't have them and does have SHT_GROUP? I will try
to come up with a testcase for that.


H.J.

Re: PATCH: SHN_XINDEX support is broken

[Alan Modra]

On Sun, Feb 06, 2005 at 10:18:59AM -0800, H. J. Lu wrote:
> I have been looking at the change. I am not 100% sure if it will be
> always correct. Will the symbol table be read in by SHT_REL/SHT_RELA?

Yes.  elf.c:1940
	/* Get the symbol table.  */
	if ((elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB
	     || elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_DYNSYM)
	    && ! bfd_section_from_shdr (abfd, hdr->sh_link))
	  return FALSE;

> What if a file doesn't have them and does have SHT_GROUP?

group_signature loads the symbol table.

elf.c:450
  /* First we need to ensure the symbol table is available.  */
  if (! bfd_section_from_shdr (abfd, ghdr->sh_link))
    return NULL;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

SHN_XINDEX support is broken

[H. J. Lu]

Hi Alan,

SHN_XINDEX support doesn't work. I have a testcase to show it. The
problem is that binutils assumes the full symbol table is available
before all sections are processed. It only works without SHN_XINDEX
since SHT_REL/SHT_RELA sections will load SHT_SYMTAB. The problem
affects both bfd and readelf. I will see what I can do.


H.J.