-
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: internal compiler error: missing typecheck [1.19 backport] #56744
Comments
Change https://go.dev/cl/451155 mentions this issue: |
https://go.dev/wiki/MinorReleases includes:
@randall77 Can you please add a rationale for why this backport should be made? It's needed for the cherry-pick review process to make progress. Thanks. |
Some static initialization code causes the compiler to crash. |
We discussed with the release team, and we think we don't know exactly the importance and safety for this backport. Could the compiler team give more input to make the decision? Thanks. |
This backport I would describe as mildly important. Without it there would be some static initialization code that causes the compiler to crash. It isn't common, but it isn't fuzz-generated either. Real people have run into it. I suspect it is not too hard to work around. The backport is very safe. It just does some extra typechecking. |
Thanks. As #56727 (comment) mentioned, this issue was not in Go 1.19.2 but caused by a backport of #56105 in 1.19.3. I guess another option would be reverting that backport. The input in #56105 does look a bit contrived. The concern here is that the backport of #56105 has caused a chain of more issues and backports. How would that sound? Thanks. |
Yes, the other option is to revert the fix for #56106. That seems ok to me. The test case for that issue is certainly more contrived than this one. There's an outside chance that reverting the fix for #56106 leaves open the possibility of incorrectly compiled code instead of an ICE. But if it is possible it is certainly very rare. |
I'm still leaning to backport theses. Isn't that without #56106, people won't be able to use fuzzing? |
I think we should go ahead with the backport. The backport fix is very safe (doing an unnecessary typecheck is a no-op), and can't make the situation worse. Whereas, as Keith pointed out, there's some chance that reverting the original fix reintroduces a miscompilation, which is much worse than an ICE. On balance, I support landing the backport. |
Closed by merging 926ffba to release-branch.go1.19. |
… initialization slice CL 440455 fixed missing walk pass for static initialization slice. However, slicelit may produce un-typechecked node, thus we need to do typecheck for sinit before calling walkStmtList. Fixes #56744 Change-Id: I40730cebcd09f2be4389d71c5a90eb9a060e4ab7 Reviewed-on: https://go-review.googlesource.com/c/go/+/450215 Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/451155 Reviewed-by: Joedian Reid <joedian@golang.org>
@randall77 requested issue #56727 to be considered for backport to the next 1.19 minor release.
The text was updated successfully, but these errors were encountered: