Bug 18855

Summary: String constants mixed up when linked with gold on Linux/sparc64
Product: binutils Reporter: Artyom Tarasenko <atar4qemu>
Component: goldAssignee: Cary Coutant <ccoutant>
Status: RESOLVED FIXED    
Severity: critical CC: fberckel, glaubitz, ian, sourceware-bugzilla
Priority: P2    
Version: 2.26   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:
Attachments: gcc -v -Wl,--verbose log with -Wl,-fuse-ld=gold
.s files produced with a -v --save-temps gcc call
Automated testcase

Description Artyom Tarasenko 2015-08-20 12:56:43 UTC
Created attachment 8538 [details]
gcc -v -Wl,--verbose log with -Wl,-fuse-ld=gold

Systemd components linked with gold fail under sparc(64).
I can't reproduce the bug with a smaller test case, but it's 100% reproducible with building systemd:

# First, try linking with bfd:

$ gcc -pipe -Wall -Wextra -Wundef -Wformat=2 -Wformat-security -Wformat-nonliteral -Wlogical-op -Wmissing-include-dirs -Wold-style-definition -Wpointer-arith -Winit-self -Wdeclaration-after-statement -Wfloat-equal -Wsuggest-attribute=noreturn -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=missing-declarations -Werror=return-type -Werror=shadow -Wstrict-prototypes -Wredundant-decls -Wmissing-noreturn -Wshadow -Wendif-labels -Wstrict-aliasing=2 -Wwrite-strings -Wno-unused-parameter -Wno-missing-field-initializers -Wno-unused-result -Wno-format-signedness -Werror=overflow -Wdate-time -Wnested-externs -ffast-math -fno-common -fdiagnostics-show-option -fno-strict-aliasing -fvisibility=hidden -fstack-protector -fstack-protector-strong -fPIE --param=ssp-buffer-size=4 -flto -ffunction-sections -fdata-sections -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,--gc-sections -Wl,--as-needed -Wl,--no-undefined -Wl,-z -Wl,relro -Wl,-z -Wl,now -pie -Wl,-fuse-ld=bfd -Wl,-z -Wl,relro -o udevadm src/udev/udevadm.o src/udev/udevadm-info.o src/udev/udevadm-control.o src/udev/udevadm-monitor.o src/udev/udevadm-hwdb.o src/udev/udevadm-settle.o src/udev/udevadm-trigger.o src/udev/udevadm-test.o src/udev/udevadm-test-builtin.o src/udev/udevadm-util.o  ./.libs/libudev-core.a -lselinux -ldl -lrt -lm -lresolv -llzma -lgcrypt -lacl -lblkid -lkmod -lcap -pthread -v -Wl,--verbose -v > ../bfd.log 2>&1

# Save the binary

mv udevadm udevadm-bfd

# Link with gold

 gcc -pipe -Wall -Wextra -Wundef -Wformat=2 -Wformat-security -Wformat-nonliteral -Wlogical-op -Wmissing-include-dirs -Wold-style-definition -Wpointer-arith -Winit-self -Wdeclaration-after-statement -Wfloat-equal -Wsuggest-attribute=noreturn -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=missing-declarations -Werror=return-type -Werror=shadow -Wstrict-prototypes -Wredundant-decls -Wmissing-noreturn -Wshadow -Wendif-labels -Wstrict-aliasing=2 -Wwrite-strings -Wno-unused-parameter -Wno-missing-field-initializers -Wno-unused-result -Wno-format-signedness -Werror=overflow -Wdate-time -Wnested-externs -ffast-math -fno-common -fdiagnostics-show-option -fno-strict-aliasing -fvisibility=hidden -fstack-protector -fstack-protector-strong -fPIE --param=ssp-buffer-size=4 -flto -ffunction-sections -fdata-sections -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wl,--gc-sections -Wl,--as-needed -Wl,--no-undefined -Wl,-z -Wl,relro -Wl,-z -Wl,now -pie -Wl,-fuse-ld=gold -Wl,-z -Wl,relro -o udevadm src/udev/udevadm.o src/udev/udevadm-info.o src/udev/udevadm-control.o src/udev/udevadm-monitor.o src/udev/udevadm-hwdb.o src/udev/udevadm-settle.o src/udev/udevadm-trigger.o src/udev/udevadm-test.o src/udev/udevadm-test-builtin.o src/udev/udevadm-util.o  ./.libs/libudev-core.a -lselinux -ldl -lrt -lm -lresolv -llzma -lgcrypt -lacl -lblkid -lkmod -lcap -pthread -v -Wl,--verbose -v > ../gold.log 2>&1

# Now try the binaries:
$ ./udevadm-bfd --version
224
$ ./udevadm --version
%-6s[%li.%06ld] %-8s %s (%s)

As you see the binary linked with gold takes a wrong string for the version. Some other strings are mixed up as well. ( Debian Bug#790560 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=790560 )

At least the binutils versions 2.24, 2.25, 2.25.1 and HEAD are affected.
Comment 1 Cary Coutant 2015-08-20 22:44:50 UTC
Sorry, I don't have a SPARC development environment, so to investigate this, I'm going to need all the .o files (real .o files, not LTO intermediates).

Does it still fail without -flto?

Can you add -Wl,--stats to the command line and attach the output?
Comment 2 John Paul Adrian Glaubitz 2015-09-15 11:51:18 UTC
Hi Cary!

(In reply to Cary Coutant from comment #1)
> Sorry, I don't have a SPARC development environment, so to investigate this,
> I'm going to need all the .o files (real .o files, not LTO intermediates).

I can actually provide one for you. I'll contact you shortly.

> Does it still fail without -flto?
> 
> Can you add -Wl,--stats to the command line and attach the output?

I can also provide any additional debug information you need. This bug seems to seriously hamper Debian's sparc64 port, so I am more than happy to help!

Adrian
Comment 3 Artyom Tarasenko 2015-09-19 11:18:24 UTC
Since I seem to be not the only one interested in this bug anymore, here is a quick summary of the off-line discussions with Cary:

- the ".o" files provided by the gcc call reported above are useless for debugging.
- the problem is not reproducible with full ".o" files
- the problem happens only with link time optimization (-flto) which makes the debugging hard

In the attachment I add the .s files produced with the command above extended with "-v --save-temps" options.
Comment 4 Artyom Tarasenko 2015-09-19 11:21:40 UTC
Created attachment 8619 [details]
.s files produced with a -v --save-temps gcc call
Comment 5 Michael Karcher 2015-10-02 20:35:59 UTC
Created attachment 8675 [details]
Automated testcase

I made a automatic test case for it (see attachment). I think the ltrans.*.s file in my archive are my own gcc invocation on sparc64, but the files from attachment 8619 [details] should do as well. Just run make from the attached archive, after extracting it into a binutils build tree:

mkarcher@aquila:~/src/Development/binutils-gdb/testdata$ make -s clean check
First output line: address that gets passed to puts
Second output line: address that contains the string to be printed
 142c00 2025730a 00000000 252d3673 5b256c69   %s.....%-6s[%li
 1428d0 32323600 00000000 3d3d3d20 74726965  226.....=== trie
mkarcher@aquila:~/src/Development/binutils-gdb/testdata$ make -s clean check LD=../ld/ld-new
../ld/ld-new: warning: cannot find entry symbol _start; defaulting to 0000000000100120
First output line: address that gets passed to puts
Second output line: address that contains the string to be printed
 1428b0 6e000000 00000000 32323600 00000000  n.......226.....
 1428b0 6e000000 00000000 32323600 00000000  n.......226.....

Using ld, the output is OK (the same line is outputted twice), using gold (the default), the output shows that the string reference is off. The testcase is host platform independent, it just requires binutils to be compiled for target architecture sparc64-unknown-linux-gnu.

Interestingly, the output from gold has been observed to be correct for some orders of the object files, but incorrect for other orders (happened to me until I had sorting in, as the file names got mixed up as the kernel liked depending on what files were present in the same directory).
Comment 6 Sourceware Commits 2015-10-06 21:44:53 UTC
The master branch has been updated by Cary Coutant <ccoutant@sourceware.org>:

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

commit 9eacb935acd69f6532135ec1353630db5111467c
Author: Cary Coutant <ccoutant@gmail.com>
Date:   Tue Oct 6 14:43:49 2015 -0700

    Fix bug when applying R_SPARC_GOTDATA_OP* relocations to local symbols.
    
    When applying a GOTDATA_OP* relocation to a local symbol, the addend
    is being applied after getting the value of the symbol. When the
    relocation refers to a merge section, however, the addend must be
    provided when computing the symbol value, since the contents of
    the section may have been rearranged.
    
    gold/
    	PR gold/18855
    	* sparc.cc (Sparc_relocate_functions::gdop_hix22): Remove addend
    	parameter.
    	(Sparc_relocate_functions::gdop_lox10): Likewise.
    	(Target_sparc::Relocate::relocate): Use addend when computing
    	symbol value for R_SPARC_GOTDATA_OP*.
Comment 7 Cary Coutant 2015-10-06 21:46:37 UTC
Fixed on trunk.
Comment 8 Cary Coutant 2015-10-06 21:47:31 UTC
> I made a automatic test case for it (see attachment). I think the ltrans.*.s
> file in my archive are my own gcc invocation on sparc64, but the files from
> attachment 8619 [details] should do as well. Just run make from the attached
> archive, after extracting it into a binutils build tree:

Thanks for the reproducer -- that helped a lot!