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: miscompilation of codependent global `var` assigments in go1.12 [1.12 backport] #30996

Closed
gopherbot opened this issue Mar 22, 2019 · 9 comments

Comments

Projects
None yet
4 participants
@gopherbot
Copy link

commented Mar 22, 2019

@cherrymui requested issue #30977 to be considered for backport to the next 1.12 minor release.

@gopherbot please backport this to Go 1.12.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Mar 22, 2019

Change https://golang.org/cl/168817 mentions this issue: [release-branch.go1.12] cmd/compile: copy volatile values before emitting write barrier call

@bcmills bcmills changed the title cmd/go: miscompilation of codependent global `var` assigments in go1.12 [1.12 backport] cmd/compile: miscompilation of codependent global `var` assigments in go1.12 [1.12 backport] Mar 28, 2019

@thaJeztah

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

@randall77 I saw you reviewed https://golang.org/cl/168817 - does that mean this should be labeled CherryPickApproved ?

@randall77

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

No, that's still a call for the release guys to make.

Feel free to add pro-backport arguments to this issue.
I think it should be backported - it's a nasty bug with no easy workaround, and it was triggered in the wild.

@thaJeztah

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

Thanks for explaining (and apologies for being a bit noisy 🤗)

For our next release, this one is definitely a show-stopper, as the result is unpredictable. Timing is a bit unfortunate as we'll have to decide to proceed with Go 1.12, or revert all repositories to Go 1.11.

@bradfitz (I saw you applied the labels on other issues; so hope you're the right person to ask); ptal

@thaJeztah

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

For my information; Is there any documentation about the Go release procedure? (or a mailinglist / meeting people can attend to stay informed?)

@randall77

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

@randall77

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

@thaJeztah

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

Awesome, thanks!

@gopherbot

This comment has been minimized.

Copy link
Author

commented Apr 2, 2019

Closed by merging 8acc2ea to release-branch.go1.12.

@gopherbot gopherbot closed this Apr 2, 2019

gopherbot pushed a commit that referenced this issue Apr 2, 2019

[release-branch.go1.12] cmd/compile: copy volatile values before emit…
…ting write barrier call

It is possible that a "volatile" value (one that can be clobbered
by preparing args of a call) to be used in multiple write barrier
calls. We used to copy the volatile value right before each call.
But this doesn't work if the value is used the second time, after
the first call where it is already clobbered. Copy it before
emitting any call.

Updates #30977.
Fixes #30996.

Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168677
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit f23c601)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168817
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.