Bug 22471

Summary:	libraries using version scripts can cause undefined reference to linker-defined symbols
Product:	binutils	Reporter:	James Cowgill <jcowgill+sourceware>
Component:	ld	Assignee:	Not yet assigned to anyone <unassigned>
Status:	RESOLVED FIXED
Severity:	normal	CC:	hjl.tools
Priority:	P2
Version:	2.30
Target Milestone:	2.30
Host:		Target:
Build:		Last reconfirmed:	2017-11-23 00:00:00
Attachments:	testcase tar.gz I am testing this patch.

Description James Cowgill 2017-11-21 13:33:24 UTC

Created attachment 10625 [details]
testcase tar.gz

This bug arises when the following occurs:

* An executable which references the __bss_start symbol (from the linker script)
* links against a library using a version script so that __bss_start is not exported
* in turn links against another "normal" library which does export __bss_start

I this case ld will complain that the __bss_start symbol is undefined and that the "normal" library must be linked into the executable directly. I think this is incorrect and ld should be using the definition of the symbol from the linker script. Linking the "normal" library manually works around the bug, but this isn't nice because that library is an implementation detail that the executable shouldn't need to know about.

The gold linker doesn't seem to be affected (for whatever reason).

I've attached a testcase which illustrates the problem. It was first discovered while compiling mariadb (specifically mariabackup) on Debian MIPS.

> $ ./build 
> + rm -f null.c
> + touch null.c
> + gcc -c -fPIC consume.c -o consume.o
> + gcc -c -fPIC null.c -o null.o
> + gcc -shared -Wl,--rpath=/user/jcowgill/workspace/binutils/test-bss-start null.o -o libtest.so
> + gcc -shared -Wl,--rpath=/user/jcowgill/workspace/binutils/test-bss-start -Wl,--version-script=middle.ver null.o libtest.so -o libmiddle.so
> + ld --rpath=/user/jcowgill/workspace/binutils/test-bss-start consume.o libmiddle.so -o consume
> ld: consume.o: undefined reference to symbol '__bss_start'
> //user/jcowgill/workspace/binutils/test-bss-start/libtest.so: error adding symbols: DSO missing from command line

Comment 1 H.J. Lu 2017-11-23 14:27:30 UTC

This can happen to any linker defined symbols.

Comment 2 H.J. Lu 2017-11-23 14:35:22 UTC

Builtin linker scripts for each target define different sets
of symbols.

Comment 3 H.J. Lu 2017-11-23 16:01:25 UTC

Created attachment 10631 [details]
I am testing this patch.

Comment 4 Sourceware Commits 2017-11-28 12:28:00 UTC

The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 165f707ac88916aedecc96fa518be8879704d6da
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Nov 27 13:40:43 2017 +1030

    PR22471, undefined reference to linker-defined symbols
    
    This patch processes linker script assignment statements before ld
    opens DT_NEEDED libraries, in order to define symbols like __bss_start
    that might also be defined by a library, falsely triggering an error
    about "DSO missing from command line".
    
    The initial value won't be correct when assigning a symbol from dot,
    and I make no attempt to handle all expressions.  For example, an
    assignment like "_start_foo = ADDR (.foo)" isn't valid until sections
    are laid out, so won't define _start_foo early.  What's here should be
    enough for most common scripts, and hopefully won't perturb fragile
    scripts.
    
    bfd/
    	PR 22471
    	* elflink.c (_bfd_elf_merge_symbol): Allow weak symbols to override
    	early passes over linker script symbols.
    	* linker.c (_bfd_generic_link_add_one_symbol): Allow symbols to
    	override early passes over linker script symbols.  Clear ldscript_def
    	on symbol definitions.
    ld/
    	PR 22471
    	* ldexp.c (struct definedness_hash_entry): Delete "by_script".  Make
    	"iteration" an 8-bit field, and update mask in all uses.
    	(definedness_newfunc): Don't init "by_script".
    	(update_definedness): Test ldscript_def rather than by_script.
    	(is_sym_value): Likewise.
    	(fold_name <DEFINED>): Return a result for first phase.  Test
    	ldscript_def.
    	(fold_name <NAME>): Return a result for first phase.
    	* ldlang.c (open_input_bfds): Process all assignments, not just
    	defsym.
    	(lang_process): Increment lang_statement_iteration before
    	open_input_bfds.
    	* testsuite/ld-mips-elf/tlsdyn-o32-1.d: Adjust for larger .dynsym.
    	* testsuite/ld-mips-elf/tlsdyn-o32-1.got: Likewise.
    	* testsuite/ld-mips-elf/tlsdyn-o32-2.d: Likewise.
    	* testsuite/ld-mips-elf/tlsdyn-o32-2.got: Likewise.
    	* testsuite/ld-mips-elf/tlsdyn-o32-3.d: Likewise.
    	* testsuite/ld-mips-elf/tlsdyn-o32-3.got: Likewise.

Comment 5 Sourceware Commits 2017-11-29 12:27:40 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=08f650e6b657e59959d7db8af100720721bd10a2

commit 08f650e6b657e59959d7db8af100720721bd10a2
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Nov 29 04:20:48 2017 -0800

    ld: Add a testcase for PR ld/22471
    
    	PR ld/22471
    	* testsuite/ld-elf/pr22471.t: New file.
    	* testsuite/ld-elf/pr22471a.s: Likewise.
    	* testsuite/ld-elf/pr22471b.s: Likewise.
    	* testsuite/ld-elf/shared.exp (ASFLAGS): Define UNDERSCORE for
    	underscore targets.
    	Run PR ld/22471 tests.

Comment 6 H.J. Lu 2017-11-29 12:56:05 UTC

Fixed.

Comment 7 Sourceware Commits 2017-11-30 05:09:35 UTC

The master branch has been updated by Alan Modra <amodra@sourceware.org>:

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

commit 668247656d5dec815a57761e98720dda09f72481
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Nov 30 13:10:35 2017 +1030

    Fix testcase for PR ld/22471
    
    ASFLAGS should be restored after any modification.  Fixes
    metag-linux  +FAIL: Long branch stub
    metag-linux  +FAIL: Long branch stub (PIC, app)
    mn10300-elf  +FAIL: difference of two same-section symbols where the difference is held in another section
    mn10300-elf  +FAIL: relaxation and alignment directives
    
    	PR 22471
    	* testsuite/ld-elf/shared.exp: Save and restore ASFLAGS.