Bug 1460254

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.

*** Bug 1442023 has been marked as a duplicate of this bug. ***

(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>
Date:   Wed Jul 12 13:38:36 2017 +0200

    Bump to 1.9beta2
    Workaround build failure on s390x

commit 3cc854f4230879f9513708723bcc0b1e101abf7d
Author: Jakub Čajka <jcajka>
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>.

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

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.

Any thoughts Nick?

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

Reported to golang upstream https://github.com/golang/go/issues/20996 (before seeing the recent development), will link this bug in.

------- Comment From Andreas.Krebbel.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.com 2017-07-14 03:29 EDT-------
So a "fix" for older Binutils versions might be to backport:

commit dc1e4d6dedcb8ee3bb195f0db711f6aa164b8ab4
Author: Nick Clifton <nickc>
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.

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

(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

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 From Andreas.Krebbel.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-

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

(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

The patch for this caused bug #1472486.

Verified in rawhide, it resolves the golang issue, thanks for quick fix :).
Workaround reverted in all dist-git branches, waiting for next rebuild.

FYI binutils-2.28-11.fc27 got untagged, because it's causing bug #1472486

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.

This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

This message is a reminder that Fedora 27 is nearing its end of life.
On 2018-Nov-30  Fedora will stop maintaining and issuing updates for
Fedora 27. It is Fedora's policy to close all bug reports from releases
that are no longer maintained. At that time this bug will be closed as
EOL if it remains open with a Fedora  'version' of '27'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 27 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Fedora 27 changed to end-of-life (EOL) status on 2018-11-30. Fedora 27 is
no longer maintained, which means that it will not receive any further
security or bug fix updates. As a result we are closing this bug.

If you can reproduce this bug against a currently maintained version of
Fedora please feel free to reopen this bug against that version. If you
are unable to reopen this bug, please file a new report against the
current release. If you experience problems, please add a comment to this
bug.

Thank you for reporting this bug and we are sorry it could not be fixed.