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

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

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 7, 2021

@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
Copy link
Author

@gopherbot gopherbot commented Jan 8, 2021

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

Loading

@randall77
Copy link
Contributor

@randall77 randall77 commented Jan 8, 2021

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.

Loading

@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Jan 21, 2021

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

Loading

@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
… 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
Author

@gopherbot gopherbot commented Jan 21, 2021

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

Loading

@gopherbot gopherbot closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@dmitshur @randall77 @gopherbot and others