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.19 backport] #54629

Closed
randall77 opened this issue Aug 23, 2022 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone

Comments

@randall77
Copy link
Contributor

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

@randall77 randall77 added CherryPickCandidate Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. labels Aug 23, 2022
@randall77 randall77 added this to the Go1.19.1 milestone Aug 23, 2022
@randall77 randall77 changed the title cmd/compile: miscompilation of partially-overlapping array assignments [1.18 backport] cmd/compile: miscompilation of partially-overlapping array assignments [1.19 backport] Aug 23, 2022
@gopherbot
Copy link

Change https://go.dev/cl/425234 mentions this issue: [release-branch.go1.19] 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 #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
@gopherbot
Copy link

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
@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

3 participants