-
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: liveness / GC bug with variadic call #16996
Comments
CL demo of above for easier cherry-picking is https://go-review.googlesource.com/28534 (git fetch https://go.googlesource.com/go refs/changes/34/28534/2 && git cherry-pick FETCH_HEAD) |
I tried to distill it to a standalone test but couldn't reproduce in its smaller form, so my guess of the problem is likely incorrect. |
CL https://golang.org/cl/28534 mentions this issue. |
Go does not guarantee that a finalizer will be run in some bounded amount This test seems to assume otherwise. If a program want to execute On Mon, Sep 5, 2016 at 5:19 PM, Brad Fitzpatrick notifications@github.com
|
@RLH, I know. This isn't a bug report about finalizers. I'm using finalizers as a means to demonstrate what I think is a compiler bug. |
From a quick look, here's what's going on. Note that I changed Ok.
autotmp_238 here is the After the call to io.MultiReader, autotmp_238 is never touched again. So the references to buf1 and buf2 remain on the stack until the function exits. One fix would be to teach the compiler to explicitly zero the temporary that it creates (in mkdotargslice, in walk.go) after the call. This would add a bit of overhead to variadic calls, but allow faster reclamation. I don't currently plan to work on this further. |
We take the address of the Josh's solution (zero it after the call) would also work. |
Simple repro:
It is indeed a missing VarKill. There seems to be some confusion around levels of escapedness. If the ... arg escapes, or does not escape, then we generate correct code. This example seems to be in a gray area - the arg of |
CL https://golang.org/cl/28575 mentions this issue. |
CL above reverted. Reopening. |
CL https://golang.org/cl/28771 mentions this issue. |
Updates #16983 Updates #16996 Change-Id: I76390766385b2668632c95e172b2d243d7f66651 Reviewed-on: https://go-review.googlesource.com/28771 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Reverted the revert at https://go-review.googlesource.com/c/28582/ |
When writing a test for #16983, I encountered what looks like perhaps a liveness bug with variadic calls.
If I set "const useVariadic" to true below, the test fails (buf1 is never collected), but if I set it to false, buf1 is collected.
/cc @randall77 @josharian @minux @cherrymui
The text was updated successfully, but these errors were encountered: