-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: function inlining produces incorrect results in certain conditions #30566
Comments
Smaller repro:
@siddharthab if you could do a bisect, that would be very helpful. |
@gopherbot please backport to 1.12. |
Backport issue(s) opened: #30567 (for 1.12). Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
Here's what I can understand. If you look at the assembly code, you will see that the return value from the non-inlined function call was not saved further down the stack before executing the inline function call. In the actual code in our code base, the return value was moved to registers but then the execution immediately jumps to the inline function. I am not familiar with the compiler code base, so can not point to the source of the bug.
|
Thanks @marigonzes! Then we know to ping @randall77 @dr2chase. |
Hmm. Then the bug may have existed prior to that CL, and that CL only uncovered. Any chance you could re-bisect using |
Using |
Thanks. That seems way more plausible. @randall77 |
This appears to be a bug in the order pass. Here's a simple repro:
This should print 1 then 2. Instead, it prints 2 then 1. The problem is that order rewrites the code to:
Order wants to extract all the calls to their own statements so it can ensure the function calls all happen in order. But it can't extract the call from the RHS of a As far as I can tell, my autotmp reuse code just triggers this underlying bug. There's nothing wrong with the autotmp reuse otherwise. |
|
In particular, what happens in the OP's code is that the introduced temporaries are incorrectly reused because order processes the code in a different order than the code is emitted. We first process
We then process
But then it issues the code out of order. We end up with
Boom, we end up with |
Change https://golang.org/cl/165617 mentions this issue: |
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?
https://play.golang.org/p/7xK7ufXbSA6
What did you expect to see?
Actual and expected values in the demo above to match.
What did you see instead?
Results of one function call are being unintentionally overwritten by the results of another function call.
The text was updated successfully, but these errors were encountered: