-
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: ICE on deferred call to syscall.LazyDLL.Call #44415
Comments
Note that the dll and proc instantiation should probably be outside the function, something like:
But, it looks like a compiler bug nevertheless. |
Error message looks the same as #44355. |
Thanks for the detailed issue. I think you're right that this is a duplicate of #44355. I'll close this in lieu of that issue. I think this is an interesting case to mention there though. |
Issue #44355 is pretty narrow: it should only affect inlinable functions that explicitly name all of their result parameters as blank (i.e., It would be great to have a standalone reproducer for this issue. Ideally one that can be built from Linux (e.g., using (FWIW, "Value live at entry" is a pretty generic catch-all failure that happens later in the compiler when almost anything goes wrong in the frontend and we didn't catch it sooner to give a more descriptive error report. So I don't recommend using that alone to deduplicate issues.) |
Windows reproducer (based on the initial cause in https://github.com/moonD4rk/HackBrowserData/blob/6a11361e1dbf0c3e455ccb06a4f31d073f7d8e8e/core/decrypt/decrypt_windows.go):
|
Thanks @egonelbre! Further minimized, and confirmed it reproduces on Linux with
Notably, it doesn't reproduce with /cc @cuonglm |
Oh, in the fix for #24491, we only fixed direct function calls. We forgot that for method calls, there's a receiver argument that needs to be passed through the wrapper function too. |
@gopherbot Backport to Go 1.16. Compiler crash on valid Windows code that compiled with Go 1.15. |
@mdempsky I think you have to say "please" or the gopherbot won't listen to you. A somewhat questionable design decision... |
@gopherbot Please backport to Go 1.16. Compiler crash on valid Windows code that compiled with Go 1.15. |
Backport issue(s) opened: #44463 (for 1.15), #44464 (for 1.16). 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/294849 mentions this issue: |
Change https://golang.org/cl/296489 mentions this issue: |
Reopening because the fix is incomplete in the presence of non-open-coded defers. |
It looks like it only worked for open-coded defers because open-coded defers already keep their arguments alive past the deferred call: Failing test case using open-coded defer: https://play.golang.org/p/9SEiOeaLmCS Passing test case using heap-allocated defer: https://play.golang.org/p/BWEQuC5EowK |
Issue with open-coded defers reported as #44623. As for CL 296489, I think the issue was that when we rewrite Two possible options I see are:
I was initially thinking 2 would be the easier and safer option; but if option 1 isn't too invasive, I think that might be better actually. As for a test case, locally I've just added:
It would probably make sense to add a similar variant for normal deferred calls too. (On the upside, these are at least already handled correctly.) |
@mdempsky FYI, the fix in CL 296569 is the 2nd. |
@mdempsky I'm leaning to doing 2nd option for now, so the fix and the backports CL will be in synced. Then we can refactoring to 1st option later. |
Change https://golang.org/cl/296490 mentions this issue: |
Change https://golang.org/cl/296769 mentions this issue: |
…r arguments with call method in go/defer In CL 253457, we did the same fix for direct function calls. But for method calls, the receiver argument also need to be passed through the wrapper function, which we are not doing so the compiler crashes with the code in #44415. It will be nicer if we can rewrite OCALLMETHOD to normal OCALLFUNC, but that will be for future CL. The passing receiver argument to wrapper function is easier for backporting to go1.16 branch. Fixes #44464 Change-Id: I03607a64429042c6066ce673931db9769deb3124 Reviewed-on: https://go-review.googlesource.com/c/go/+/296490 Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-on: https://go-review.googlesource.com/c/go/+/296769 Trust: Bryan C. Mills <bcmills@google.com>
What version of Go are you using (
go version
)?go version go1.16 windows/amd64
Does this issue reproduce with the latest release?
Yes, it does
What operating system and processor architecture are you using (
go env
)?What did you do?
My previous version is Go 1.15.6 and everything is ok.
I updated my Go and my func can not build with the lastest version:
What did you expect to see?
My func can run and build with my Go
What did you see instead?
proj-path/core/browsers/decrypt
core\browsers\decrypt\decrypt_windows.go:89:2: internal compiler error: 'wrap·2': Value live at entry. It shouldn't be. func wrap·2, node procLocalFree, value v35
Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new
core\browsers\decrypt\decrypt_windows.go:75:2: internal compiler error: 'wrap·1': Value live at entry. It shouldn't be. func wrap·1, node procLocalFree, value v35
Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new
Desc:
decrypt_windows.go:89 is line
defer procLocalFree.Call(uintptr(unsafe.Pointer(outBlob.pbData)))
decrypt_windows.go:75 is line
func DPApi(data []byte) ([]byte, error) {
The text was updated successfully, but these errors were encountered: