Bug 15835 - binutils can't deal with more than one SHT_SYMTAB_SHNDX section (such as from Solaris or illumos link editor)
Summary: binutils can't deal with more than one SHT_SYMTAB_SHNDX section (such as from...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.23
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-13 17:59 UTC by Rich Lowe
Modified: 2015-09-23 16:27 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
test case (2.49 MB, application/octet-stream)
2015-07-16 14:46 UTC, Rich Lowe
Details
Proposed patch+ (4.79 KB, patch)
2015-08-12 15:30 UTC, Nick Clifton
Details | Diff
Alternative patch for the 2.25 sources (4.79 KB, patch)
2015-09-03 13:47 UTC, Nick Clifton
Details | Diff
Extended patch (6.34 KB, patch)
2015-09-21 13:33 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rich Lowe 2013-08-13 17:59:47 UTC
in `bfd_section_from_shdr', in the case for SHT_SYMTAB_SHNDX
there's an assumption (and BFD_ASSERT to back it up)
that there will only be one section of that type per file.  With
dynamic objects created by the Solaris or illumos link editors,
there will generally be 3 such sections, one each for .symtab,
.dynsym and .SUNW_ldynsym.  This causes the code in question to
fail utterly, and seemingly destroy the object (in the binutils
I'm using, BFD_ASSERT seems to be little more than printf, no
abort or exit).

I have a libxul.so with 130,000 sections, when attempting to
modify it to test some changes to our toolchain, I did:

  $ binutils/objcopy --add-section .SUNW_ctf=foo.ctf ../libxul.so libxul-new.so
  BFD: BFD (GNU Binutils) 2.23.2 assertion fail elf.c:1714
  BFD: BFD (GNU Binutils) 2.23.2 assertion fail elf.c:1714

(these are the assertions regarding there not already being a symtab shndx section)

The resulting libxul-new.so is, as you'd expect, trash:
   $ elfdump libxul-new.so 2>&1 >/dev/null | head
   libxul-new.so: .SUNW_ldynsym: invalid sh_link: 0
   libxul-new.so: .dynsym: index[1]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[4]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[8]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[11]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[12]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[13]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[14]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[17]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   libxul-new.so: .dynsym: index[24]: invalid SHN_XINDEX reference: SHT_SYMTAB_SHNDX section not found
   ...
(there are many, many similar such complaints)

A simple test could be constructed using gcc's -ffunction-sections and -fdata-sections and sufficient symbols to trigger extended section indexing.
Comment 1 Nick Clifton 2015-07-16 11:11:32 UTC
Hi Rich,

  Please could you create a test case in way you suggested and upload it here ?

  I do not have access to a Solaris or illumos system at the moment. :-(

Cheers
  Nick
Comment 2 Rich Lowe 2015-07-16 14:46:03 UTC
Created attachment 8443 [details]
test case

Attached, it's a few thousand symbols named fooN and a main just to get it linked, built with -fdata-sections.  You should have SYMTAB_SHNDX sections for .symtab, .dynsym, and .SUNW_ldynsym, which should be enough to trigger whatever problems
Comment 3 Ali Bahrami 2015-07-23 19:29:22 UTC
As additional evidence that binutils/bfd are wrong
in preventing more than one SHT_SYMTAB_SHNDX, I'd like
to reference this discussion in the ELF gABI list where
we just modified the ELF spec to make it clear that
multiple such sections are allowed:

    https://groups.google.com/forum/#!topic/generic-abi/-XJAV5d8PRg

So binutils should be fixed. However, I think it's
also probably true that the Illumos ld, as with the
Solaris ld, needs to be updated to understand the
special section merging rules that the GNU linkers use
to remerge the sections that result from the use of
-ffunction-sections. Extended sections really shouldn't
occur in an executable or shared library.
Comment 4 Nick Clifton 2015-08-12 15:30:54 UTC
Created attachment 8510 [details]
Proposed patch+
Comment 5 Nick Clifton 2015-08-12 15:31:53 UTC
Hi Rich,

  Please could you try out the uploaded patch and let me know if it works for you.  It should allow the binutils to read files with multiple symtab_shndx sections, but it will not make the linker produce them.

Cheers
  Nick
Comment 6 Richard PALO 2015-08-27 06:01:10 UTC
(In reply to Nick Clifton from comment #5)
> Hi Rich,
> 
>   Please could you try out the uploaded patch and let me know if it works
> for you.  It should allow the binutils to read files with multiple
> symtab_shndx sections, but it will not make the linker produce them.
> 
> Cheers
>   Nick

following this issue, tried the patch on binutils 2.25.1, unfortunately:

...
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -DCP_ACP=1 -I. -I. -I./../include -DHAVE_i386_elf32_sol2_vec -DHAVE_i386_coff_vec -DHAVE_x86_64_elf64_sol2_vec -DHAVE_l1om_elf64_vec -DHAVE_k1om_elf64_vec -DHAVE_elf64_le_vec -DHAVE_elf64_be_vec -DHAVE_elf32_le_vec -DHAVE_elf32_be_vec -DHAVE_plugin_vec -DBINDIR=\"/opt/local/bin\" -I/opt/local/include -I/usr/include -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -O2 -I/opt/local/include -I/usr/include -MT elf.lo -MD -MP -MF .deps/elf.Tpo -c elf.c -o elf.o
elf.c: In function 'assign_file_positions_for_non_load_sections':
elf.c:5075:9: warning: implicit declaration of function 'elf_symtab_shndx' [-Wimplicit-function-declaration]
         || hdr == i_shdrpp[elf_symtab_shndx (abfd)]
         ^
elf.c: In function 'assign_file_positions_except_relocs':
elf.c:5333:13: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
        || i == elf_symtab_shndx (abfd)
             ^
elf.c: In function 'swap_out_syms':
elf.c:6909:39: error: 'struct elf_obj_tdata' has no member named 'symtab_shndx_hdr'
   symtab_shndx_hdr = &elf_tdata (abfd)->symtab_shndx_hdr;
                                       ^
Comment 7 Nick Clifton 2015-09-03 13:47:13 UTC
Created attachment 8575 [details]
Alternative patch for the 2.25 sources
Comment 8 Nick Clifton 2015-09-03 13:48:51 UTC
Hi Richard,

> following this issue, tried the patch on binutils 2.25.1, unfortunately:

Sorry about that.  The patch was made against the mainline development sources, not the 2.25 branch.  I have uploaded a variant of the patch which should apply to the 2.25 sources.  Please could you try it out and let me know how you get on.

Cheers
  Nick
Comment 9 Richard PALO 2015-09-07 16:37:34 UTC
For a very simpistic test, I have an existing thunderbird libxul.so which, with which the currently installed binutils readelf gives:
>richard@omnis:/tmp/pkgsrc/devel/binutils/work/binutils-2.25.1/binutils$ greadelf -s /opt/local/lib/thunderbird24/libxul.so >/dev/null
>readelf: Error: File contains multiple symtab shndx tables
>readelf: Error: File contains multiple symtab shndx tables

With that patched binutils, I don't get the same error:
>richard@omnis:/tmp/pkgsrc/devel/binutils/work/binutils-2.25.1/binutils$ ./readelf -s /opt/local/lib/thunderbird24/libxul.so >/dev/null
>richard@omnis:/tmp/pkgsrc/devel/binutils/work/binutils-2.25.1/binutils$

I'll have to leave to Rich to eventually test specific section handling.

cheers
Comment 10 Richard PALO 2015-09-08 02:57:19 UTC
I did try two things in addition...
First, and again with the existing libxul.so:
>$ gobjdump -T /opt/local/lib/thunderbird24/libxul.so 
>
>/opt/local/lib/thunderbird24/libxul.so:     file format elf32-i386-sol2
>
>gobjdump: /opt/local/lib/thunderbird24/libxul.so symbol number 1 references nonexistent SHT_SYMTAB_SHNDX section
>gobjdump: /opt/local/lib/thunderbird24/libxul.so: File in wrong format


then I rebuilt with the patched binutils in place and tried with the new libxul.so:
>$ gobjdump -T /tmp/pkgsrc/mail/thunderbird24/work/comm-esr24/mozilla/toolkit/library/libxul.so
>
>/tmp/pkgsrc/mail/thunderbird24/work/comm-esr24/mozilla/toolkit/library/libxul.so:     file format elf32-i386-sol2
>
>gobjdump: /tmp/pkgsrc/mail/thunderbird24/work/comm-esr24/mozilla/toolkit/library/libxul.so symbol number 1 references nonexistent SHT_SYMTAB_SHNDX section
>gobjdump: /tmp/pkgsrc/mail/thunderbird24/work/comm-esr24/mozilla/toolkit/library/libxul.so: File in wrong format


no change, so perhaps there is still an issue, either with binutils, illumos ld or both.

I can provide both libxul.so, but I note that elfdump output for both are equivalent.
Comment 11 Nick Clifton 2015-09-08 16:17:37 UTC
Hi Richard,

(In reply to Richard PALO from comment #10)
> I can provide both libxul.so,

yes please - that would definitely help.

Cheers
  Nick
Comment 12 Richard PALO 2015-09-08 17:19:54 UTC
they are quite large:
> $ gls -sh pr15835.libxuls.tar.bz2 
> 676M pr15835.libxuls.tar.bz2

can I copy them somewhere in particular?
I could also do one at a time which is half the size.
Comment 13 Nick Clifton 2015-09-09 07:40:29 UTC
Hi Richard,

> they are quite large:
> > $ gls -sh pr15835.libxuls.tar.bz2 
> > 676M pr15835.libxuls.tar.bz2

xz compression might make them a little bit smaller, but probably not enough.

> can I copy them somewhere in particular?

Sure, put them on a could file server somewhere if you like.

> I could also do one at a time which is half the size.

That would be fine too.

Cheers
  Nick
Comment 14 Richard PALO 2015-09-17 13:45:47 UTC
sorry, my habitual large file site no longer likes FF24...

this link will only be valid for about a week: http://tnow.it/82kx31b6uncf
Comment 15 Nick Clifton 2015-09-21 13:33:20 UTC
Created attachment 8625 [details]
Extended patch

Hi Richard,

  Please try out this extended patch, which should work with the libuxl.so files.

Cheers
  Nick
Comment 16 Richard PALO 2015-09-21 14:08:20 UTC
Comment on attachment 8625 [details]
Extended patch

are all these sim/* bits applicable or by accident they are included?
Comment 17 Nick Clifton 2015-09-21 15:02:42 UTC
Hi Richard,

> are all these sim/* bits applicable or by accident they are included?

Accident - sorry !

Also there is a bug in the patch to the bfd/elf32-rl78.c file, (fixed 
locally), but this will only affect you if try to build for the RL78 target.

Cheers
   Nick
Comment 18 Richard PALO 2015-09-22 06:00:37 UTC
great, both greadelf and gobjdump seem fine with libxul.so now
Comment 19 cvs-commit@gcc.gnu.org 2015-09-23 16:25:36 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6a40cf0c5c845683fdb82721813ebd5dd867cce5

commit 6a40cf0c5c845683fdb82721813ebd5dd867cce5
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Sep 23 17:23:58 2015 +0100

    Add support for files that contain multiple symbol index tables.  Fixes PR 15835
    
    binutils	PR binutils/15835
    	* readelf.c (struct elf_section_list): New structure.
    	(symtab_shndx_hdr): Replace with symtab_shndx_list.
    	(get_32bit_elf_symbols): Scan for a symbol index table matching
    	the symbol table in use.
    	(get_64bit_elf_symbols): Likewise.
    	(process_section_headers): Handle multiple symbol index sections.
    
    bfd	* elf-bfd.h (struct elf_section_list): New structure.
    	(struct elf_obj_tdata): Replace symtab_shndx_hdr with
    	symtab_shndx_list.  Delete symtab_shndx_section.
    	(elf_symtab_shndx): Replace macro with elf_symtab_shndx_list.
    	* elf.c (bfd_elf_get_syms): If symtab index sections are present,
    	scan them for the section that matches the provided symbol table.
    	(bfd_section_from_shdr): Record all SHT_SYMTAB_SHNDX sections.
    	(assign_section_numbers): Use the first symtab index table in the
    	list.
    	(_bfd_elf_compute_section_file_positions): Replace use of
    	symtab_shndx_hdr with use of symtab_shndx_list.
    	(find_section_in_list): New function.
    	(assign_file_postions_except_relocs): Use new function.
    	(_bfd_elf_copy_private_symbol_data): Likewise.
    	(swap_out_syms): Handle multiple symbol table index sections.
    	* elf32-m32c.c (m32c_elf_relax_section): Replace use of
    	symtab_shndx_hdr with use of symtab_shndx_list.
    	* elf32-rl78.c (rl78_elf_relax_section): Likewise.
    	* elf32-rx.c (rx_relax_section): Likewise.
    	* elf32-v850.c (v850_elf_relax_delete_bytes): Likewise.
    	* elflink.c (bfd_elf_final_link): Likewise.
Comment 20 Nick Clifton 2015-09-23 16:27:01 UTC
Patch checked in.

Cheers
  Nick

bfd/ChangeLog
2015-09-23  Nick Clifton  <nickc@redhat.com>

	PR binutils/15835
	* elf-bfd.h (struct elf_section_list): New structure.
	(struct elf_obj_tdata): Replace symtab_shndx_hdr with
	symtab_shndx_list.  Delete symtab_shndx_section.
	(elf_symtab_shndx): Replace macro with elf_symtab_shndx_list.
	* elf.c (bfd_elf_get_syms): If symtab index sections are present,
	scan them for the section that matches the provided symbol table.
	(bfd_section_from_shdr): Record all SHT_SYMTAB_SHNDX sections.
	(assign_section_numbers): Use the first symtab index table in the
	list.
	(_bfd_elf_compute_section_file_positions): Replace use of
	symtab_shndx_hdr with use of symtab_shndx_list.
	(find_section_in_list): New function.
	(assign_file_postions_except_relocs): Use new function.
	(_bfd_elf_copy_private_symbol_data): Likewise.
	(swap_out_syms): Handle multiple symbol table index sections.
	* elf32-m32c.c (m32c_elf_relax_section): Replace use of
	symtab_shndx_hdr with use of symtab_shndx_list.
	* elf32-rl78.c (rl78_elf_relax_section): Likewise.
	* elf32-rx.c (rx_relax_section): Likewise.
	* elf32-v850.c (v850_elf_relax_delete_bytes): Likewise.
	* elflink.c (bfd_elf_final_link): Likewise.

binutils/ChangeLog
2015-09-23  Nick Clifton  <nickc@redhat.com>

	PR binutils/15835
	* readelf.c (struct elf_section_list): New structure.
	(symtab_shndx_hdr): Replace with symtab_shndx_list.
	(get_32bit_elf_symbols): Scan for a symbol index table matching
	the symbol table in use.
	(get_64bit_elf_symbols): Likewise.
	(process_section_headers): Handle multiple symbol index sections.