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: miscompilation of partially-overlapping array assignments [1.18 backport] #54603

Closed
gopherbot opened this issue Aug 22, 2022 · 3 comments
Assignees
Labels
CherryPickApproved 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 Aug 22, 2022

@randall77 requested issue #54467 to be considered for backport to the next 1.18 minor release.

@gopherbot please open backport issues for 1.18 and 1.17.

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

gopherbot commented Aug 23, 2022

Change https://go.dev/cl/425198 mentions this issue: cmd/compile: handle partially overlapping assignments

@gopherbot
Copy link
Author

gopherbot commented Aug 23, 2022

Change https://go.dev/cl/425198 mentions this issue: [release-branch.go1.18] cmd/compile: handle partially overlapping assignments

@dr2chase dr2chase added the CherryPickApproved Used during the release process for point releases label Aug 24, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 24, 2022
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 #54603

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/+/425198
@gopherbot
Copy link
Author

gopherbot commented Aug 29, 2022

Closed by merging 66197f0 to release-branch.go1.18.

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

No branches or pull requests

2 participants