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: Unified IR exports table is binary unstable in presence of generics [1.20 backport] #59585

Closed
gopherbot opened this issue Apr 12, 2023 · 3 comments
Closed

cmd/compile: Unified IR exports table is binary unstable in presence of generics [1.20 backport] #59585

gopherbot opened this issue Apr 12, 2023 · 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.20.4

Comments

@gopherbot
Copy link

@mdempsky requested issue #59571 to be considered for backport to the next 1.20 minor release.

@rsc Agreed. I've narrowed down the cause; just figuring out the best way to fix it.

@gopherbot Please backport to Go 1.20.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Apr 12, 2023
@gopherbot gopherbot mentioned this issue Apr 12, 2023
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 12, 2023
@gopherbot gopherbot added this to the Go1.20.4 milestone Apr 12, 2023
@mdempsky
Copy link
Member

This is a compiler issue resulting in non-reproducible builds with no workaround.

@gopherbot
Copy link
Author

Change https://go.dev/cl/484160 mentions this issue: [release-branch.go1.20] cmd/compile: fix reproducible build of aliased generic types

@prattmic prattmic added the CherryPickApproved Used during the release process for point releases label Apr 19, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Apr 19, 2023
@gopherbot
Copy link
Author

Closed by merging ee42d46 to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Apr 24, 2023
@mdempsky @gopherbot
[release-branch.go1.20] cmd/compile: fix reproducible build of aliase…
ee42d46 
…d generic types

Due to a missing "&& !alias" check, the unified linker was treating
type aliases the same as defined types for the purpose of exporting
method bodies. The methods will get exported anyway alongside the
aliased type, so this mistake is normally harmless.

However, if multiple type aliases instantiated the same generic type
but with different type arguments, this could result in the
same (generic) method body being exported multiple times under
different symbol names. Further, because bodies aren't expected to be
exported multiple times, we were sorting them simply based on index.
And consequently, the sort wasn't total and is sensitive to the map
iteration order used while ranging over linker.bodies.

The fix is simply to add the missing "&& !alias" check, so that we
don't end up with duplicate bodies in the first place.

Thanks rsc@ for providing a minimal repro case.

Fixes #59585.

Change-Id: Iaa55968cc7110b601e2f0f9b620901c2d55f7014
Reviewed-on: https://go-review.googlesource.com/c/go/+/484155
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit f58c6cc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/484160
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue May 25, 2023
@mdempsky @bradfitz
[release-branch.go1.20] cmd/compile: fix reproducible build of aliase…
aadf2d6 
…d generic types

Due to a missing "&& !alias" check, the unified linker was treating
type aliases the same as defined types for the purpose of exporting
method bodies. The methods will get exported anyway alongside the
aliased type, so this mistake is normally harmless.

However, if multiple type aliases instantiated the same generic type
but with different type arguments, this could result in the
same (generic) method body being exported multiple times under
different symbol names. Further, because bodies aren't expected to be
exported multiple times, we were sorting them simply based on index.
And consequently, the sort wasn't total and is sensitive to the map
iteration order used while ranging over linker.bodies.

The fix is simply to add the missing "&& !alias" check, so that we
don't end up with duplicate bodies in the first place.

Thanks rsc@ for providing a minimal repro case.

Fixes golang#59585.

Change-Id: Iaa55968cc7110b601e2f0f9b620901c2d55f7014
Reviewed-on: https://go-review.googlesource.com/c/go/+/484155
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit f58c6cc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/484160
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
bradfitz pushed a commit to tailscale/go that referenced this issue May 25, 2023
@mdempsky @bradfitz
[release-branch.go1.20] cmd/compile: fix reproducible build of aliase…
fefb237 
…d generic types

Due to a missing "&& !alias" check, the unified linker was treating
type aliases the same as defined types for the purpose of exporting
method bodies. The methods will get exported anyway alongside the
aliased type, so this mistake is normally harmless.

However, if multiple type aliases instantiated the same generic type
but with different type arguments, this could result in the
same (generic) method body being exported multiple times under
different symbol names. Further, because bodies aren't expected to be
exported multiple times, we were sorting them simply based on index.
And consequently, the sort wasn't total and is sensitive to the map
iteration order used while ranging over linker.bodies.

The fix is simply to add the missing "&& !alias" check, so that we
don't end up with duplicate bodies in the first place.

Thanks rsc@ for providing a minimal repro case.

Fixes golang#59585.

Change-Id: Iaa55968cc7110b601e2f0f9b620901c2d55f7014
Reviewed-on: https://go-review.googlesource.com/c/go/+/484155
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
(cherry picked from commit f58c6cc)
Reviewed-on: https://go-review.googlesource.com/c/go/+/484160
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@golang golang locked and limited conversation to collaborators Apr 23, 2024
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.20.4
Development

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