Bug 1460254 - golang 1.8.1 fails to build oci-register-machine
golang 1.8.1 fails to build oci-register-machine
Status: MODIFIED
Product: Fedora
Classification: Fedora
Component: binutils (Show other bugs)
27
s390x Linux
unspecified Severity unspecified
: ---
: ---
Assigned To: Nick Clifton
Fedora Extras Quality Assurance
: Patch
: 1442023 (view as bug list)
Depends On: 1472486
Blocks: ZedoraTracker 1466865
  Show dependency treegraph
 
Reported: 2017-06-09 09:41 EDT by Dan Horák
Modified: 2017-08-15 03:28 EDT (History)
17 users (show)

See Also:
Fixed In Version: binutils-2.28-11.fc27
Doc Type: Bug Fix
Doc Text:
Cause: The s390 assembler would emit fake local labels whose name is L0^A. Consequence: The presence of a control character (ASCII code 001) in a symbol name was confusing the golang build machinery. Fix: Stop the s390 assembler from emitting these symbols. Result: The golang build machinery works.
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
IBM Linux Technology Center 155900 None None None 2017-06-21 11:24 EDT

  None (edit)
Description Dan Horák 2017-06-09 09:41:22 EDT
golang 1.8.1 fails to rebuild the oci-register-machine package due

...
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.u0ec4f
+ umask 022
+ cd /builddir/build/BUILD
+ cd oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1
+ mkdir -p src/github.com/projectatomic
+ ln -s ../../../ src/github.com/projectatomic/oci-register-machine
++ pwd
+ export GOPATH=/builddir/build/BUILD/oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1:/usr/share/gocode
+ GOPATH=/builddir/build/BUILD/oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1:/usr/share/gocode
+ make -j2
GOPATH=$GOPATH:/usr/share/gocode go build -a -ldflags " -B 0x77a6d4ab6a395ddbb703bbeb833479ff025e62b2" -o oci-register-machine
go-md2man -in "oci-register-machine.1.md" -out "oci-register-machine.1"
sed -i 's|$HOOKSDIR|/usr/libexec/oci/hooks.d|' oci-register-machine.1
# _/builddir/build/BUILD/oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1
L0: $WORK/runtime/cgo.a(_all.o): sym#625: ignoring symbol in section 17 (type 0)
L0: $WORK/runtime/cgo.a(_all.o): sym#626: ignoring symbol in section 17 (type 0)
L0: $WORK/runtime/cgo.a(_all.o): sym#627: ignoring symbol in section 17 (type 0)
RPM build errors:
make: *** [Makefile:25: oci-register-machine] Error 2


for full logs see https://koji.fedoraproject.org/koji/taskinfo?taskID=19927556

can be reproduced locally (after "fedpkg prep") with
[sharkcz@devel3 oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1]$ export GOPATH=/home/sharkcz/oci-register-machine/oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1:/usr/share/gocode
[sharkcz@devel3 oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1]$ make build
GOPATH=$GOPATH:/usr/share/gocode go build -a -ldflags " -B 0xfa783c231ce95bd7ebbc24bade68c14aeaf7f94e" -o oci-register-machine
# _/home/sharkcz/oci-register-machine/oci-register-machine-bb20b00165c008f473ee09163cdb7db66faa00b1
L0: $WORK/runtime/cgo.a(_all.o): sym#624: ignoring symbol in section 17 (type 0)
L0: $WORK/runtime/cgo.a(_all.o): sym#625: ignoring symbol in section 17 (type 0)
L0: $WORK/runtime/cgo.a(_all.o): sym#626: ignoring symbol in section 17 (type 0)
make: *** [Makefile:25: oci-register-machine] Error 2



Version-Release number of selected component (if applicable):
golang-1.8.1-2.fc26.s390x
golang-1.8.1-2.fc27.s390x

Additional information:
Seems the previous build in s390 koji using golang-1.8-0.rc3.2.fc26 .s390x was successful.
Comment 1 Dan Horák 2017-06-13 06:10:33 EDT
So it's not golang, but binutils, there is no error with binutils-2.28-2.fc27.s390x and the issue was introduced with http://pkgs.fedoraproject.org/cgit/rpms/binutils.git/commit/?id=416e014455167094f9d8f8890e6ce31c0a48e4a4

And it's most likely a dupe of #1442023 with much smaller reproducer.
Comment 2 Dan Horák 2017-06-30 11:42:01 EDT
*** Bug 1442023 has been marked as a duplicate of this bug. ***
Comment 3 Carlos O'Donell 2017-07-12 15:37:26 EDT
(In reply to Dan Horák from comment #1)
> So it's not golang, but binutils, there is no error with
> binutils-2.28-2.fc27.s390x and the issue was introduced with
> http://pkgs.fedoraproject.org/cgit/rpms/binutils.git/commit/
> ?id=416e014455167094f9d8f8890e6ce31c0a48e4a4
> 
> And it's most likely a dupe of #1442023 with much smaller reproducer.

Jakub and I were debugging this and the 3 symbols produced look wrong (they have a corrupt looking identical name of 'L0<U0001>A'), so while this is indeed a binutils issue, Jakub has chosen to work around this by ignoring the 3 new incorrectly named symbols.

commit 5767354120e6457ba9b75f1266e8cc55f2307b03
Author: Jakub Čajka <jcajka@redhat.com>
Date:   Wed Jul 12 13:38:36 2017 +0200

    Bump to 1.9beta2
    Workaround build failure on s390x

commit 3cc854f4230879f9513708723bcc0b1e101abf7d
Author: Jakub Čajka <jcajka@redhat.com>
Date:   Wed Jul 12 17:26:26 2017 +0200

    Improved s390x patch

e.g.

if sect.name == ".debug_str" && sym.name == "L0^A" && sym.type_ == 0 {
   ... ignore the symbol
}

Not that the ^ is actually <U0001>.
Comment 4 Carlos O'Donell 2017-07-12 19:32:50 EDT
(In reply to Carlos O'Donell from comment #3)
> if sect.name == ".debug_str" && sym.name == "L0^A" && sym.type_ == 0 {
>    ... ignore the symbol
> }
> 
> Not that the ^ is actually <U0001>.

DJ Delorie just told me that binutils uses ^A in temporary symbols which are supposed to filled in later. I assume that what's going on here is that s390x is missing some of the generic processing hooks to turn these 3 temporary symbols into the appropriate symbols. So someone IMO will have to look at the s390x backend to see what's missing.
Comment 5 Carlos O'Donell 2017-07-12 19:35:45 EDT
I'm reassigning this to binutils to see if Nick has any input on the problem.

We don't have a reproducer except the Go build, someone would have to figure out the link step which generates the bad object file and put together a test case.

However, we might as well ask Nick if he's seen any other problems with s390x.
Comment 6 Carlos O'Donell 2017-07-12 19:36:12 EDT
Any thoughts Nick?
Comment 7 Nick Clifton 2017-07-13 04:14:55 EDT
Right - so ^A in a symbol name would normally indicate the use of a dollar local label in the assembler sources:

  Dollar Local Labels
  -------------------

  On some targets `as' also supports an even more local form of local
  labels called dollar labels.  These labels go out of scope (i.e., they
  become undefined) as soon as a non-local label is defined.  Thus they
  remain valid for only a small region of the input source code.  Normal
  local labels, by contrast, remain in scope for the entire file, or
  until they are redefined by another occurrence of the same local label.

  Dollar labels are defined in exactly the same way as ordinary local
  labels, except that they have a dollar sign suffix to their numeric
  value, e.g., `55$:'.

  They can also be distinguished from ordinary local labels by their
  transformed names which use ASCII character `\001' (control-A) as the
  magic character to distinguish them from ordinary labels.  For example,
  the fifth definition of `6$' may be named `.L6C-A5'.

However this feature may not be to blame in this case since dollar local labels all have the form of L<number>^A<number> and the symbols being matched by the code in comment #3 do not conform to this template.

I think instead that the problem is most likely that the code added by the binutils-2.28-gas-comp_dir.patch is buggy.  I suspect that it is using a buffer
for a symbol name that is either freed before use or held on the stack and corrupted via stack unwinding.  It is likely that running the assembler with address sanitization enabled would detect this.

I am not sure why the problem should be specific to the s390x, since the code in the patch is generic, and the s390x assembler backend does not do anything particularly funny.  If there was a small testcase I would like to investigate further.  (By small I am hoping for something like an assembler source file and command line, which I can then test using an s390 cross assembler.  Finding an s390 machine with f26 installed would be a bit of a pain).

Cheers
  Nick
Comment 9 Jakub Čajka 2017-07-13 04:55:58 EDT
Reported to golang upstream https://github.com/golang/go/issues/20996 (before seeing the recent development), will link this bug in.
Comment 12 IBM Bug Proxy 2017-07-14 03:30:40 EDT
------- Comment From Andreas.Krebbel@de.ibm.com 2017-07-14 03:20 EDT-------
(In reply to comment #10)
> I am not sure why the problem should be specific to the s390x, since the
> code in the patch is generic, and the s390x assembler backend does not do
> anything particularly funny.  If there was a small testcase I would like to
> investigate further.  (By small I am hoping for something like an assembler
> source file and command line, which I can then test using an s390 cross
> assembler.  Finding an s390 machine with f26 installed would be a bit of a
> pain).

The S/390 Binutils backend used to keep the internal labels in debug sections as distinct symbols instead of making them section relative. You have fixed this recently:  https://sourceware.org/bugzilla/show_bug.cgi?id=21333

This probably explains why:

1. The problem appeared to be S/390 specific. Other targets already did that.
2. The problem goes away with more recent Binutils versions.

------- Comment From Andreas.Krebbel@de.ibm.com 2017-07-14 03:29 EDT-------
So a "fix" for older Binutils versions might be to backport:

commit dc1e4d6dedcb8ee3bb195f0db711f6aa164b8ab4
Author: Nick Clifton <nickc@redhat.com>
Date:   Fri Mar 31 12:54:38 2017 +0100

Reduce the size of s390 symbol tables by allowing relocations in mergeable string sections (eg .debug_str) to be made section relative rather than symbol relative.

PR gas/21333
* config/tc-s390.c (tc_s390_fix_adjustable): Allow non pc-relative
fixups in mergeable sections to be adjusted.

However, this might just hide a problem with the common code patch by getting rid of the problematic symbols. It would be better to track down what caused the symbol names to be garbled.
Comment 15 Jakub Čajka 2017-07-14 09:39:57 EDT
For the record, workaround(ignoring the symbols) to keep building of 390x on is now part of golang-1.8.3-1.fc26 and golang-1.9-0.beta2.1.fc27
Comment 16 Jakub Čajka 2017-07-14 10:34:59 EDT
(In reply to Jakub Čajka from comment #15)
> For the record, workaround(ignoring the symbols) to keep building of 390x on
> is now part of golang-1.8.3-1.fc26 and golang-1.9-0.beta2.1.fc27
Correction golang-1.8.3-2.fc26
Comment 17 Nick Clifton 2017-07-17 12:44:10 EDT
Found it.  The patch mentioned by Andreas on comment #12 was the clue.  What happens is that the code added by the gas-comp-dir.patch generates fake local labels (whose name is .L0^A - but I had forgotten this), and then it expects the backend to convert these into section relative offsets.  Most targets do this, but for some reason the s390 does not.  (I believe that this is a snafu on the part of the s390 backend.  There is no good reason not to convert these labels).

The patch from comment #12 had the side effect of fixing this bug, because it allows non-pc-relative symbols in the .debug_str section to be adjusted, and
these fake local label just happen to match that criteria.  But it does not fix the broader problem of ensuring that all fake local labels are converted.  So I have added an extra patch to the rawhide binutils sources to do this, and I will also commit a patch to the FSF mainline sources as well.

Cheers
  Nick
Comment 18 IBM Bug Proxy 2017-07-18 02:20:25 EDT
------- Comment From Andreas.Krebbel@de.ibm.com 2017-07-18 02:17 EDT-------
(In reply to comment #17)
> Found it.  The patch mentioned by Andreas on comment #12 was the clue.  What

Great! Thanks for tracking this down!

-Andreas-
Comment 19 Dan Horák 2017-07-18 06:09:46 EDT
Nick, please put the same fix into F-26 too.
Comment 20 Nick Clifton 2017-07-18 06:51:33 EDT
(In reply to Dan Horák from comment #19)

> Nick, please put the same fix into F-26 too.

Done.  Please try:

 binutils-2.27-25.fc26

Cheers
  Nick
Comment 21 Patrick Uiterwijk 2017-07-18 17:41:50 EDT
The patch for this caused bug #1472486.
Comment 22 Jakub Čajka 2017-07-19 04:11:55 EDT
Verified in rawhide, it resolves the golang issue, thanks for quick fix :).
Workaround reverted in all dist-git branches, waiting for next rebuild.
Comment 23 Dan Horák 2017-07-19 05:19:57 EDT
FYI binutils-2.28-11.fc27 got untagged, because it's causing bug #1472486
Comment 24 Patrick Uiterwijk 2017-07-19 12:51:15 EDT
If anything was built with binutils-2.28-11.fc27, that should be considered for a rebuild, as the binaries might be broken on s390x.
Comment 25 Jan Kurik 2017-08-15 03:28:44 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Note You need to log in before you can comment on or make changes to this bug.