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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: internal compiler error: method Len on *uint8 not found [1.19 backport] #55270

Closed
gopherbot opened this issue Sep 20, 2022 · 3 comments
Closed

cmd/compile: internal compiler error: method Len on *uint8 not found [1.19 backport] #55270

gopherbot opened this issue Sep 20, 2022 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone
Go1.19.2

Comments

@gopherbot
Copy link

@mdempsky requested issue #54225 to be considered for backport to the next 1.19 minor release.

@gopherbot Please backport to Go 1.19
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 20, 2022
@gopherbot gopherbot mentioned this issue Sep 20, 2022
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Sep 20, 2022
@gopherbot gopherbot added this to the Go1.19.2 milestone Sep 20, 2022
@mdempsky
Copy link
Member

The backport CL is https://go-review.googlesource.com/c/go/+/429895

It's intentionally minimally invasive to address the issue while minimizing risk of collateral damage.

Specifically, instead of hitting an ICE while emitting broken dictionary symbols, it changes the compiler to skip emitting those symbols instead. If the symbols are actually needed, the consequence is this just turns an ICE into a linker failure: no real harm.

But I believe in practice the broken symbols aren't actually needed in the final executable, and so the linker's reachability analysis will simply ignore the missing symbol.

@gopherbot
Copy link
Author

Change https://go.dev/cl/429895 mentions this issue: [release-branch.go1.19] cmd/compile: skip emitting dictionaries with missing method expressions

@cherrymui cherrymui added the CherryPickApproved Used during the release process for point releases label Sep 21, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Sep 21, 2022
@gopherbot
Copy link
Author

Closed by merging 3747bb2 to release-branch.go1.19.

gopherbot pushed a commit that referenced this issue Sep 21, 2022
@mdempsky @cherrymui
[release-branch.go1.19] cmd/compile: skip emitting dictionaries with …
3747bb2 
…missing method expressions

The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.

The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.

This CL is a minimally invasive solution to #54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.

Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.

Fixes #55270.

Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429895
Reviewed-by: Keith Randall <khr@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Oct 5, 2022
@mdempsky @bradfitz
[release-branch.go1.19] cmd/compile: skip emitting dictionaries with …
93abd99 
…missing method expressions

The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.

The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.

This CL is a minimally invasive solution to golang#54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.

Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.

Fixes golang#55270.

Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429895
Reviewed-by: Keith Randall <khr@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Oct 5, 2022
@mdempsky @bradfitz
[release-branch.go1.19] cmd/compile: skip emitting dictionaries with …
3904902 
…missing method expressions

The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.

The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.

This CL is a minimally invasive solution to golang#54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.

Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.

Fixes golang#55270.

Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429895
Reviewed-by: Keith Randall <khr@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue Oct 5, 2022
@mdempsky @bradfitz
[release-branch.go1.19] cmd/compile: skip emitting dictionaries with …
4aa68b2 
…missing method expressions

The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.

The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.

This CL is a minimally invasive solution to golang#54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.

Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.

Fixes golang#55270.

Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/429895
Reviewed-by: Keith Randall <khr@google.com>
@ALTree ALTree mentioned this issue Dec 23, 2022
Closed
@hasheddan hasheddan mentioned this issue May 25, 2023
Merged
@golang golang locked and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
Go1.19.2
Development

No branches or pull requests
3 participants
@mdempsky @gopherbot @cherrymui