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: unexpected string to byte casting behavior in 1.5 #12226
Comments
Interestingly, the problem can't be reproduced with s := "foo"
fmt.Println(len(s) == 0 || []byte(s)[0] == []byte{'#'}[0]) This correctly prints |
Also, the len() call seems to be irrelevant. This reproduces as well: fmt.Println([]byte("0")[0] == []byte("1")[0] || false) |
Following program reproduces the bug for me on AMD64:
It seems to be an issue with the register allocator, the compiler produces CMP BL, BL for the equality operator, which is always true. |
Smells like a compiler optimizer bug to me. |
cc @rsc @josharian |
Correction: Statement is go tool compile -N -S produces the same statement, so I suppose it is not the optimizer. |
Hi, not sure if this is relevant or not; apologies if it's noise. Did a bit of git bisect... Test program: package main
import (
"os"
)
func main() {
if []byte("0")[0] == []byte("1")[0] {
os.Exit(1)
}
} The first bad commit I can see for this program is b115c35. It causes a panic:
At a44bece the behaviour changes to no panic but an exit code of |
This diff fixes the bug for me:
|
Want to send a CL? That diff makes sense. |
I have sent the CL https://go-review.googlesource.com/#/c/13771/ (It is my first.) |
Should that test be an actual test in the CL instead of in the commit message? |
You are right. But before I can do that, I have to understand where and how to add the test. I will work on it. |
CL https://golang.org/cl/13771 mentions this issue. |
Thanks for that. The CL includes now test/fixedbugs/issue12226.go. |
Probably worth running @dvyukov's Gosmith 1.4.2 vs 1.5. Previously I think it was only run gc vs gccgo. |
No bug: foo doesn't start with b |
@myml: The original test program is definitely buggy on 1.5. Your test is different and it probably doesn't follow the same path in the code generation for the equal operator. |
@uli-go
and
on 1.5 get different results; |
@myml: We agree that your program is different. |
Why you censor my comments? |
Off-topic or uncivil comments adding no technical value to the discussion may be deleted. Please keep the discussions focused. If you want to discuss something unrelated, use the golang-nuts list, but keep it civil there too. |
Confirm on AMD64 |
Thanks very much for the fix, @uli-go. |
CL https://golang.org/cl/14227 mentions this issue. |
…erator The issue 12226 has been caused by the allocation of the same register for the equality check of two byte values. The code in cgen.go freed the register for the second operand before the allocation of the register for the first operand. Fixes #12226 Change-Id: Ie4dc33a488bd48a17f8ae9b497fd63c1ae390555 Reviewed-on: https://go-review.googlesource.com/13771 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-on: https://go-review.googlesource.com/14227 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Needed to pick up the fix for golang/go#12226 which affects other equality comparisons than the one mentioned in that issue.
@bradfitz Roger that, thank you for explaining! |
This snippet of code prints the strings as you would expect when compiled with 1.4, but produces no output when compiled with 1.5.
http://play.golang.org/p/hR7iVDC-Vu
Here's a slightly simpler example of the same bug:
This results in
false
with 1.4, buttrue
with 1.5.by taruti in #golang-nuts on irc
The text was updated successfully, but these errors were encountered: