Bug 19019 - [SPARC64] Only registers %g[2367] can be declared using STT_REGISTER when linking against libsystemd
Summary: [SPARC64] Only registers %g[2367] can be declared using STT_REGISTER when lin...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.25
: P2 normal
Target Milestone: ---
Assignee: Cary Coutant
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-29 12:24 UTC by John Paul Adrian Glaubitz
Modified: 2016-03-21 20:59 UTC (History)
6 users (show)

See Also:
Host:
Target: sparc*-*-*
Build:
Last reconfirmed:


Attachments
libsystemd0_226-3_sparc64 (204.37 KB, application/gzip)
2015-10-02 12:38 UTC, Frans van Berckel
Details
libsystemd.so.0.3.1 (61.66 KB, application/gzip)
2015-10-02 14:19 UTC, Frans van Berckel
Details
machine object files created during LTO link (267.17 KB, application/bz2)
2015-10-02 21:01 UTC, Michael Karcher
Details
Minimal gold testcase for STT_REGISTER relocations (1.39 KB, patch)
2015-10-03 22:10 UTC, Michael Karcher
Details | Diff
Minimal gold testcase for STT_REGISTER relocations (fixed "written by" line) (1.40 KB, patch)
2015-10-03 22:15 UTC, Michael Karcher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Paul Adrian Glaubitz 2015-09-29 12:24:53 UTC
Hello!

We are currently having problems on sparc64 when linking against libsystemd:

/usr/bin/ld: /usr/lib/gcc/sparc64-linux-gnu/5/../../../sparc64-linux-gnu/libsystemd.so: Only registers %g[2367] can be declared using STT_REGISTER
/usr/lib/gcc/sparc64-linux-gnu/5/../../../sparc64-linux-gnu/libsystemd.so: error adding symbols: File format not recognized
collect2: error: ld returned 1 exit status

This issues seems to have been introduced with one of the recent versions of binutils as systemd package versions linked with binutils_2.24.51.20140814 do not expose this bug.

A full example build log of one of the packages affected can be found in [1].

Cheers,

Adrian

> [1] https://buildd.debian.org/status/fetch.php?pkg=mate-session-manager&arch=sparc64&ver=1.10.2-2&stamp=1443524013
Comment 1 Nick Clifton 2015-10-02 08:51:18 UTC
Hi John,

  Please could you upload the libsystemd.so file so that I can examine it ?

  It seems strange that this problem has only recently arisen, as the code that checks STT_REGISTER and issues the error message has been in the binutils since 1999.

  It is also not clear to me if the error message is correct.  Looking the version 2.4.1 of the Sparc Compliance Definition document (which is the latest version I could find), the only restriction appears to be that the STT_REGISTER symbol should refer to an integer register.  I am not a SPARC expert however, so if you know more about the ABI, please let me know.

Cheers
  Nick
Comment 2 Frans van Berckel 2015-10-02 12:38:47 UTC
Created attachment 8671 [details]
libsystemd0_226-3_sparc64

File: /lib/sparc64-linux-gnu/libsystemd.so.0.11.0
Extracted: libsystemd0_226-3_sparc64.deb
Origin: https://packages.debian.org/sid/sparc64/libsystemd0/download
Zip: by gzip < libsystemd.so.0.11.0 >libsystemd.so.0.11.0.gz
Comment 3 Jose E. Marchesi 2015-10-02 13:07:24 UTC
The SCD in page 4P-3 declares that the purpose of STT_SPARC_REGISTER is to indicate the "Usage of a global register reserved by the application" and, optionally, global registers reserved for system software.  The subset of global registers reserved for applications is defined in page 3P-10 as %g2 and %g3.  Likewise, the subset of global registers reserved for system software is defined in the same table.

So I think that the BFD check is pertinent and matches the ABI.
Comment 4 Jose E. Marchesi 2015-10-02 13:23:02 UTC
$ readelf -s libsystemd.so.0.11.0
[...]
   617: 0000000000000000     0 REGISTER GLOBAL DEFAULT  UND
    
There it is... st_value is 0.  How was this library linked, exactly?  I tried to build systemd 226 in my non-Debian sparc64 system, but gave up due to many build errors...
Comment 5 John Paul Adrian Glaubitz 2015-10-02 13:28:21 UTC
Hi Jose!

(In reply to Jose E. Marchesi from comment #3)
> So I think that the BFD check is pertinent and matches the ABI.

Thank you very much for the analysis!

So you'd say that this is indeed not a bug in binutils but gcc?

If yes, I will try to pinpoint the gcc version which introduced the issue.

I know, for fact, that systemd_215-5 does not have this isssue.

@Frans: Could you fetch systemd_215-5 for sparc64 from snapshots.debian.org and extract libsystemd.so from it to attach it to this bug tracker? I'm currently sitting at the airport on my way to Japan and I only have my phone at the moment.

systemd_215-5 was compiled with gcc-4.9 while 226-3 was compiled with gcc-5, so it might be a regression introduced with gcc-5.

Again, thanks everyone for the help so far. I am currently working to bring Debian's sparc ports (both 32- and 64-bit), and I need your help to resolve any toolchain issues first.

Adrian
Comment 6 Jose E. Marchesi 2015-10-02 13:48:06 UTC
This could still be a bug in either binutils or GCC.  However, I have bootstrapped GCC 5 (also svn trunk GCC) with the latest binutils in sparc64 and have never seen this issue.
Comment 7 Frans van Berckel 2015-10-02 14:19:26 UTC
Created attachment 8673 [details]
libsystemd.so.0.3.1

File: /lib/sparc64-linux-gnu/libsystemd.so.0.3.1
Extracted: libsystemd0_215-5_sparc64.deb
Origin: http://snapshot.debian.org/archive/debian-ports/20150101T014653Z/pool-sparc64/main/s/systemd/
Zip: by gzip < libsystemd.so.0.3.1 >libsystemd.so.0.3.1.gz
Comment 8 Jose E. Marchesi 2015-10-02 14:33:32 UTC
libsystemd.so.0.3.1 contains the expected STT_SPARC_REGISTER entries, for %g2 and %g3:

     4: 0000000000000002     0 REGISTER GLOBAL DEFAULT  UND
     5: 0000000000000003     0 REGISTER GLOBAL DEFAULT  UND
Comment 9 Michael Karcher 2015-10-02 20:58:29 UTC
With access to a Debian build box, I found out that the broken shared library is created by LTO-linking with gold. The invocation is basically the following gcc command.

gcc -shared -fPIC $(OBJECTS) -flto -O2 -Wl,--gc-sections -Wl,--as-needed -Wl,-z -Wl,relro -Wl,-z -Wl,now -Wl,-fuse-ld=gold -Wl,--version-script=../src/libsystemd/libsystemd.sym -Wl,-z -Wl,relro   -pthread -Wl,-soname -Wl,libsystemd.so.0 -o .libs/libsystemd.so.0.11.0

If I omit "-Wl,-fuse-ld=gold", I obtain a correct libsystemd.so.0.11.0:

$ readelf -s .libs/libsystemd.so.0.11.0  | grep REGISTER
     4: 0000000000000002     0 REGISTER GLOBAL DEFAULT  UND 
     5: 0000000000000003     0 REGISTER GLOBAL DEFAULT  UND 
  2007: 0000000000000002     0 REGISTER GLOBAL DEFAULT  UND 
  2008: 0000000000000003     0 REGISTER GLOBAL DEFAULT  UND 

Using gcc --save-temps, I obtained the LTO-generated assembler files, assembled them using

as -s -K PIC -Av9 -64 -no-undeclared-regs -relax -o $x.o $x

objdumping those files shows only REGISTER declarations for %g2 and %g3, which seem fine, linking them with gold results in a broken output file. I will attach the object files.
Comment 10 Michael Karcher 2015-10-02 21:01:16 UTC
Created attachment 8676 [details]
machine object files created during LTO link

Attached: seemingly correct object files resulting in the broken shared object.

(sid-sparc64-sbuild)mkarcher@ravirin:~/systemd-226/build-deb$ for x in libsystemd.so.0.11.0.ltrans*.s.o; do readelf -s $x | grep REGISTER ; done 
    65: 0000000000000002     0 REGISTER GLOBAL DEFAULT  UND 
    66: 0000000000000003     0 REGISTER GLOBAL DEFAULT  UND 
[...]
    27: 0000000000000002     0 REGISTER GLOBAL DEFAULT  UND 
    28: 0000000000000003     0 REGISTER GLOBAL DEFAULT  UND 
(sid-sparc64-sbuild)mkarcher@ravirin:~/systemd-226/build-deb$ gold -shared lib*ltrans*s.o -o temp.so
(sid-sparc64-sbuild)mkarcher@ravirin:~/systemd-226/build-deb$ readelf -s temp.so  | grep REGISTER
  1379: 0000000000000000     0 REGISTER GLOBAL DEFAULT  UND 
(sid-sparc64-sbuild)mkarcher@ravirin:~/systemd-226/build-deb$ tar -cvjf intermediates.tar.bz2 lib*ltrans*.s.o
Comment 11 Michael Karcher 2015-10-03 22:10:34 UTC
Created attachment 8679 [details]
Minimal gold testcase for STT_REGISTER relocations

No need to use any files from libsystemd. This is a generic gold issue, minimal testcase attached.
Comment 12 Michael Karcher 2015-10-03 22:15:49 UTC
Created attachment 8680 [details]
Minimal gold testcase for STT_REGISTER relocations (fixed "written by" line)

The first version contained incorrect copy-pasta in the "written by" line in the (trivial) test shell script. Sorry for that.
Comment 13 Cary Coutant 2015-10-06 19:50:22 UTC
I'm having trouble understanding what an STT_SPARC_REGISTER symbol is supposed to be processed. There's nothing in the SPARC Processor Supplement (Third Edition), dated 1996, that mentions STT_SPARC_REGISTER. I found a SPARC Compliance Definition (SCD 2.4), dated 1998, that mentions it, but it's under an "Experimental" heading. It says that the symbol should be SHN_ABS if the module initializes the register, and SHN_UNDEF otherwise.

I presume that an UNDEF REGISTER symbol needs to have the st_value preserved when linking, but that's not clear from the SCD, and it's also not clear whether symbols with the same st_value field (i.e., register number) should be bound together. Should two SHN_ABS REGISTER symbols with the same number produce an error?

Currently, the only thing gold does with STT_SPARC_REGISTER is suppress an undefined symbol warning, with the following comment:

    // XXX Really need to support this better...
    if (sym->type() == elfcpp::STT_SPARC_REGISTER)
      return 1;

If my presumptions are close, it sounds like we'll need a new target hook, because most of this special treatment is probably going to need to be in the non-target-specific parts of gold.

Can someone point me to an up-to-date version of the SPARC psABI?
Comment 14 Jose E. Marchesi 2015-10-06 20:08:29 UTC
The latest version of the psABI is sparc.org/wp-content/uploads/2014/01/SCD.2.4.1.pdf.gz for 64 bits, and psABI31 for 32 bits.  These are old, so you can basically ignore the "experimental" remarks.  I am working internally at Oracle to get the psABI updated, expanded (it is time, right :)) and unified in a single document.

So, lacking a normative reference I would basically mimic what ld/bfd does to handle STT_REGISTER symbols.  There are several hooks in BFD used by ld to add and merge that stuff.
Comment 15 Frans van Berckel 2015-10-06 20:14:39 UTC
Do not know it's useful for gold. This a old mail (1999) from binutils mailing list about getting 'Support for SPARC STT_REGISTER' ...

https://sourceware.org/ml/binutils/1999-q3/msg00029.html
Comment 16 Jose E. Marchesi 2015-10-06 20:25:31 UTC
Looks like davem already tried to add proper support for STT_REGISTER to gold: https://sourceware.org/ml/binutils/2008-04/msg00286.html
Comment 17 Jose E. Marchesi 2015-10-12 10:10:56 UTC
Cary, are you still working on this issue?  Otherwise I will jump in and try to hack the needed support in gold...
Comment 18 Cary Coutant 2015-10-13 23:31:03 UTC
Yes, I'm working on it. I think it's going to need some additional
work in non-target-specific code to support this feature.

-cary


On Mon, Oct 12, 2015 at 3:10 AM, jose.marchesi at oracle dot com
<sourceware-bugzilla@sourceware.org> wrote:
> https://sourceware.org/bugzilla/show_bug.cgi?id=19019
>
> --- Comment #17 from Jose E. Marchesi <jose.marchesi at oracle dot com> ---
> Cary, are you still working on this issue?  Otherwise I will jump in and try to
> hack the needed support in gold...
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
Comment 19 Jose E. Marchesi 2016-01-14 13:04:40 UTC
Hi Cary.  Any progress on this?
Comment 20 Cary Coutant 2016-01-15 07:14:48 UTC
(In reply to Jose E. Marchesi from comment #19)
> Hi Cary.  Any progress on this?

I have a fix in progress, but I'm not quite satisfied with it. The Target::make_symbols() hook needs a few parameters, so that the target-specific code can keep track of the register symbols, and we need a new hook at Symbol_table::finalize() time to allow the target to insert the register symbols on output.

I'll be out of the country for the next week, but I'll finish this up as soon as I get back.
Comment 21 Sourceware Commits 2016-03-04 02:22:43 UTC
The master branch has been updated by Cary Coutant <ccoutant@sourceware.org>:

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

commit dc1c8a16a38dec431c77f49cf50a9b62d6366138
Author: Cary Coutant <ccoutant@gmail.com>
Date:   Thu Feb 4 16:55:39 2016 -0800

    Add support for STT_SPARC_REGISTER symbols.
    
    gold/
    	PR gold/19019
    	* layout.h (Layout::add_target_specific_dynamic_tag): New function.
    	* layout.cc (Layout::add_target_specific_dynamic_tag): New function.
    	* mips.cc (Target_mips::make_symbol): Adjust function signature.
    	* sparc.cc (Target_sparc::Target_sparc): Initialize register_syms_.
    	(Target_sparc::do_is_defined_by_abi): Remove test for
    	STT_SPARC_REGISTER.
    	(Target_sparc::Register_symbol): New struct type.
    	(Target_sparc::register_syms_): New data member.
    	(Target_sparc<64, true>::sparc_info): Set has_make_symbol to true.
    	(Target_sparc::make_symbol): New function.
    	(Target_sparc::do_finalize_sections): Add register symbols and new
    	dynamic table entries.
    	* symtab.h (Sized_symbol::init_undefined): Add value parameter.
    	(Symbol_table::add_target_global_symbol): New function.
    	(Symbol_table::target_symbols_): New data member.
    	* symtab.cc (Sized_symbol::init_undefined): Add value parameter.
    	(Symbol_table::Symbol_table): Initialize target_symbols_.
    	(Symbol_table::add_from_object): Pass additional parameters to
    	Target::make_symbol.
    	(Symbol_table::define_special_symbol): Likewise.
    	(Symbol_table::add_undefined_symbol_from_command_line): Pass 0 for
    	undefined symbol value.
    	(Symbol_table::set_dynsym_indexes): Process target-specific symbols.
    	(Symbol_table::sized_finalize): Likewise.
    	(Symbol_table::sized_write_globals): Likewise.
    	* target.h (Sized_target::make_symbol): Add name, st_type, object,
    	st_shndx, and value parameters.
Comment 22 Sourceware Commits 2016-03-04 16:14:18 UTC
The master branch has been updated by Cary Coutant <ccoutant@sourceware.org>:

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

commit 8d04e81db4967fa1ea2897bb52ccb21e17d9ed91
Author: Cary Coutant <ccoutant@gmail.com>
Date:   Fri Mar 4 08:10:57 2016 -0800

    Fix undefined symbol errors introduced with previous commit.
    
    gold/
    	PR gold/19019
    	PR gold/19763
    	* symtab.cc: Instantiate Sized_symbol::init_constant and
    	Sized_symbol::init_undefined.
Comment 23 Cary Coutant 2016-03-21 20:59:53 UTC
Should be fixed. Please reopen if you find any further issues with STT_SPARC_REGISTER.