Bug 19019

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

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

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.

$ 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...

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

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.

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

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

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.

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

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.

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.

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?

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.

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

Looks like davem already tried to add proper support for STT_REGISTER to gold: https://sourceware.org/ml/binutils/2008-04/msg00286.html

Cary, are you still working on this issue?  Otherwise I will jump in and try to hack the needed support in gold...

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.

Hi Cary.  Any progress on this?

(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.

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.

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.

Should be fixed. Please reopen if you find any further issues with STT_SPARC_REGISTER.