Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

strings: SIGILL on s390x on z196 #17032

Closed
mwhudson opened this issue Sep 8, 2016 · 10 comments
Closed

strings: SIGILL on s390x on z196 #17032

mwhudson opened this issue Sep 8, 2016 · 10 comments
Labels
FrozenDueToAge
Milestone
Go1.7.3

Comments

@mwhudson
Copy link
Contributor

mwhudson commented Sep 8, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.7 or 1.7.1

What operating system and processor architecture are you using (go env)?

unname -a says "Linux zelenka 3.16.0-4-s390x #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) s390x GNU/Linux". This has been seen on https://db.debian.org/machines.cgi?host=zelenka and https://db.debian.org/machines.cgi?host=zandonai, a Debian porter and build box respectively. /proc/cpuinfo has "processor 0: version = FF, identification = 06A18A, machine = 2817".

It doesn't happen on a z13 running Ubuntu 16.10.

What did you do?

./all.bash

What did you expect to see?

tests passing

What did you see instead?

SIGILL: illegal instruction
PC=0x755be m=4

goroutine 52 [running]:
runtime.checkvectorfacility()
    /«PKGBUILDDIR»/src/runtime/asm_s390x.s:35 +0x4e fp=0xc42003cd78 sp=0xc42003cd50
created by testing.(*T).Run
    /«PKGBUILDDIR»/src/testing/testing.go:646 +0x376

goroutine 1 [chan receive]:
testing.(*T).Run(0xc42004acc0, 0x185eca, 0xf, 0x19e300, 0x10000000003f372)
    /«PKGBUILDDIR»/src/testing/testing.go:647 +0x3a0
testing.RunTests.func1(0xc420070000)
    /«PKGBUILDDIR»/src/testing/testing.go:793 +0xe6
testing.tRunner(0xc420070000, 0xc42003edb8)
    /«PKGBUILDDIR»/src/testing/testing.go:610 +0xd8
testing.RunTests(0x19db20, 0x238520, 0x31, 0x31, 0x100000000000c03)
    /«PKGBUILDDIR»/src/testing/testing.go:799 +0x3da
testing.(*M).Run(0xc42003ef08, 0xf)
    /«PKGBUILDDIR»/src/testing/testing.go:743 +0x88
main.main()
    strings/_test/_testmain.go:292 +0x14c

r0   0x1    r1   0x40
r2   0xc42003cda0   r3   0xc4202855c0
r4   0x11   r5   0x6e
r6   0xc4202855c0   r7   0x1
r8   0xc4202855d1   r9   0x6e
r10  0x40   r11  0x1b8cb54
r12  0x3fffd27faa0  r13  0xc420001380
r14  0x78afc    r15  0xc42003cd50
pc   0x755be    link 0x78afc
FAIL    strings 0.155s

(full log at https://buildd.debian.org/status/fetch.php?pkg=golang-1.7&arch=s390x&ver=1.7.1-1&stamp=1473301050)
@mwhudson
Copy link
Contributor Author

mwhudson commented Sep 8, 2016

paging @mundaym

@mwhudson
Copy link
Contributor Author

mwhudson commented Sep 8, 2016

Oh yes, we don't think this happened at the time of the 1.7 release, so it's possible the configuration of the machine has changed somehow recently?

@tianon
Copy link
Contributor

tianon commented Sep 8, 2016

(Although at the time, we were hitting #16780 as well, so tests were disabled during the package build on s390x and thus it's hard to say for sure whether our memory is accurate. 😇)

@mundaym
Copy link
Member

mundaym commented Sep 8, 2016

I think the memory needs to cleared before the STFLE instruction.

Do you have a z196 you can test on? Could you try the following patch?

--- a/src/runtime/asm_s390x.s
+++ b/src/runtime/asm_s390x.s
@@ -20,6 +20,7 @@ TEXT runtime·checkvectorfacility(SB),NOSPLIT,$32-0
        MOVD    $2, R0
        MOVD    R1, tmp-32(SP)
        MOVD    $x-24(SP), R1
+       XC      $24, 0(R1), 0(R1)
 //      STFLE   0(R1)
        WORD    $0xB2B01000
        MOVBZ   z-8(SP), R1

@tianon
Copy link
Contributor

tianon commented Sep 8, 2016

@mundaym that was incredibly fast 👍

I've applied that patch against 1.7.1 and tested on the zelenka machine @mwhudson referenced above, and the strings test now passes successfully (ok strings 0.147s). 🤘

@tianon
Copy link
Contributor

tianon commented Sep 8, 2016

For completeness I let the tests finish, and got all the way to ALL TESTS PASSED. 💚

@gopherbot
Copy link

CL https://golang.org/cl/28850 mentions this issue.

@mwhudson
Copy link
Contributor Author

mwhudson commented Sep 9, 2016

No Go1.7.2 milestone yet?

@bradfitz bradfitz added this to the Go1.7.2 milestone Sep 9, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Sep 9, 2016

Done.

@gopherbot
Copy link

CL https://golang.org/cl/31267 mentions this issue.

gopherbot pushed a commit that referenced this issue Oct 17, 2016
@mundaym @broady
[release-branch.go1.7] runtime: fix SIGILL in checkvectorfacility on …
230c391 
…s390x

STFLE does not necessarily write to all the double-words that are
requested. It is therefore necessary to clear the target memory
before calling STFLE in order to ensure that the facility list does
not contain false positives.

Fixes #17032.

Change-Id: I7bec9ade7103e747b72f08562fe57e6f091bd89f
Reviewed-on: https://go-review.googlesource.com/28850
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/31267
Reviewed-by: Michael Munday <munday@ca.ibm.com>
ceseo pushed a commit to powertechpreview/go that referenced this issue Dec 1, 2016
@mundaym @ceseo
[release-branch.go1.7] runtime: fix SIGILL in checkvectorfacility on …
8fa3873 
…s390x

STFLE does not necessarily write to all the double-words that are
requested. It is therefore necessary to clear the target memory
before calling STFLE in order to ensure that the facility list does
not contain false positives.

Fixes golang#17032.

Change-Id: I7bec9ade7103e747b72f08562fe57e6f091bd89f
Reviewed-on: https://go-review.googlesource.com/28850
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/31267
Reviewed-by: Michael Munday <munday@ca.ibm.com>
@golang golang locked and limited conversation to collaborators Oct 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7.3
Development

No branches or pull requests
5 participants
@bradfitz @tianon @mwhudson @gopherbot @mundaym