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. FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@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

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

@gopherbot
Copy link
Author

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

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

@golang golang locked and limited conversation to collaborators Aug 29, 2023
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
Projects
None yet
Development

No branches or pull requests

2 participants