-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: corrupted value #20530
Comments
I think the problem is these rules:
The MOVBQZX is merged into the MOVBload, which is then replaced with a copy. (s390x had the same problem which was fixed in 094992e (CL 37154). That CL hasn't been backported to 1.8.x.) |
CL https://golang.org/cl/44355 mentions this issue. |
This bug exists in 1.7 and 1.8, but not 1.6. |
And also x86:
and ARM:
So I'd predict some likely failures in the trybots with my shiny new test. :-) |
PPC64 lacks the pattern and the bug also does not reproduce there. |
Reopening for 1.8.4. |
A slight variation of the test case can trigger the failure on ARM. The CL fixed it.
It also fails on MIPS. CL coming. |
CL https://golang.org/cl/44430 mentions this issue. |
Apply the fix in CL 44355 to MIPS. ARM64 has these rules but commented out for performance reason. Fix the commented rules, in case they are enabled in the future. Enhance the test so it triggers the failure on ARM and MIPS without the fix. Updates #20530. Change-Id: I82d77448e3939a545fe519d0a29a164f8fa5417c Reviewed-on: https://go-review.googlesource.com/44430 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
What's the 1.8.4 process after reopening? |
Just leave it for now. When we get to doing a 1.8.4 release, all the CLs attached to 1.8.4 marked bugs will be backported to the release branch. |
I believe this is fixed in 1.9. Someone please cross-check and close. |
This is kept open for Go 1.8.4, if there is one. |
CL 37154 OK for Go 1.8.5 (after CL 41395). |
Change https://golang.org/cl/70840 mentions this issue: |
Change https://golang.org/cl/70841 mentions this issue: |
…r amd64, x86, arm Replacing byteload-of-bytestore-of-x with x is incorrect when x contains a larger-than-byte value (and so on for 16 and 32-bit load/store pairs). Replace "x" with the appropriate zero/sign extension of x, which if unnecessary will be repaired by other rules. Made logic for arm match x86 and amd64; yields minor extra optimization, plus I am (much) more confident it's correct, despite inability to reproduce bug on arm. Ppc64 lacks this optimization, hence lacks this problem. See related https://golang.org/cl/37154/ Fixes #20530. [Merge conflicts in generated rewrite files resolved by regenerating from scratch, using the programs in ssa/gen.] Change-Id: I6af9cac2ad43bee99cafdcb04725ce7e55a43323 Reviewed-on: https://go-review.googlesource.com/44355 Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-on: https://go-review.googlesource.com/70841 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
…r MIPS Apply the fix in CL 44355 to MIPS. ARM64 has these rules but commented out for performance reason. Fix the commented rules, in case they are enabled in the future. Enhance the test so it triggers the failure on ARM and MIPS without the fix. Updates #20530. Change-Id: I82d77448e3939a545fe519d0a29a164f8fa5417c Reviewed-on: https://go-review.googlesource.com/44430 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-on: https://go-review.googlesource.com/70840 Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
go1.8.5 has been packaged and includes: The release is posted at golang.org/dl. — golang.org/x/build/cmd/releasebot, Oct 26 21:07:48 UTC |
What version of Go are you using (
go version
)?go version go1.8.3 linux/amd64
What operating system and processor architecture are you using (
go env
)?What did you do?
Playground
What did you expect to see?
Nothing
What did you see instead?
got 255 expected 255
Additional info
//println(a)
) "fixes" the problem.The text was updated successfully, but these errors were encountered: