-
Notifications
You must be signed in to change notification settings - Fork 951
core: compile failures due to SSA race condition #4206
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
Comments
for additional context, here is the link to the msg in slack where we started this conversation: https://gophers.slack.com/archives/CDJD3SUP6/p1711334267168799 |
I tried reproducing by calling TinyGo build in a loop, but no luck so far. It's gone over 600 iterations. I'll keep trying. The suggestion was to restrict TinyGo build to one CPU using taskset -c 0 tinygo build ..., but right now I'm just trying to get it to fail in a loop without taskset... However, just in my normal GitHub workflow build, I did get another one:
I've noticed that all of the failures I've posted to this Issue are with -target=wioterminal, but I'm pretty sure I've seen if fail with other targets. |
It just hit me. Oddly, this was just after I did "brew upgrade". Here's the error:
And here's the program I was compiling:
Options included -opt=2 -target=wasi -gc=leaking and probably a few others. |
I hit it again today, this time on -target nano-rp2040 and with a different failure signature:
|
@aykevl any clues on this? |
I think dgryski suggested building with race detection on to see if that catches anything, and ayke had a couple suggestions on top of that: https://gophers.slack.com/archives/CN4R2DQV7/p1711749220916969 |
Is there anybody able to provide a reproducer? If you try to reproduce, here are a few ways to increase success:
|
Pretty sure I have seen this occur in CI builds as well. |
I've added this step to my project's CI, after each TinyGo build cmd. So far I haven't seen anything, but I'll keep an eye on it. |
Hit another one tonight. And this is with rm .cache files after each TinyGo build per @aykevl's suggestion.
|
I am also seeing this |
Duplicate of #3062? |
I will admit, this bug is remarkably similar to #4206 |
Ugh clearly haven't had my coffee yet - I copied the wrong issue number: #3062 |
Tinygo has some race/bugs where it fails due to tinygo-org/tinygo#4206 Add retries to mitigate. Signed-off-by: Tyler Rockwood <[email protected]>
Tinygo has some race/bugs where it fails due to tinygo-org/tinygo#4206 Add retries to mitigate. Signed-off-by: Tyler Rockwood <[email protected]>
To all the people reporting crashes: please provide a reproducer. I can't debug this without a reproducer, I have to see the crash happening on my own system. Getting more crash logs is near-useless, what I need is:
|
For the record, apparently this command failed at some point in CI (CC @dkegel-fastly):
EDIT: this doesn't seem to reproduce on my system even after many attempts. Or maybe I should leave it to run for a longer time... |
On the note of it being rare, I have seen it crop up on the order of ~1/10000 builds based on our CI stats. |
via CI https://github.com/tinygo-org/drivers/actions/runs/8812492890/job/24189343381#step:8:503 tinygo build -size short -o ./build/test.hex -target=challenger-rp2040 ./examples/net/ntpclient/
Basic Block in function '(*time.Time).Second' does not have terminator!
usage: go run ./smoketest.go smoketest.txt
label %entry
-xtensa
error: verification error after compiling package context
Enable Xtensa tests (default true)
exit status 1
make: *** [Makefile:13: smoke-test] Error 1 |
I think the race condition pretty consistently appears in this check https://github.com/tinygo-org/tinygo/blob/dev/builder/build.go#L384-L386 Is it perhaps because the IR for |
I've found a reliable reproducer! Run this in the fish shell in ./tests/os/smoke:
|
Found a race condition! |
So now I have a reliable reproducer, I found that the bug does not reproduce with |
golang.org/x/tools seems to be the culprit, I'm bisecting between v0.14.0 and v0.15.0. |
I think it's this commit: golang/tools@75ff53b |
Here is the upstream bug report: golang/go#67079 |
Here is our latest example of this bug doing its thing 🐛 |
the go tools change in question is https://go-review.googlesource.com/c/tools/+/538358/7 |
Currently down to this set of packages that when compiled together leads to a race:
Interestingly, this doesn't seem to include any reflection? (Though I haven't investigated the packages deeply). (Still figuring stuff out, I plan on sharing my findings once I know exactly which packages are the culprit). |
Got a bit further, and managed to capture a race condition log. See the details over at golang/go#67079 (comment). Hopefully this is enough information for the Go developers to find the race. |
There is now a fix, only waiting to be merged: https://go-review.googlesource.com/c/tools/+/590815 |
Temporary fix (applying one of the upstream proposed patches): #4292 |
This was released with |
This commit reverts commit 13a8eae. It appars that the race has been fixed by golang/tools@db513b091504, which we now use because it fixes another race condition as well. See: #4206
This commit reverts commit 13a8eae. It appars that the race has been fixed by golang/tools@db513b091504, which we now use because it fixes another race condition as well. See: #4206
Several folks on the Slack TinyGo channel reported in-frequent compile failures with 0.31.2.
Here's some I've captured in my build logs. I would estimate it happens 1/40 compiles. A re-compile is the work-around.
The text was updated successfully, but these errors were encountered: