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

x/crypto/sha3: build failure on linux/s390x at Go master due to illegal instruction #36459

Closed
toothrot opened this issue Jan 8, 2020 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@toothrot
Copy link
Contributor

toothrot commented Jan 8, 2020

https://build.golang.org/log/b664bfe4716a53a4766a8340a2e54fb3673cdca9

SIGILL: illegal instruction
PC=0x149156 m=6 sigcode=2

goroutine 9 [running]:
golang.org/x/crypto/sha3.kimd(0x20, 0xc0000ea380, 0xc00011c000, 0x10000, 0x10000)
	/data/golang/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_s390x.s:17 +0x16 fp=0xc00006fae8 sp=0xc00006fae8 pc=0x149156
golang.org/x/crypto/sha3.(*asmState).Write(0xc0000ea380, 0xc00011c000, 0x10000, 0x10000, 0x0, 0x0, 0x0)
	/data/golang/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_s390x.go:116 +0x1fe fp=0xc00006fb48 sp=0xc00006fae8 pc=0x14246e
golang.org/x/crypto/sha3.TestUnalignedWrite.func1(0x19d4b0, 0x7)
	/data/golang/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:198 +0x2d4 fp=0xc00006ff48 sp=0xc00006fb48 pc=0x147a74
golang.org/x/crypto/sha3.testUnalignedAndGeneric(...)
	/data/golang/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:81
golang.org/x/crypto/sha3.TestUnalignedWrite(0xc00009a5a0)
	/data/golang/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:193 +0x94 fp=0xc00006ff80 sp=0xc00006ff48 pc=0x144864
testing.tRunner(0xc00009a5a0, 0x1a82e0)
	/data/golang/workdir/go/src/testing/testing.go:954 +0xd8 fp=0xc00006ffc8 sp=0xc00006ff80 pc=0xe4a88
runtime.goexit()
	/data/golang/workdir/go/src/runtime/asm_s390x.s:779 +0x2 fp=0xc00006ffc8 sp=0xc00006ffc8 pc=0x79f32
created by testing.(*T).Run
	/data/golang/workdir/go/src/testing/testing.go:1005 +0x34a

goroutine 1 [chan receive]:
testing.(*T).Run(0xc00009a5a0, 0x19fa66, 0x12, 0x1a82e0, 0x1000000000e4a3a)
	/data/golang/workdir/go/src/testing/testing.go:1006 +0x36a
testing.runTests.func1(0xc00009a000)
	/data/golang/workdir/go/src/testing/testing.go:1247 +0x88
testing.tRunner(0xc00009a000, 0xc000070e00)
	/data/golang/workdir/go/src/testing/testing.go:954 +0xd8
testing.runTests(0xc00000c060, 0x2c6de0, 0x8, 0x8, 0x0)
	/data/golang/workdir/go/src/testing/testing.go:1245 +0x2a6
testing.(*M).Run(0xc000096000, 0x0)
	/data/golang/workdir/go/src/testing/testing.go:1162 +0x17c
main.main()
	_testmain.go:84 +0x142

r0   0x20	r1   0xc0000ea380
r2   0xc00011c000	r3   0x10000
r4   0x10000	r5   0xc00011c000
r6   0x10000	r7   0x90
r8   0x0	r9   0xd00
r10  0x0	r11  0x0
r12  0x1c	r13  0xc000001200
r14  0x14246e	r15  0xc00006fae8
pc   0x149156	link 0x14246e
FAIL	golang.org/x/crypto/sha3	0.054s

/cc @FiloSottile @katiehockman

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 8, 2020
@toothrot toothrot added this to the Unreleased milestone Jan 8, 2020
@bradfitz
Copy link
Contributor

bradfitz commented Jan 8, 2020

@mundaym, is this related of moving the s390x builder to z15?

@FiloSottile FiloSottile changed the title x/crypto: build failure on linux/s390x at Go master due to illegal instruction x/crypto/sha3: build failure on linux/s390x at Go master due to illegal instruction Jan 8, 2020
@mundaym
Copy link
Member

mundaym commented Jan 8, 2020

@mundaym, is this related of moving the s390x builder to z15?

Probably, I'll take a look.

@mundaym mundaym self-assigned this Jan 8, 2020
@gopherbot
Copy link

Change https://golang.org/cl/213857 mentions this issue: sha3: fix SHA-3 on s390x when using KIMD instruction

@mundaym
Copy link
Member

mundaym commented Jan 8, 2020

The SIGILL was due to a bug in the way I rounded down the length of an input to a multiple of the rate suitable for the KIMD instruction. It occurs when more than 3KiB of data is written to the hash at once and the size is not a multiple of the rate. The KIMD instruction doesn't have SHA-3 support on z13 (the old buildlet machine) and the test that is failing now was added after the assembly implementation of SHA-3 was added which is why we didn't see this before.

The fix is simple but generally we should probably add better testing for generic vs. assembly implementations both for amd64 and s390x. That will take me a little while.

@mundaym
Copy link
Member

mundaym commented Jan 8, 2020

(Note that KIMD won't give an incorrect result, it will always just SIGILL.)

@golang golang locked and limited conversation to collaborators Jan 7, 2021
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
An illegal instruction would occur due to a bug in the way input
slices were rounded down in size to a multiple of the rate for a
given hash type. This would only occur when the Write function
was called with more than ~3KiB of data and the length of the data
was not a multiple of the rate.

Fixes golang/go#36459.

Change-Id: I621ef8d75602bcd59bb44491e17f721050001e6d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/213857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
An illegal instruction would occur due to a bug in the way input
slices were rounded down in size to a multiple of the rate for a
given hash type. This would only occur when the Write function
was called with more than ~3KiB of data and the length of the data
was not a multiple of the rate.

Fixes golang/go#36459.

Change-Id: I621ef8d75602bcd59bb44491e17f721050001e6d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/213857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
An illegal instruction would occur due to a bug in the way input
slices were rounded down in size to a multiple of the rate for a
given hash type. This would only occur when the Write function
was called with more than ~3KiB of data and the length of the data
was not a multiple of the rate.

Fixes golang/go#36459.

Change-Id: I621ef8d75602bcd59bb44491e17f721050001e6d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/213857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc rsc unassigned mundaym Jun 23, 2022
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
An illegal instruction would occur due to a bug in the way input
slices were rounded down in size to a multiple of the rate for a
given hash type. This would only occur when the Write function
was called with more than ~3KiB of data and the length of the data
was not a multiple of the rate.

Fixes golang/go#36459.

Change-Id: I621ef8d75602bcd59bb44491e17f721050001e6d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/213857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
An illegal instruction would occur due to a bug in the way input
slices were rounded down in size to a multiple of the rate for a
given hash type. This would only occur when the Write function
was called with more than ~3KiB of data and the length of the data
was not a multiple of the rate.

Fixes golang/go#36459.

Change-Id: I621ef8d75602bcd59bb44491e17f721050001e6d
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/213857
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants