Skip to content
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

Open
gopherbot opened this issue Nov 15, 2022 · 8 comments
Open
Labels
CherryPickCandidate Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@gopherbot
Copy link

gopherbot commented Nov 15, 2022

@randall77 requested issue #56727 to be considered for backport to the next 1.19 minor release.

@gopherbot please open a backport issue for 1.19.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Nov 15, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 15, 2022
@gopherbot gopherbot added this to the Go1.19.4 milestone Nov 15, 2022
@gopherbot
Copy link
Author

gopherbot commented Nov 16, 2022

Change https://go.dev/cl/451155 mentions this issue: [release-branch.go1.19] cmd/compile: fix missing typecheck for static initialization slice

@dmitshur
Copy link
Contributor

dmitshur commented Nov 16, 2022

https://go.dev/wiki/MinorReleases includes:

The issue should include [...] and a short rationale about why the backport might be needed.

@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.

@randall77
Copy link
Contributor

randall77 commented Nov 16, 2022

Some static initialization code causes the compiler to crash.
(This issue is a fix-to-the-fix for #56105, which has already been backported.)

@cherrymui
Copy link
Member

cherrymui commented Nov 23, 2022

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.

@randall77
Copy link
Contributor

randall77 commented Nov 23, 2022

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 fix for #56105 was for the same underlying issue, but was incomplete. This issue+CL fixes that incompleteness.

The backport is very safe. It just does some extra typechecking.

@cherrymui
Copy link
Member

cherrymui commented Nov 23, 2022

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.

@randall77
Copy link
Contributor

randall77 commented Nov 23, 2022

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.

@cuonglm
Copy link
Member

cuonglm commented Nov 24, 2022

I'm still leaning to backport theses.

Isn't that without #56106, people won't be able to use fuzzing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickCandidate Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
Status: No status
Development

No branches or pull requests

5 participants