-
Notifications
You must be signed in to change notification settings - Fork 17.3k
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 #43570
Comments
Thanks @rkojedzinszky @neolit123 @MikeSpreitzer @brandond @liggitt for help reporting & investigating |
Hrm, but that wouldn't explain a repro on 386. |
I can reproduce on 386. Looks like something is getting clobbered. If I print out the good and bad time structs in hex, I get
The first 2 words there are valid pointers to the heap. Not sure about the 3rd one. I think this only happens on 32-bit because on 32-bit the I belive the bad code is in Still looking. |
FWIW, moving the |
I think I see the problem. We have (irrelevant code elided):
That's all fine. But then we run the
Which basically says, if you're moving from After the opt pass, we have the same code, except v60 copies from v53 instead of v15. Which is bad, because that source is the volatile memory area. It's only a problem if there is an intervening call. In this case there is one, but only if the write barrier is enabled. So the corruption doesn't happen until the write barrier is turned on. (The write barrier replaces v54 with a bunch of code that may call I think the right fix is to add an extra condition to the move optimization rule to disable it if the source is a volatile region. |
I have a small repro that also works on 64 bit. |
Backport issue(s) opened: #43574 (for 1.14), #43575 (for 1.15). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Change https://golang.org/cl/282492 mentions this issue: |
Thanks @randall77 for the fast analysis & fix! |
This avoids golang/go#43570 and should take care of kubernetes#97685 .
This avoids golang/go#43570 and should take care of kubernetes#97685 .
to tiptoe around golang/go#43570 for #97685 Kubernetes-commit: 611184aa59d0cd40466bc3bc4b40a3712a038171
to tiptoe around golang/go#43570 for #97685 Kubernetes-commit: d8a1dfb21f1f78595d1aaf9985eb58ee50edc940
…innerSet move all variables in sampleAndWaterMarkHistograms::innerSet to tiptoe around golang/go#43570 for kubernetes#97685
…innerSet move all variables in sampleAndWaterMarkHistograms::innerSet to tiptoe around golang/go#43570 for kubernetes#97685
to tiptoe around golang/go#43570 for #97685 Kubernetes-commit: f393cb405bd04b9bcc014b7852e8c76ee90eb418
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?
While running a 32-bit golang executable random data corruption may occur. Here is an example Kubernetes issue which is able to reproduce the problem with 32-bit arm and 386:
kubernetes/kubernetes#97685
The unit test is compiled and attached as a standalone example here:
32bit-bug.tar.gz
Test code, commands, armv6 binaries, and relevant objdump data is available in above tarball.
If the test is compiled without optimizations the test will pass (on arm at least).
(edit: remove link to aarch64 issue)
What did you expect to see?
What did you see instead?
The text was updated successfully, but these errors were encountered: