Bug 12772 - linker failed to remove all dead code dependency properly
Summary: linker failed to remove all dead code dependency properly
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.24
: P2 normal
Target Milestone: ---
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-17 13:25 UTC by Michael Matz
Modified: 2017-01-18 18:47 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 Michael Matz 2011-05-17 13:25:06 UTC
(this got initially reported into Novell bugzilla #694266).
Compile this with the following compile command on x86_64-linux:

# g++ -O3 -Wl,-gc-sections -fpic -shared \
-fdata-sections -ffunction-sections \
-fvisibility-inlines-hidden -fvisibility=hidden \
-o test.so test.cpp

# cat test.cpp
#include <string.h>
#include <stdlib.h>
#include <unistd.h>

extern "C" int __attribute__((visibility("default"))) func(char *ptr)
{
    static const char g_const[] = { 1, 1, 0 };
    memcpy(ptr, g_const, 3);

    return 0;
}

extern "C" char *func2(char *ptr)
{
    static char buf[4096];
    if (!getcwd(buf, sizeof(buf)))
        buf[0] = 0;
#define STR "test_string"
    memcpy(ptr, STR, sizeof(STR));
    return buf;
}

Note that func2 will be hidden, and as it's unreferenced the .text.func2
section will be discarded, as will be the .bss._ZZ5func2E3buf section:

# ...
./ld/ld-new: Removing unused section '.text.func2' in file 'show-link-discard-fail.o'
./ld/ld-new: Removing unused section '.bss._ZZ5func2E3buf' in file 'show-link-discard-fail.o'

But the relocation to getcwd (or better it's default decorated variant
getcwd@@GLIBC_2.2.5) will stay and transferred into the output file.
It should have been discarded too.  In fact the gc_sweep_hook on x86_64
for instance does lower the PLT count for this reloc, and hence seems
to assume that it really won't be emitted.  But alas, it is.
Comment 1 H.J. Lu 2011-05-17 21:21:11 UTC
Works for me:

[hjl@gnu-6 pr12772]$ cat x.cc
#include <string.h>
#include <stdlib.h>
#include <unistd.h>

extern "C" int __attribute__((visibility("default"))) func(char *ptr)
{
    static const char g_const[] = { 1, 1, 0 };
    memcpy(ptr, g_const, 3);

    return 0;
}

extern "C" char *func2(char *ptr)
{
    static char buf[4096];
    if (!getcwd(buf, sizeof(buf)))
        buf[0] = 0;
#define STR "test_string"
    memcpy(ptr, STR, sizeof(STR));
    return buf;
}
[hjl@gnu-6 pr12772]$ make
g++  -c -fpic -O3 -fdata-sections -ffunction-sections -fvisibility-inlines-hidden -fvisibility=hidden -o x.o x.cc
g++  -shared -Wl,-gc-sections -o x.so x.o
readelf -r x.so

Relocation section '.rela.dyn' at offset 0x3f0 contains 4 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000200640  000000000008 R_X86_64_RELATIVE                    0000000000200640
0000002007f8  000200000006 R_X86_64_GLOB_DAT 0000000000000000 __gmon_start__ + 0
000000200800  000300000006 R_X86_64_GLOB_DAT 0000000000000000 _Jv_RegisterClasses + 0
000000200808  000400000006 R_X86_64_GLOB_DAT 0000000000000000 __cxa_finalize + 0

Relocation section '.rela.plt' at offset 0x450 contains 1 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000200828  000400000007 R_X86_64_JUMP_SLO 0000000000000000 __cxa_finalize + 0
[hjl@gnu-6 pr12772]$
Comment 2 lion 2011-05-18 09:33:37 UTC
Works in what meaning?
I have same readelf -r output:
Relocation section '.rela.dyn' at offset 0x440 contains 4 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
0000000018c8  000000000008 R_X86_64_RELATIVE                    00000000000018c8
000000001868  000100000006 R_X86_64_GLOB_DAT 0000000000000000 __gmon_start__ + 0
000000001870  000500000006 R_X86_64_GLOB_DAT 0000000000000000 __cxa_finalize + 0
000000001878  000600000006 R_X86_64_GLOB_DAT 0000000000000000 _Jv_RegisterClasses + 0

Relocation section '.rela.plt' at offset 0x4a0 contains 1 entries:
  Offset          Info           Type           Sym. Value    Sym. Name + Addend
000000001898  000500000007 R_X86_64_JUMP_SLO 0000000000000000 __cxa_finalize + 0

But readelf -s shows:
Symbol table '.dynsym' contains 12 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
     1: 00000000000004c8     0 SECTION LOCAL  DEFAULT   10
     2: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND __gmon_start__
     3: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
     4: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND __cxa_finalize@GLIBC_2.2.5 (2)
     5: 0000000000000000     0 FUNC    GLOBAL DEFAULT  UND getcwd@GLIBC_2.2.5 (2)
     6: 0000000000201020     0 NOTYPE  GLOBAL DEFAULT  ABS _end
     7: 0000000000201010     0 NOTYPE  GLOBAL DEFAULT  ABS _edata
     8: 0000000000201010     0 NOTYPE  GLOBAL DEFAULT  ABS __bss_start
     9: 00000000000004c8     0 FUNC    GLOBAL DEFAULT   10 _init
    10: 000000000000062c    16 FUNC    GLOBAL DEFAULT   13 _fini
    11: 00000000000005d0    23 FUNC    GLOBAL DEFAULT   12 func

I not obvious to me that relocations is in blame here, but as in my original report - getcwd dependency not discarded properly.
Comment 3 lion 2011-05-18 11:43:47 UTC
*It`s not obvious to me
Comment 4 Michael Matz 2011-05-18 11:54:22 UTC
Yeah, sorry, my title was wrong.  The relocations really are gone.  But the
symbol table entry will stay, even though nothing refers to it anymore
(ultimately this then leads to libraries being included that aren't used
by anything).
Comment 5 Alan Modra 2011-05-18 15:36:45 UTC
This optimisation could be done in a target gc_sweep_symbol hook.  You can't do so in the generic ELF code because you'd need to look at plt, got and dyn_reloc fields of the sym.
Comment 6 lion 2011-12-02 17:57:19 UTC
gold linker already fix this issue http://sourceware.org/bugzilla/show_bug.cgi?id=12818
any troubles to fix this in ld?
Comment 7 lion 2011-12-04 17:11:29 UTC
This bug leads to some difficulties in android. Application loads many useless .so libraries (and very heavy) and debug analysys become harder when there lot of garbage symbols and I don`t know which of them really used.
Comment 8 Sourceware Commits 2011-12-07 12:16:03 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2011-12-07 12:15:54

Modified files:
	bfd            : ChangeLog elflink.c 

Log message:
	PR ld/12772
	* elflink.c (elf_gc_sweep_symbol): Discard unmarked symbols
	defined in shared libraries.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5533&r2=1.5534
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elflink.c.diff?cvsroot=src&r1=1.431&r2=1.432
Comment 9 Alan Modra 2011-12-07 12:17:21 UTC
Fixed mainline
Comment 10 Sourceware Commits 2017-01-18 18:32:26 UTC
The master branch has been updated by Maciej W. Rozycki <macro@sourceware.org>:

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

commit 81ff47b3a54633819fac4d973e34f1ff0c65606e
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Mon Jan 16 22:10:57 2017 +0000

    PR ld/20828: Fix linker script symbols wrongly forced local with section GC
    
    Fix a generic ELF linker regression introduced with a chain of changes
    made to unused input section garbage collection:
    
    - commit 1a766c6843ce ("Also hide symbols without PLT nor GOT
      references."),
      <https://sourceware.org/ml/binutils/2011-09/msg00076.html>,
    
    - commit 1d5316ab67e1 ("PR ld/13177: garbage collector retains zombie
      references to external libraries"),
      <https://sourceware.org/ml/binutils/2011-10/msg00161.html>,
    
    - commit 6673f753c019 ("Fix PR 12772, garbage collection of dynamic
      syms"), <https://sourceware.org/ml/binutils/2011-12/msg00077.html>,
    
    causing the garbage collection of unused symbols present in a DSO
    involved in a link to make identically named symbols ordinarily defined
    (i.e. not hidden or PROVIDEd) by a linker script local, even though the
    latter symbols are supposed to be global as if no DSO defined them as
    well.
    
    This is because linker script assignments are processed very late as
    `lang_process' proceeds, down in the call to `ldemul_before_allocation',
    which is made after the call to `lang_gc_sections' to do input section
    garbage collecting.  Consequently if unused, then any such DSO-defined
    symbol has already been garbage-collected and internally marked local.
    It would ordinarily be removed from dynamic symbol table output, however
    a linker script assignment correctly replaces its original definition
    with the new one and enters it into the dynamic symbol table produced as
    it is supposed to be exported.  The original local marking is however
    retained making the symbol local in the dynamic symbol table and
    therefore not available externally.  This also causes a sorting problem
    with the MIPS target, which does not expect non-section local dynamic
    symbols to be output and produces an invalid binary.
    
    Fix the problem then, by removing the `forced_local' marking for the
    offending case and add suitable test cases.  First to verify that unused
    symbols ordinarily defined with linker script assignments remain
    exported in the context of input section garbage collection whether or
    not a DSO defining identically named symbols is present in the link.
    Second that a linker version script still correctly retains or removes
    such symbols as requested.
    
    	bfd/
    	PR ld/20828
    	* elflink.c (bfd_elf_record_link_assignment): Clear any
    	`forced_local' marking for DSO symbols that are not being
    	provided.
    
    	ld/
    	PR ld/20828
    	* testsuite/ld-elf/pr20828-1.sd: New test.
    	* testsuite/ld-elf/pr20828-2a.sd: New test.
    	* testsuite/ld-elf/pr20828-2b.sd: New test.
    	* testsuite/ld-elf/pr20828.ld: New test linker script.
    	* testsuite/ld-elf/pr20828.ver: New test version script.
    	* testsuite/ld-elf/pr20828.s: New test source.
    	* testsuite/ld-elf/shared.exp: Run the new test.
Comment 11 Sourceware Commits 2017-01-18 18:47:17 UTC
The binutils-2_28-branch branch has been updated by Maciej W. Rozycki <macro@sourceware.org>:

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

commit 4030096d6a689125c026ce1e6300717c2050d105
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Mon Jan 16 22:10:57 2017 +0000

    PR ld/20828: Fix linker script symbols wrongly forced local with section GC
    
    Fix a generic ELF linker regression introduced with a chain of changes
    made to unused input section garbage collection:
    
    - commit 1a766c6843ce ("Also hide symbols without PLT nor GOT
      references."),
      <https://sourceware.org/ml/binutils/2011-09/msg00076.html>,
    
    - commit 1d5316ab67e1 ("PR ld/13177: garbage collector retains zombie
      references to external libraries"),
      <https://sourceware.org/ml/binutils/2011-10/msg00161.html>,
    
    - commit 6673f753c019 ("Fix PR 12772, garbage collection of dynamic
      syms"), <https://sourceware.org/ml/binutils/2011-12/msg00077.html>,
    
    causing the garbage collection of unused symbols present in a DSO
    involved in a link to make identically named symbols ordinarily defined
    (i.e. not hidden or PROVIDEd) by a linker script local, even though the
    latter symbols are supposed to be global as if no DSO defined them as
    well.
    
    This is because linker script assignments are processed very late as
    `lang_process' proceeds, down in the call to `ldemul_before_allocation',
    which is made after the call to `lang_gc_sections' to do input section
    garbage collecting.  Consequently if unused, then any such DSO-defined
    symbol has already been garbage-collected and internally marked local.
    It would ordinarily be removed from dynamic symbol table output, however
    a linker script assignment correctly replaces its original definition
    with the new one and enters it into the dynamic symbol table produced as
    it is supposed to be exported.  The original local marking is however
    retained making the symbol local in the dynamic symbol table and
    therefore not available externally.  This also causes a sorting problem
    with the MIPS target, which does not expect non-section local dynamic
    symbols to be output and produces an invalid binary.
    
    Fix the problem then, by removing the `forced_local' marking for the
    offending case and add suitable test cases.  First to verify that unused
    symbols ordinarily defined with linker script assignments remain
    exported in the context of input section garbage collection whether or
    not a DSO defining identically named symbols is present in the link.
    Second that a linker version script still correctly retains or removes
    such symbols as requested.
    
    	bfd/
    	PR ld/20828
    	* elflink.c (bfd_elf_record_link_assignment): Clear any
    	`forced_local' marking for DSO symbols that are not being
    	provided.
    
    	ld/
    	PR ld/20828
    	* testsuite/ld-elf/pr20828-1.sd: New test.
    	* testsuite/ld-elf/pr20828-2a.sd: New test.
    	* testsuite/ld-elf/pr20828-2b.sd: New test.
    	* testsuite/ld-elf/pr20828.ld: New test linker script.
    	* testsuite/ld-elf/pr20828.ver: New test version script.
    	* testsuite/ld-elf/pr20828.s: New test source.
    	* testsuite/ld-elf/shared.exp: Run the new test.
    
    (cherry picked from commit 81ff47b3a54633819fac4d973e34f1ff0c65606e)