Bug 12942 - Plugin not handling correctly resolution of COMDATs.
Summary: Plugin not handling correctly resolution of COMDATs.
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.22
: P2 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-27 18:08 UTC by Jan Hubicka
Modified: 2014-10-28 06:44 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Hubicka 2011-06-27 18:08:20 UTC
Hi,
you need the following fix to GCC to output symbol table correctly. I will commit it after testing.
Index: ipa.c
===================================================================
--- ipa.c       (revision 175350)
+++ ipa.c       (working copy)
@@ -542,13 +542,13 @@
 {
   int i;
   struct ipa_ref *ref;
-  for (i = 0; ipa_ref_list_reference_iterate (&node->ref_list, i, ref); i++)
+  for (i = 0; ipa_ref_list_refering_iterate (&node->ref_list, i, ref); i++)
     if (ref->use == IPA_REF_ADDR)
       {
        struct varpool_node *node;
-       if (ref->refered_type == IPA_REF_CGRAPH)
+       if (ref->refering_type == IPA_REF_CGRAPH)
          return true;
-       node = ipa_ref_varpool_node (ref);
+       node = ipa_ref_refering_varpool_node (ref);
        if (!DECL_VIRTUAL_P (node->decl))
          return true;
       }

With fixed GCC the following testcase fails with GNU LD but not with Gold:
evans:/abuild/jh/dealIIb/:[0]# cat a.h
inline int test (void)
{
}

typedef int (*test_t) (void);
evans:/abuild/jh/dealIIb/:[0]# cat a.cc

#include "a.h"
extern "C" void abort ();

test_t b(void);

int
main(void)
{
  if (test != b ())
    abort ();
}
evans:/abuild/jh/dealIIb/:[0]# cat b.cc
#include <stdio.h>
#include "a.h"

test_t b(void)
{
  return test;
}
evans:/abuild/jh/dealIIb/:[0]# /abuild/jh/trunk-install/bin/g++ a.cc -flto -c -O2
evans:/abuild/jh/dealIIb/:[0]# /abuild/jh/trunk-install/bin/g++ b.cc  -c -O2
evans:/abuild/jh/dealIIb/:[0]# /abuild/jh/trunk-install/bin/g++ a.o b.o  -save-temps -O2 -flto -fuse-linker-plugin          
[Leaving LTRANS /tmp/cc7tSz5y.args]
[Leaving LTRANS /tmp/ccAl4bQf.ltrans.out]
[Leaving LTRANS /tmp/cc0AoaEb.args]
[Leaving LTRANS /tmp/ccAl4bQf.ltrans0.o]
evans:/abuild/jh/dealIIb/:[0]# ./a.out
Aborted (core dumped)
evans:/abuild/jh/dealIIb/:[134]# /abuild/jh/trunk-install/bin/g++ a.o b.o  -save-temps -O2                           
evans:/abuild/jh/dealIIb/:[0]# ./a.out
evans:/abuild/jh/dealIIb/:[0]# more ./-lm.res 
1
a.o 3
164 623d5db5 PREVAILING_DEF_IRONLY _Z4testv
170 623d5db5 PREVAILING_DEF main
181 623d5db5 RESOLVED_EXEC _Z1bv

I.e. with LTO compilation the _Z4testv is not merged. Golds variant of -lm.res is:

1
a.o 3
164 623d5db5 PREVAILING_DEF _Z4testv
170 623d5db5 PREVAILING_DEF main
181 623d5db5 RESOLVED_EXEC _Z1bv
Comment 1 H.J. Lu 2011-06-27 22:13:52 UTC
Works for me with the current bfd linker:

[hjl@gnu-6 pr12942]$ make
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -O2 -flto    -c -o a.o a.cc
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -O2   -c -o b.o b.cc
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -O2 -flto -fuse-linker-plugin -o x a.o b.o
./x
[hjl@gnu-6 pr12942]$ /export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -O2 -fuse-linker-plugin -flto=jobserver -o x a.o b.o --save-temps
[Leaving LTRANS /tmp/ccyenCIi.args]
[Leaving LTRANS x.ltrans.out]
[Leaving LTRANS /tmp/ccLMrqHc.mk]
[Leaving LTRANS x.ltrans0.o]
[hjl@gnu-6 pr12942]$ cat  ./-lm.res 
1
a.o 3
164 18e9d954 PREEMPTED_REG _Z4testv
170 18e9d954 PREVAILING_DEF main
181 18e9d954 RESOLVED_EXEC _Z1bv
[hjl@gnu-6 pr12942]$
Comment 2 H.J. Lu 2011-06-28 17:29:31 UTC
Here is the real testcase:

[hjl@gnu-6 pr12942]$ cat a.h 
extern void link_error ();

inline int test (void)
{
  int exp = -1;
  if ((exp < 2 ? 2U : (unsigned int) exp) != 2)
    link_error ();

  return 0;
}

typedef int (*test_t) (void);
[hjl@gnu-6 pr12942]$ cat a.cc
#include "a.h"
extern "C" void abort ();

test_t b(void);

int
main(void)
{
  if (test != b ())
    abort ();
}
[hjl@gnu-6 pr12942]$ cat b.cc
#include <stdio.h>
#include "a.h"

test_t b(void)
{
  return test;
}
[hjl@gnu-6 pr12942]$ make
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -flto    -c -o a.o a.cc
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++    -c -o b.o b.cc
/export/build/gnu/gcc/build-x86_64-linux/gcc/../../release/usr/gcc-4.7.0/bin/g++ -O2 -flto -fuse-linker-plugin -B./ -o x a.o b.o
b.o: In function `test()':
b.cc:(.text._Z4testv[test()]+0x16): undefined reference to `link_error()'
collect2: error: ld returned 1 exit status
make: *** [x] Error 1
[hjl@gnu-6 pr12942]$ 

Gold works fine.  The problem is BFD linker failed to pick the
correct definition.
Comment 3 H.J. Lu 2011-06-29 01:13:39 UTC
There are at least 2 problems:

1. plugin totally ignores comdat_key.
2. section for plugin symbol with comdat_key should override other sections
with the same comdat_key.
Comment 4 cvs-commit@gcc.gnu.org 2011-07-09 06:20:56 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2011-07-09 06:20:52

Modified files:
	bfd            : ChangeLog elflink.c cofflink.c elf-bfd.h 
	                 libbfd-in.h libbfd.h linker.c targets.c 
	                 bfd-in2.h 
	include        : ChangeLog bfdlink.h 
	ld             : ChangeLog ldlang.c plugin.c 

Log message:
	PR ld/12942
	bfd/
	* elflink.c (elf_link_add_object_symbols): Use elf_discarded_section
	rather than kept_section to determine whether a symbol is from
	a discarded section.
	* cofflink.c (coff_link_add_symbols): Make symbols from discarded
	sections appear undefined.
	
	* elf-bfd.h (_bfd_elf_section_already_linked): Replace
	"asection *" with "struct already_linked *".
	* libbfd-in.h (_bfd_nolink_section_already_linked): Likewise.
	(_bfd_generic_section_already_linked): Likewise.
	(bfd_section_already_linked_table_insert): Likewise.
	(struct already_linked): New.
	(struct bfd_section_already_linked): Use it.
	* elflink.c (_bfd_elf_section_already_linked): Replace.
	"asection *" with "struct already_linked *".  Replace the plugin
	dummy with the LTO output.
	* linker.c (_bfd_generic_section_already_linked): Likewise.
	* targets.c (struct already_linked): Add forward declaration.
	(bfd_target): Replace "struct bfd_section *" with
	"struct already_linked *" in _section_already_linked.
	* bfd-in2.h: Regenerate.
	* libbfd.h: Regenerate.
	
	include/
	* bfdlink.h (bfd_link_info): Add loading_lto_outputs.
	
	ld/
	* ldlang.c (section_already_linked): Pass "struct already_linked *"
	to bfd_section_already_linked.
	(lang_process): Set link_info.loading_lto_outputs before
	loading LTO outputs.
	* plugin.c: Include "libbfd.h".
	(add_symbols): Call bfd_section_already_linked with comdat_key.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5417&r2=1.5418
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elflink.c.diff?cvsroot=src&r1=1.412&r2=1.413
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/cofflink.c.diff?cvsroot=src&r1=1.77&r2=1.78
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf-bfd.h.diff?cvsroot=src&r1=1.323&r2=1.324
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/libbfd-in.h.diff?cvsroot=src&r1=1.88&r2=1.89
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/libbfd.h.diff?cvsroot=src&r1=1.253&r2=1.254
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/linker.c.diff?cvsroot=src&r1=1.85&r2=1.86
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/targets.c.diff?cvsroot=src&r1=1.202&r2=1.203
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/bfd-in2.h.diff?cvsroot=src&r1=1.540&r2=1.541
http://sourceware.org/cgi-bin/cvsweb.cgi/src/include/ChangeLog.diff?cvsroot=src&r1=1.541&r2=1.542
http://sourceware.org/cgi-bin/cvsweb.cgi/src/include/bfdlink.h.diff?cvsroot=src&r1=1.88&r2=1.89
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ChangeLog.diff?cvsroot=src&r1=1.2350&r2=1.2351
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ldlang.c.diff?cvsroot=src&r1=1.372&r2=1.373
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/plugin.c.diff?cvsroot=src&r1=1.35&r2=1.36
Comment 5 H.J. Lu 2012-12-07 20:16:38 UTC
Works in 2.23.
Comment 6 cvs-commit@gcc.gnu.org 2013-04-03 11:44:43 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2013-04-03 11:44:41

Modified files:
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-plugin: lto.exp pr12942a.cc 

Log message:
	PR ld/15227
	* ld-plugin/lto.exp (PR ld/12942 (3)): Remove file name and
	line number from regexp.
	(PR ld/15146 (2)): Similarly.
	* ld-plugin/pr12942a.cc (main): Use __builtin_abort.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1704&r2=1.1705
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-plugin/lto.exp.diff?cvsroot=src&r1=1.7&r2=1.8
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-plugin/pr12942a.cc.diff?cvsroot=src&r1=1.1&r2=1.2
Comment 7 cvs-commit@gcc.gnu.org 2014-10-18 12:44:11 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  b53dfeb26ed06e97fff1e8f469e33637ebdf6624 (commit)
      from  a841bdf5d3bfb9c687f938be7388597db2e461de (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

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

commit b53dfeb26ed06e97fff1e8f469e33637ebdf6624
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Oct 18 21:46:48 2014 +1030

    PowerPC64 ELFv1 function symbol definition vs LTO and discarded sections
    
    When functions are emitted in comdat groups, global symbols defined in
    duplicates of the group are treated as if they were undefined.  That
    prevents the symbols in the discarded sections from affecting the
    linker's global symbol hash table or causing duplicate symbol errors.
    Annoyingly, when gcc emits a function to a comdat group, it does not
    put *all* of a function's code and data in the comdat group.
    Typically, constant tables, exception handling info, and debug info
    are emitted to normal sections outside of the group, which is a
    perennial source of linker problems due to the special handling needed
    to deal with the extra-group pieces that ought to be discarded.  In
    the case of powerpc64-gcc, the OPD entry for a function is not put in
    the group.  Since the function symbol is defined on the OPD entry this
    means we need to handle symbols in .opd specially.
    
    To see how this affects LTO in particular, consider the linker
    testcase PR ld/12942 (1).  This testcase links an LTO object file
    pr12942a.o with a normal (non-LTO) object pr12942b.o.  Both objects
    contain a definition for _Z4testv in a comdat group.  On loading
    pr12942a.o, the linker sees a comdat group (actually linkonce section)
    for _Z4testv and a weak _Z4testv defined in the IR.  On loading
    pr12942b.o, the linker sees the same comdat group, and thus discards
    it.  However, _Z4testv is a weak symbol defined in .opd, not part of
    the group, so this weak symbol overrides the weak IR symbol.  On
    (re)loading the LTO version of pr12942a.o, the linker sees another
    weak _Z4testv, but this one does not override the value we have from
    pr12942b.o.  The result is a linker complaint about "`_Z4testv'
    ... defined in discarded section `.group' of tmpdir/pr12942b.o".
    
    	* elf64-ppc.c (ppc64_elf_add_symbol_hook): If function code
    	section for function symbols defined in .opd is discarded, let
    	the symbol appear to be undefined.
    	(opd_entry_value): Ensure the result section is that for the
    	function code section in the same object as the OPD entry.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog   |    8 ++++
 bfd/elf64-ppc.c |  105 ++++++++++++++++++++++++++++++++----------------------
 2 files changed, 70 insertions(+), 43 deletions(-)
Comment 8 cvs-commit@gcc.gnu.org 2014-10-28 06:44:39 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, binutils-2_25-branch has been updated
       via  a809b386e59dfcb3f4dedd8465975dabc55db5db (commit)
       via  2609fe3d3f84ce62b373daa56ba615465c2d5293 (commit)
       via  334efb9a97018d8b2f508e5c22e5acb7b824cc76 (commit)
       via  35ac6b6a54f9be85dd9794fa6645fac3631e510b (commit)
      from  7cc2a6b6a4c63cba363836475052a71e0387e621 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

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

commit a809b386e59dfcb3f4dedd8465975dabc55db5db
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Oct 21 20:44:38 2014 +1030

    Relax ppc64_elf_tls_optimize assertion
    
    The code in ppc64_elf_tls_optimize looking at the .toc is only
    interested in .toc entries that are addresses.  .toc can contain more
    than just an array of addresses, so if we have items that aren't
    8-byte aligned, ignore them.
    
    	* elf64-ppc.c (ppc64_elf_tls_optimize): Ignore relocs against toc
    	entries that aren't a multiple of 8 rather than failing assertion.

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

commit 2609fe3d3f84ce62b373daa56ba615465c2d5293
Author: Jan Beulich <jbeulich@novell.com>
Date:   Tue Oct 21 09:56:38 2014 +0200

    ppc: enable msgclr and msgsnd on Power8
    
    According to my reading of the spec it was an oversight for them to
    not having got enabled when Power8 support got added.

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

commit 334efb9a97018d8b2f508e5c22e5acb7b824cc76
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Oct 21 17:06:01 2014 +1030

    Add gcc-4.9 libgomp symbols requiring --plt-thread-safe for power7
    
    powerpc64 ld builds plt call stubs with a read barrier to provide
    thread safety on lazy plt updates, necessary on multi-threaded apps
    with power7 or later weakly ordered memory.  gcc-4.9 libgomp
    introduced more functions that could call pthread_create, which means
    we have more functions that if referenced in an executable should
    cause a default of --plt-thread-safe.
    
    bfd/
    	* elf64-ppc.c (ppc64_elf_size_stubs): Add gcc-4.9 libgomp functions
    	to thread_starter.
    gold/
    	* powerpc.cc (do_relax): Add gcc-4.9 libgomp functions to
    	thread_starter.

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

commit 35ac6b6a54f9be85dd9794fa6645fac3631e510b
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Oct 18 21:46:48 2014 +1030

    PowerPC64 ELFv1 function symbol definition vs LTO and discarded sections
    
    When functions are emitted in comdat groups, global symbols defined in
    duplicates of the group are treated as if they were undefined.  That
    prevents the symbols in the discarded sections from affecting the
    linker's global symbol hash table or causing duplicate symbol errors.
    Annoyingly, when gcc emits a function to a comdat group, it does not
    put *all* of a function's code and data in the comdat group.
    Typically, constant tables, exception handling info, and debug info
    are emitted to normal sections outside of the group, which is a
    perennial source of linker problems due to the special handling needed
    to deal with the extra-group pieces that ought to be discarded.  In
    the case of powerpc64-gcc, the OPD entry for a function is not put in
    the group.  Since the function symbol is defined on the OPD entry this
    means we need to handle symbols in .opd specially.
    
    To see how this affects LTO in particular, consider the linker
    testcase PR ld/12942 (1).  This testcase links an LTO object file
    pr12942a.o with a normal (non-LTO) object pr12942b.o.  Both objects
    contain a definition for _Z4testv in a comdat group.  On loading
    pr12942a.o, the linker sees a comdat group (actually linkonce section)
    for _Z4testv and a weak _Z4testv defined in the IR.  On loading
    pr12942b.o, the linker sees the same comdat group, and thus discards
    it.  However, _Z4testv is a weak symbol defined in .opd, not part of
    the group, so this weak symbol overrides the weak IR symbol.  On
    (re)loading the LTO version of pr12942a.o, the linker sees another
    weak _Z4testv, but this one does not override the value we have from
    pr12942b.o.  The result is a linker complaint about "`_Z4testv'
    ... defined in discarded section `.group' of tmpdir/pr12942b.o".
    
    	* elf64-ppc.c (ppc64_elf_add_symbol_hook): If function code
    	section for function symbols defined in .opd is discarded, let
    	the symbol appear to be undefined.
    	(opd_entry_value): Ensure the result section is that for the
    	function code section in the same object as the OPD entry.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog                  |   15 +++++
 bfd/elf64-ppc.c                |  116 +++++++++++++++++++++++++---------------
 gas/testsuite/ChangeLog        |    4 ++
 gas/testsuite/gas/ppc/power8.d |    2 +
 gas/testsuite/gas/ppc/power8.s |    2 +
 gold/ChangeLog                 |    7 +++
 gold/powerpc.cc                |    6 ++
 opcodes/ChangeLog              |    6 ++
 opcodes/ppc-opc.c              |    4 +-
 9 files changed, 116 insertions(+), 46 deletions(-)