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: ICE due to gc.usefield panic #42686

Closed
mdempsky opened this issue Nov 18, 2020 · 2 comments
Closed

cmd/compile: ICE due to gc.usefield panic #42686

mdempsky opened this issue Nov 18, 2020 · 2 comments

Comments

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Nov 18, 2020

To reproduce:

GOEXPERIMENT=fieldtrack ./make.bash
go get github.com/golang/protobuf/proto

@cherrymui bisected to CL 266199 (f2c0c2b).

@mdempsky mdempsky added this to the Go1.16 milestone Nov 18, 2020
@mdempsky mdempsky self-assigned this Nov 18, 2020
@mdempsky
Copy link
Member Author

@mdempsky mdempsky commented Nov 18, 2020

Here's a minimal reproducer:

package p
func a(x struct { f int }) { _ = x.f }
func b() { a(struct { f int }{}) }

The issue is that the frontend doesn't guarantee uniqueness of types.Type instances for identical types; code has to use types.Identical for this. For example, in the code above the two struct { f int } type literals each get represented as their own types.Type instances.

Further, when inlining, we use the argument types to declare the inlined-parameter variables, but then copy the original AST nodes without re-typechecking them. When x.f was first typechecked, x referred to a variable with the first types.Type instance. But after substitution in inlining, it now refers to a variable of the second type. (To emphasize, the two instances both represent identical types, but they have separate pointer values.)

The field tracking code breaks because it uses the struct type instance as a map key, when this isn't strictly guaranteed to be unique. It worked up until cced777, I think because that CL seems to have had a side effect of changing the inlined-parameter variable's type from the types.Type instance used for the parameter to the one used for the argument. (f2c0c2b then allowed further inlining, which evidently led to the protobuf failure.)

I think the easiest fix is to simply get rid of the map, and use Node.Opt to store the *types.Field directly. I'll send a CL in the morning.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 18, 2020

Change https://golang.org/cl/271217 mentions this issue: cmd/compile: fix panic in field tracking logic

Loading

@gopherbot gopherbot closed this in 5b0ec1a Nov 18, 2020
@golang golang locked and limited conversation to collaborators Nov 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants