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

cmd/compile: miscompilation in pointer operations [1.18 backport] #52961

Closed
gopherbot opened this issue May 18, 2022 · 4 comments
Closed

cmd/compile: miscompilation in pointer operations [1.18 backport] #52961

gopherbot opened this issue May 18, 2022 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.18.5

Comments

@gopherbot
Copy link

@bcmills requested issue #52953 to be considered for backport to the next 1.18 minor release.

@gopherbot, please backport to Go 1.17 and 1.18. This bug causes miscompilation of otherwise-correct Go programs.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 18, 2022
@gopherbot gopherbot mentioned this issue May 18, 2022
Closed
@gopherbot gopherbot added this to the Go1.18.3 milestone May 18, 2022
@randall77
Copy link
Contributor

I'm not sure this needs backporting. The bug was only found by fuzzing the compiler, not in a real program. The code to trigger it is unusual (reinitializing a variable using a pointer to that very same variable), and easily worked around.

That said, the fix is pretty safe, and if you do hit the bug it causes incorrect behavior.

@randall77 randall77 mentioned this issue May 18, 2022
Closed
@bcmills
Copy link
Contributor

bcmills commented May 18, 2022

The bug was only found by fuzzing the compiler, not in a real program. The code to trigger it is unusual (reinitializing a variable using a pointer to that very same variable), and easily worked around.

That may be true, but the behavior is also pretty subtle — I wouldn't be at all surprised if there are a few real-world programs that do trigger it but don't notice the incorrect behavior, and of course we don't have any way to identify them if they're still on an older Go release.

@dmitshur dmitshur modified the milestones: Go1.18.3, Go1.18.4 Jun 1, 2022
@heschi heschi added the CherryPickApproved Used during the release process for point releases label Jun 15, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jun 15, 2022
@gopherbot gopherbot modified the milestones: Go1.18.4, Go1.18.5 Jul 12, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/419450 mentions this issue: [release-branch.go1.18] cmd/compile: do not use special literal assignment if LHS is address-taken

@gopherbot
Copy link
Author

Closed by merging ed50277 to release-branch.go1.18.

gopherbot pushed a commit that referenced this issue Jul 26, 2022
@cherrymui
[release-branch.go1.18] cmd/compile: do not use special literal assig…
ed50277 
…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>
@golang golang locked and limited conversation to collaborators Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.18.5
Development

No branches or pull requests
5 participants
@dmitshur @bcmills @randall77 @gopherbot @heschi