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: unsafe.SliceData incoherent resuilt with nil argument [1.20 backport] #59296

Closed
gopherbot opened this issue Mar 28, 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

Comments

@gopherbot
Copy link

@randall77 requested issue #59293 to be considered for backport to the next 1.20 minor release.

@gopherbot, please open a backport issue for 1.20.

This is an incorrect optimizer result. I guess it is possible to work around it, but it can be tricky to fight the optimizer like this.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 28, 2023
@gopherbot gopherbot added this to the Go1.20.3 milestone Mar 28, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/479899 mentions this issue: [release-branch.go1.20] cmd/compile: don't assume pointer of a slice is non-nil

@seankhliao seankhliao changed the title affected/package: unsafe.SliceData incoherent resuilt with nil argument [1.20 backport] cmd/compile: unsafe.SliceData incoherent resuilt with nil argument [1.20 backport] Mar 28, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 28, 2023
@heschi heschi added the CherryPickApproved Used during the release process for point releases label Mar 29, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Mar 29, 2023
@heschi
Copy link
Contributor

heschi commented Mar 29, 2023

Miscompilation with no good workaround, and the fix seems relatively small. Approved.

@gopherbot
Copy link
Author

Closed by merging 8dce4ca to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Mar 29, 2023
…is non-nil

unsafe.SliceData can return pointers which are nil. That function gets
lowered to the SSA OpSlicePtr, which the compiler assumes is non-nil.
This used to be the case as OpSlicePtr was only used in situations
where the bounds check already passed. But with unsafe.SliceData that
is no longer the case.

There are situations where we know it is nil. Use Bounded() to
indicate that.

I looked through all the uses of OSPTR and added SetBounded where it
made sense. Most OSPTR results are passed directly to runtime calls
(e.g. memmove), so even if we know they are non-nil that info isn't
helpful.

Fixes #59296

Change-Id: I437a15330db48e0082acfb1f89caf8c56723fc51
Reviewed-on: https://go-review.googlesource.com/c/go/+/479896
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
(cherry picked from commit b899641ecea7d07c997282e985beb295c31d1097)
Reviewed-on: https://go-review.googlesource.com/c/go/+/479899
Run-TryBot: Keith Randall <khr@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
@golang golang locked and limited conversation to collaborators Mar 28, 2024
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