Bug 15149

Summary: Weak reference leads to DT_NEEDED entry
Product: binutils Reporter: H.J. Lu <hjl.tools>
Component: ldAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: normal CC: amodra, jakub, meadori, vapier
Priority: P2    
Version: 2.24   
Target Milestone: ---   
See Also: http://sourceware.org/bugzilla/show_bug.cgi?id=15126
Host: Target:
Build: Last reconfirmed:

Description H.J. Lu 2013-02-14 19:22:25 UTC
[hjl@gnu-6 xxx]$ cat foo.c
extern void bar (void);

int main()
{
  bar ();
  return 0;
}
[hjl@gnu-6 xxx]$ cat bar.c
extern int xxx __attribute__((weak));

int
bar (void)
{
  return xxx;
}
[hjl@gnu-6 xxx]$ cat xxx.c
int xxx = 3;
[hjl@gnu-6 xxx]$ cat yyy.c
[hjl@gnu-6 xxx]$ make
gcc -Wl,--no-copy-dt-needed-entries    -c -o bar.o bar.c
gcc -Wl,--no-copy-dt-needed-entries    -c -o foo.o foo.c
gcc -Wl,--no-copy-dt-needed-entries -shared -fPIC -o libxxx.so xxx.c
gcc -Wl,--no-copy-dt-needed-entries -shared -fPIC -o libyyy.so yyy.c libxxx.so
gcc -Wl,--no-copy-dt-needed-entries  -o x bar.o foo.o libyyy.so -Wl,-rpath-link,.
gcc -Wl,--no-copy-dt-needed-entries -fuse-ld=gold  -o y bar.o foo.o libyyy.so -Wl,-rpath-link,.
readelf -d x | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libyyy.so]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libxxx.so]
readelf -d y | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libyyy.so]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
[hjl@gnu-6 xxx]$
Comment 1 H.J. Lu 2013-02-15 23:41:33 UTC
This patch:

http://sourceware.org/ml/binutils/2013-02/msg00186.html

caused:

/export/gnu/import/git/gcc-test-intel64/bld/gcc/xgcc -B/export/gnu/import/git/gcc-test-intel64/bld/gcc/ /export/gnu/import/git/gcc-test-intel64/src-trunk/libjava/testsuite/libjava.jni/invocation/PR16923.c -I. -I.. -I/export/gnu/import/git/gcc-test-intel64/src-trunk/libjava/testsuite/libjava.jni -I/export/gnu/import/git/gcc-test-intel64/src-trunk/libjava/testsuite/../include -I/export/gnu/import/git/gcc-test-intel64/src-trunk/libjava/testsuite/../classpath/include -fdollars-in-identifiers -L/export/gnu/import/git/gcc-test-intel64/bld/x86_64-unknown-linux-gnu/32/libjava/.libs -ljvm -lm -m32 -o PR16923^M
/usr/local/bin/ld: /export/gnu/import/git/gcc-test-intel64/bld/gcc/32/crtbegin.o: undefined reference to symbol '_Jv_RegisterClasses'^M
/usr/local/bin/ld: note: '_Jv_RegisterClasses' is defined in DSO /export/gnu/import/git/gcc-test-intel64/bld/x86_64-unknown-linux-gnu/32/libjava/.libs/libgcj.so.14 so try adding it to the linker command line^M
/export/gnu/import/git/gcc-test-intel64/bld/x86_64-unknown-linux-gnu/32/libjava/.libs/libgcj.so.14: could not read symbols: Invalid operation^M
collect2: error: ld returned 1 exit status^M
compiler exited with status 1

_Jv_RegisterClasses is weak reference and libgcj.so isn't on the
linker command line. Since libgcj.so isn't on the linker command line,
gold resolves _Jv_RegisterClasses to 0 and the old bfd linker silently
resolves it to _Jv_RegisterClasses in libgcj.so.  Now executable
generated by gold and ld behaves differently:

[hjl@gnu-13 testsuite]$ readelf -sW /tmp/PR16923.gold | grep _Jv_RegisterClasses 
     8: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
    37: 0000000000000000     0 NOTYPE  WEAK   DEFAULT  UND _Jv_RegisterClasses
[hjl@gnu-13 testsuite]$ readelf -sW /tmp/PR16923.bfd | grep _Jv_RegisterClasses  
     9: 00000000004005e0     0 FUNC    WEAK   DEFAULT  UND _Jv_RegisterClasses
    59: 00000000004005e0     0 FUNC    WEAK   DEFAULT  UND _Jv_RegisterClasses
Comment 2 cvs-commit@gcc.gnu.org 2013-02-16 17:49:03 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	hjl@sourceware.org	2013-02-16 17:48:57

Modified files:
	bfd            : ChangeLog elflink.c 

Log message:
	Also track weak references
	
	PR ld/15149
	* elflink.c (elf_link_add_object_symbols): Also track weak
	references.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5968&r2=1.5969
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elflink.c.diff?cvsroot=src&r1=1.467&r2=1.468
Comment 3 H.J. Lu 2013-02-16 18:24:41 UTC
Fixed.
Comment 4 Jakub Jelinek 2013-03-11 17:44:08 UTC
This change is bogus, and breaks lots of packages.
There is nothing wrong with undef weak references that might sometimes be satisfied by some library, but in the next version of the library no longer brought in.  DT_NEEDED for undef weak references shouldn't be added, and nothing should be diagnosed.  Please revert.
Comment 5 H.J. Lu 2013-03-11 19:45:05 UTC
(In reply to comment #4)
> This change is bogus, and breaks lots of packages.
> There is nothing wrong with undef weak references that might sometimes be
> satisfied by some library, but in the next version of the library no longer
> brought in.  DT_NEEDED for undef weak references shouldn't be added, and
> nothing should be diagnosed.  Please revert.

The old linker silently adds the DSO to DT_NEEDED for weak reference.
Even if the next version of the library no longer brings in the DSO,
it is still in DT_NEEDED of executables linked against the current
library.

Gold doesn't add the DSO to DT_NEEDED for weak reference.  But it
leads to different run-time behavior.  See:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56431

for example.
Comment 6 Meador Inge 2013-04-04 01:52:09 UTC
I see a regression from this change when using a cross ARM GNU/Linux toolchain
built from trunk GCC and binutils sources:

$ cat a.c
#include <pthread.h>

void foo(void)
{
   pthread_create(0, 0, 0, 0);
}

$  cat test.cpp
#include <string>

extern "C" void foo(void);

int main(void)
{
   foo();
   std::string s("foo");
   return s.length();
}

$ ./install/bin/arm-none-linux-gnueabi-gcc -c -fPIC a.c
$ ./install/bin/arm-none-linux-gnueabi-gcc -shared -o liba.so a.o -lc -lpthread
$ ./install/bin/arm-none-linux-gnueabi-g++ test.cpp -L. -la
/scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/install/bin/../lib/gcc/arm-none-linux-gnueabi/4.9.0/../../../../arm-none-linux-gnueabi/bin/ld: /tmp/ccrHRnUy.o: undefined reference to symbol '__pthread_key_create@@GLIBC_2.4'
/scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/install/bin/../lib/gcc/arm-none-linux-gnueabi/4.9.0/../../../../arm-none-linux-gnueabi/bin/ld: note: '__pthread_key_create@@GLIBC_2.4' is defined in DSO /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/install/bin/../arm-none-linux-gnueabi/libc/lib/libpthread.so.0 so try adding it to the linker command line
/scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/install/bin/../arm-none-linux-gnueabi/libc/lib/libpthread.so.0: could not read symbols: Invalid operation
collect2: error: ld returned 1 exit status
Comment 7 Mike Frysinger 2013-04-04 04:38:14 UTC
(In reply to comment #6)

that error is being explored in bug 15126
Comment 8 H.J. Lu 2013-04-04 16:00:51 UTC
This change detects the package which behaves differently
when linked with gold. Now the linker issues an error instead
of silently adds DT_NEEDED for weak reference. We can change
error to warning.  But it won't fix the package's gold problem.
Comment 9 Meador Inge 2013-04-17 20:56:03 UTC
(In reply to comment #8)
> This change detects the package which behaves differently
> when linked with gold. Now the linker issues an error instead
> of silently adds DT_NEEDED for weak reference. We can change
> error to warning.  But it won't fix the package's gold problem.

I must be missing something.

I am not quite sure what you mean by package.  The reproduction
case I provided is a standalone C/C++ program that linked before
the patch applied for this issue and doesn't after the patch is
applied.

The issue crops up because of a '__pthread_key_create' weak reference
that gets pulled in from 'gthr-default.h' (which is originally derived
from 'gthr-posix.h' in libgcc) as a part of the concurrency extension
implementation (which I guess gets dragged in with #include <string>).

With this patch applied the weak reference to '__pthread_key_create'
in test.o gets tracked and thus an error is issued.
Comment 10 Alan Modra 2014-11-27 23:48:13 UTC
This has been fixed