Bug 21167

Summary: gas v2.28.51 does not include .rel[a] sections to groups
Product: binutils Reporter: georgerim
Component: gasAssignee: Alan Modra <amodra>
Status: RESOLVED FIXED    
Severity: normal CC: emaste, georgerim, rafael
Priority: P2    
Version: 2.29   
Target Milestone: 2.30   
Host: Target:
Build: Last reconfirmed: 2017-10-03 00:00:00

Description georgerim 2017-02-15 16:13:20 UTC
Assume next code (1.s):

.section .text,"axG",@progbits,foo,comdat
.quad bar

./as-new 1.s -o out
readelf -a out

 [ 5] .text             PROGBITS         0000000000000000  00000048
       0000000000000008  0000000000000000 AXG       0     0     1
 [ 6] .rela.text        RELA             0000000000000000  00000120
       0000000000000018  0000000000000018   I       7     5     8

As shown, .rela.text is present for .text, it has 5 as sh_info field, but gas does not include it into group:
COMDAT group section [    1] `.group' [foo] contains 1 sections:
   [Index]    Name
   [    5]   .text

During the "regular" part of the link, the linker includes or discards groups as a unit.
ELF spec that says: "There may *not* be non-symbol references to the sections comprising a group from outside the group, for example, use of a group member's section header index in an sh_link or sh_info member."

So it looks to be violation of spec.
Comment 1 cvs-commit@gcc.gnu.org 2017-10-04 23:08:55 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit db4677b8bd90b49f826807352c6c3c7eb0d57814
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Oct 4 09:53:53 2017 +1030

    PR21167, relocation sections not included in groups
    
    This fixes a wart I've known about for years, but haven't done
    anything about because BFD treats relocation sections as an adjunct to
    the section they relocate.  SHF_GROUP on the section thus implicitly
    applies to its relocation section(s), but it is an error that the
    reloc sections aren't part of the group.
    
    Like many patches to gas, this wasn't as straightforward as it could
    be due to a number of backends, i386, cr16 and others, removing relocs
    in tc_get_reloc rather than marking them as "done" earlier in
    md_apply_reloc.  So it isn't possible for the group support to
    reliably detect the presence of relocs by looking at fixups earlier
    than write_relocs.  However the group support needs to create
    signature symbols, and that must be done before the symbol table is
    frozen, before write_relocs.  So split off the group sizing from
    elf_adjust_symtab and put it in elf_frob_file_after_relocs.
    
    bfd/
    	PR 21167
    	* elf.c (_bfd_elf_setup_sections): Don't trim reloc sections from
    	groups.
    	(_bfd_elf_init_reloc_shdr): Pass sec_hdr, use it to copy SHF_GROUP
    	flag from section.
    	(elf_fake_sections): Adjust calls.  Exit immediately on failure.
    	(bfd_elf_set_group_contents): Add associated reloc section indices
    	to group contents
    gas/
    	PR 21167
    	* config/obj-elf.c (struct group_list): Delete elt_count.
    	(groups): New static.
    	(build_group_lists): Don't count elements.
    	(elf_adjust_symtab): Use groups rather than auto list.  Set up
    	pointer from group member to SHT_GROUP section.  Don't size
    	SHT_GROUP section or clean up here..
    	(elf_frob_file_after_relocs): ..do so here instead.
    	* testsuite/gas/arc/jli-1.d,
    	* testsuite/gas/elf/groupautob.d,
    	* testsuite/gas/mips/compact-eh-eb-2.d,
    	* testsuite/gas/mips/compact-eh-eb-5.d,
    	* testsuite/gas/mips/compact-eh-el-2.d,
    	* testsuite/gas/mips/compact-eh-el-5.d: Adjust.
    ld/
    	PR 21167
    	* testsuite/ld-elf/group9b.d: Adjust for relocs included in group.
Comment 2 Alan Modra 2017-10-04 23:52:05 UTC
Fixed master, and in my opinion the risk/benefit makes this patch not suitable for backporting to 2.29.
Comment 3 cvs-commit@gcc.gnu.org 2017-10-05 01:07:46 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

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

commit 7d36e2799141d206651410c68080f40b88809a3b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Oct 4 18:01:47 2017 -0700

    Add an assembler test for PR gas/21167
    
    	PR gas/21167
    	* testsuite/gas/elf/elf.exp: Run group3.
    	* testsuite/gas/elf/group3.d: New file.
    	* testsuite/gas/elf/group3.s: Likewise.