-
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: miscompilation in pointer operations #52953
Comments
The go compiler compiles func UF6 () (V16 T13) do:
The go compiler seems to eliminated |
cc @randall77 |
Allowed myself to do some renaming in example: https://go.dev/play/p/qFeFB85-o9H (old https://go.dev/play/p/y2tKrbrW8Bg) //Seed :1636136934
package main
import "fmt"
type T struct {
Field1 bool
}
func main() {
var ret T
ret.Field1 = true
var v *bool = &ret.Field1
ret = T{Field1: *v}
fmt.Println(ret.Field1)
} |
@gopherbot, please backport to Go 1.17 and 1.18. This bug causes miscompilation of otherwise-correct Go programs. |
Backport issue(s) opened: #52960 (for 1.17), #52961 (for 1.18). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases. |
It seems to me this is a bug in ascompatee, since when it only happens with named return. IIRC, ascompatee was optimized/refactored during 1.17 cycle. Will take a deeper look later if no one beat me then. cc @mdempsky |
@cuonglm my example has this bug without a return statement |
It looks to me that the AST is fine, but the initial SSA isn't quite right. It generates a wrong value for the assignment |
Okay, maybe the AST is indeed not quite right.
which is effectively
This is not equivalent to the original code as the value of Perhaps we need to evaluate the RHS ( |
Thanks for better reproducible program. Sounds like the problem was introduced long time ago, at least go1.16 also print false. |
Change https://go.dev/cl/407014 mentions this issue: |
Yeah, @orian 's example #52953 (comment) prints false since at least Go 1.11 (maybe even earlier, I didn't try), whereas the original example starts to fail only since Go 1.17... |
FWIW, @orian's example happens as far back as Go 1.3. With such a simple repro, it's surprising this hasn't been run into (or at least reported) earlier!
|
Change https://go.dev/cl/419450 mentions this issue: |
Change https://go.dev/cl/419451 mentions this issue: |
…nment if LHS is address-taken A composite literal assignment x = T{field: v} may be compiled to x = T{} x.field = v We already do not use this form is RHS uses LHS. If LHS is address-taken, RHS may uses LHS implicitly, e.g. v = &x.field x = T{field: *v} The lowering above would change the value of RHS (*v). Updates #52953. Fixes #52961. Change-Id: I3f798e00598aaa550b8c17182c7472fef440d483 Reviewed-on: https://go-review.googlesource.com/c/go/+/407014 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> (cherry picked from commit 1c77137) Reviewed-on: https://go-review.googlesource.com/c/go/+/419450 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
…nment if LHS is address-taken A composite literal assignment x = T{field: v} may be compiled to x = T{} x.field = v We already do not use this form is RHS uses LHS. If LHS is address-taken, RHS may uses LHS implicitly, e.g. v = &x.field x = T{field: *v} The lowering above would change the value of RHS (*v). Updates #52953. Fixes #52960. Change-Id: I3f798e00598aaa550b8c17182c7472fef440d483 Reviewed-on: https://go-review.googlesource.com/c/go/+/407014 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> (cherry picked from commit 1c77137) Reviewed-on: https://go-review.googlesource.com/c/go/+/419451 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
https://go.dev/play/p/yxl_PHRHUyf
What did you expect to see?
The program print true.
What did you see instead?
The program print false.
(This bug can be found in go1.17.9, but not in go1.16.15 and before)
The text was updated successfully, but these errors were encountered: