Re: PATCH: SHN_XINDEX support is broken

["H. J. Lu" <hjl at lucon dot org>]

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.