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.

Sign up for GitHub

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: 32-bit random data corruption [1.14 backport] #43574

Closed
gopherbot opened this issue Jan 7, 2021 · 4 comments
Closed

cmd/compile: 32-bit random data corruption [1.14 backport] #43574

gopherbot opened this issue Jan 7, 2021 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.14.15

Comments

@gopherbot
Copy link
Contributor

@randall77 requested issue #43570 to be considered for backport to the next 1.14 minor release.

I have a small repro that also works on 64 bit.
It's been broken since 1.12.
@gopherbot please open backport issues.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jan 7, 2021
@gopherbot gopherbot mentioned this issue Jan 7, 2021
Closed
@gopherbot gopherbot added this to the Go1.14.14 milestone Jan 7, 2021
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/282559 mentions this issue: [release-branch.go1.14] cmd/compile: don't short-circuit copies whose source is volatile

@randall77
Copy link
Contributor

This bug causes random corruption in rare cases involving large structs being copied from function result to heap to function argument. Workaround are possible but difficult.

@dmitshur dmitshur modified the milestones: Go1.14.14, Go1.14.15 Jan 19, 2021
@dmitshur dmitshur mentioned this issue Jan 21, 2021
Closed
@dmitshur
Copy link
Contributor

Approving as a serious issue without a reasonable workaround. This backport applies to both 1.15 (#43575) and 1.14 (this issue).

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Jan 21, 2021
@dmitshur dmitshur changed the title 32-bit random data corruption [1.14 backport] cmd/compile: 32-bit random data corruption [1.14 backport] Jan 21, 2021
gopherbot pushed a commit that referenced this issue Jan 21, 2021
@randall77 @dmitshur
[release-branch.go1.14] cmd/compile: don't short-circuit copies whose…
cb39368 
… source is volatile

Current optimization: When we copy a->b and then b->c, we might as well
copy a->c instead of b->c (then b might be dead and go away).

*Except* if a is a volatile location (might be clobbered by a call).
In that case, we really do want to copy a immediately, because there
might be a call before we can do the a->c copy.

User calls can't happen in between, because the rule matches up the
memory states. But calls inserted for memory barriers, particularly
runtime.typedmemmove, can.

(I guess we could introduce a register-calling-convention version
of runtime.typedmemmove, but that seems a bigger change than this one.)

Fixes #43574

Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/282492
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 304f769)
Reviewed-on: https://go-review.googlesource.com/c/go/+/282559
@gopherbot
Copy link
Contributor Author

Closed by merging cb39368 to release-branch.go1.14.

@golang golang locked and limited conversation to collaborators Jan 21, 2022
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.14.15
Development

No branches or pull requests
4 participants
@dmitshur @randall77 @gopherbot and others