-
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: miscompilation of partially-overlapping array assignments [1.19 backport] #54629
Labels
CherryPickApproved
Used during the release process for point releases
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
Milestone
Comments
Change https://go.dev/cl/425234 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Aug 29, 2022
…ignments Normally, when moving Go values of type T from one location to another, we don't need to worry about partial overlaps. The two Ts must either be in disjoint (nonoverlapping) memory or in exactly the same location. There are 2 cases where this isn't true: 1) Using unsafe you can arrange partial overlaps. 2) Since Go 1.17, you can use a cast from a slice to a ptr-to-array. https://go.dev/ref/spec#Conversions_from_slice_to_array_pointer This feature can be used to construct partial overlaps of array types. var a [3]int p := (*[2]int)(a[:]) q := (*[2]int)(a[1:]) *p = *q We don't care about solving 1. Or at least, we haven't historically and no one has complained. For 2, we need to ensure that if there might be partial overlap, then we can't use OpMove; we must use memmove instead. (memmove handles partial overlap by copying in the correct direction. OpMove does not.) Note that we have to be careful here not to introduce a call when we're marshaling arguments to a call or unmarshaling results from a call. Fixes #54629 Change-Id: I1ca6aba8041576849c1d85f1fa33ae61b80a373d Reviewed-on: https://go-review.googlesource.com/c/go/+/425076 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Keith Randall <khr@golang.org> (cherry picked from commit 332a598) Reviewed-on: https://go-review.googlesource.com/c/go/+/425234
Closed by merging d39fd4f to release-branch.go1.19. |
bradfitz
pushed a commit
to tailscale/go
that referenced
this issue
Sep 8, 2022
…ignments Normally, when moving Go values of type T from one location to another, we don't need to worry about partial overlaps. The two Ts must either be in disjoint (nonoverlapping) memory or in exactly the same location. There are 2 cases where this isn't true: 1) Using unsafe you can arrange partial overlaps. 2) Since Go 1.17, you can use a cast from a slice to a ptr-to-array. https://go.dev/ref/spec#Conversions_from_slice_to_array_pointer This feature can be used to construct partial overlaps of array types. var a [3]int p := (*[2]int)(a[:]) q := (*[2]int)(a[1:]) *p = *q We don't care about solving 1. Or at least, we haven't historically and no one has complained. For 2, we need to ensure that if there might be partial overlap, then we can't use OpMove; we must use memmove instead. (memmove handles partial overlap by copying in the correct direction. OpMove does not.) Note that we have to be careful here not to introduce a call when we're marshaling arguments to a call or unmarshaling results from a call. Fixes golang#54629 Change-Id: I1ca6aba8041576849c1d85f1fa33ae61b80a373d Reviewed-on: https://go-review.googlesource.com/c/go/+/425076 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Keith Randall <khr@golang.org> (cherry picked from commit 332a598) Reviewed-on: https://go-review.googlesource.com/c/go/+/425234
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
CherryPickApproved
Used during the release process for point releases
compiler/runtime
Issues related to the Go compiler and/or runtime.
FrozenDueToAge
@randall77 requested issue #54467 to be considered for backport to the next 1.19 minor release.
The text was updated successfully, but these errors were encountered: