Bug 17615 - aarch64: ld.bfd generates SHN_ABS instead of SHN_UNDEF
Summary: aarch64: ld.bfd generates SHN_ABS instead of SHN_UNDEF
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.26
: P2 normal
Target Milestone: 2.26
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-17 23:23 UTC by dimitry
Modified: 2015-02-11 12:54 UTC (History)
3 users (show)

See Also:
Host:
Target: aarch64-none-linux-gnu, arm-none-linux-gnu
Build:
Last reconfirmed:


Attachments
ip-arm64-intermediates (1.01 MB, application/gzip)
2014-11-19 19:30 UTC, dimitry
Details
ip-arm-intermediates (948.01 KB, application/gzip)
2014-11-19 19:30 UTC, dimitry
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dimitry 2014-11-17 23:23:05 UTC
when compiling iproute2 'ip' binary for arm64 ld.bfd produces following symbol in .dynsym section

    11: 0000000000000000     4 OBJECT  GLOBAL DEFAULT  ABS __iproute2_user_hz_internal

The corresponding relocation is:

0000000000045f78  0000000b00000401 R_AARCH64_GLOB_DAT     0000000000000000 __iproute2_user_hz_internal + 0

when I compile the same code for arm I see symbol:

11: 00000000     0 OBJECT  GLOBAL DEFAULT  UND __iproute2_user_hz_internal

and relocation:

00030fbc  00000b15 R_ARM_GLOB_DAT         00000000   __iproute2_user_hz_internal

I believe that arm64 build should also produce undefined symbol here.
Comment 1 Marcus Shawcroft 2014-11-19 09:55:12 UTC
Please add a test case or reproducer.
Comment 2 dimitry 2014-11-19 19:30:00 UTC
Created attachment 7951 [details]
ip-arm64-intermediates
Comment 3 dimitry 2014-11-19 19:30:46 UTC
Created attachment 7952 [details]
ip-arm-intermediates
Comment 4 dimitry 2014-11-19 19:33:11 UTC
attached tar.gz files contain object files as well as linked ip binary (unstripped version is under LINKED/)

The symbol in question is __iproute2_user_hz_internal; It is referenced by ipneigh.o and iproute.o
Comment 5 Jiong Wang 2014-12-16 09:33:46 UTC
Hi Dimitry,

  Thanks for reporting this.

  A quick look at your attachment shows libiprouteutil.so and libnetlink.so are required to reproduce this issue, while I haven't found them in your attachment.

  Could you please upload them also ?

  And it will be good if you could paste your linking command when linking the final ip.

  I tried to reproduce this by build iproute2 downloaded from official release, while found they static link libutil.a which defines iproute2_user_hz_internal, so can't reproduce, I guess you was building Android version.

readelf -d ip

Dynamic section at offset 0x35880 contains 29 entries:
  Tag        Type                         Name/Value
 0x0000000000000001 (NEEDED)             Shared library: [libc.so]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so]
 0x0000000000000001 (NEEDED)             Shared library: [libiprouteutil.so]
 0x0000000000000001 (NEEDED)             Shared library: [libnetlink.so]
 0x0000000000000001 (NEEDED)             Shared library: [libc++.so]
Comment 6 Jiong Wang 2014-12-16 16:47:15 UTC
Have setup a local Android aarch64 build env, reproduced.
Comment 7 Jiong Wang 2014-12-17 09:18:29 UTC
I believe this is a generic issue.

Confirmed arm has this issue also, if you use bfd linker instead of gold linker.
the default linker "ld" in prebuilt for arm is actually ld.gold, not ld.bfd, that's why ARM is OK.

And the symbol start to be wrong since in libiprouteutil.so, it's marked as ABS with absolute value 4.
Comment 8 Jiong Wang 2014-12-17 11:33:00 UTC
And a further investigation, from my understanding this is not a bug.

actually, this is caused by --gc-sections optimization, and existed on all arch with gc-section supported enabled, including x86. the problem can easily reproduced by

1.c
==
extern int hz_internal_1;
extern int hz_internal_2;
int main (int argc, char **argv)
{
  printf ("%d, %d\n", hz_internal_1, hz_internal_2);
  return 0;
}

2.c
==
int hz_internal_1;
int hz_internal_2;

int cal (int a)
{
    return a + 1;
}

gcc  -Wl,--gc-sections -shared -o lib2.so 2.c
gcc 1.c -L. -l2


if --gc-sections specified, then linker will try to remove all unnecessary sections.

in this example, if __iproute2_user_hz_internal is defined in shared library without initialized value, there is no need to allocate space in shared library, just allocate the space in .bss in where it's referenced, and make all relocation against that new place.

while if __iproute2_user_hz_internal is defined in .so and with initialized value, then we need to always allocate space for it. and generate COPY relocation for whether it's referenced.

so mark as invalid
Comment 9 dimitry 2014-12-19 07:01:35 UTC
hi, I was looking at the .so file. I guess the part I do not understand is that .bss section is there; and PT_LOADS seems to be in place; so as far as I understand there is no reason for making st_shndx for hz_internal_* ABS; it doesn't really save any space.

Isn't the fact that st_shndx for global (.bss) == ABS a bug?
Comment 10 H.J. Lu 2014-12-19 13:50:51 UTC
(In reply to dimitry from comment #9)
> hi, I was looking at the .so file. I guess the part I do not understand is
> that .bss section is there; and PT_LOADS seems to be in place; so as far as
> I understand there is no reason for making st_shndx for hz_internal_* ABS;
> it doesn't really save any space.
> 
> Isn't the fact that st_shndx for global (.bss) == ABS a bug?

Use "ld --gc-sections -shared" directly.  You will see there is no
.bss section at all.  The .bss section you see comes from other
input files added by GCC driver.
Comment 11 dimitry 2014-12-19 17:36:43 UTC
(In reply to H.J. Lu from comment #10)
> Use "ld --gc-sections -shared" directly.  You will see there is no
> .bss section at all.  The .bss section you see comes from other
> input files added by GCC driver.

I am using 2.c and the command from comment #8 to compile lib2.so; my assumption is: if .bss section is coming from other source files it should not change it's size if I initialize one of global ints to some non-zero value; but if I do .bss section size drops from 8 to 4 bytes as expected and data section increases by 4 bytes. So it looks like .bss section is allocated for the global variables declared in 2.c. Am I missing something else here?
Comment 12 dimitry 2014-12-19 18:01:44 UTC
and yes; there is no .bss section when I use ld directly.. so is this gcc bug then?
Comment 13 dimitry 2015-01-17 00:36:00 UTC
Even thought this is not arm64 specific. I still think that this is a bug. --gc-sections shouldn't drop .bss in this case.
Comment 14 Sourceware Commits 2015-01-19 17:25:07 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=3e2aa5bbd36be9cf63530e5db2f6cf3898762a22

commit 3e2aa5bbd36be9cf63530e5db2f6cf3898762a22
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Jan 19 09:23:29 2015 -0800

    Add a testcase for PR ld/17615
    
    	PR ld/17615
    	* ld-elf/pr17615.d: New file.
    	* ld-elf/pr17615.s: Likewise.
Comment 15 H.J. Lu 2015-01-19 17:26:07 UTC
Fixed for 2.26.
Comment 16 Alan Modra 2015-01-19 21:47:43 UTC
Fixed by git commit c4621b33
Comment 17 Jiong Wang 2015-01-19 22:11:42 UTC
(In reply to Alan Modra from comment #16)
> Fixed by git commit c4621b33

thanks very much for fixing this !
Comment 18 Sourceware Commits 2015-01-20 00:43:52 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 1c9177d9a5c3e06d3344347c8068acfb7d8ecc8b
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Jan 20 11:06:28 2015 +1030

    Fix garbage collection of common symbols for powerpc64
    
    I forgot powerpc64 has its own gc_mark_dynamic_ref.
    
    	PR ld/17615
    	* elf64-ppc.c (ppc64_elf_gc_mark_dynamic_ref): Don't drop
    	ELF_COMMON_DEF syms.
Comment 19 Sourceware Commits 2015-01-20 00:55:54 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 10ab94ebf8fdabf14954f53f1d060fd470658512
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Jan 20 11:11:38 2015 +1030

    Fix pr17615 testcase
    
    	PR ld/17615
    	* ld-elf/pr17615.d: Match .sbss too.
Comment 20 Sourceware Commits 2015-02-11 12:54:12 UTC
The binutils-2_25-branch branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit c1b7d37c0d6bd15b1a154aef0c764a4350dd0f22
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Jan 20 11:06:28 2015 +1030

    Fix garbage collection of common symbols for powerpc64
    
    I forgot powerpc64 has its own gc_mark_dynamic_ref.
    
    	PR ld/17615
    	* elf64-ppc.c (ppc64_elf_gc_mark_dynamic_ref): Don't drop
    	ELF_COMMON_DEF syms.