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: function compiled without bounds checking and -1 index access [1.11 backport] #27378

Closed
gopherbot opened this issue Aug 30, 2018 · 2 comments
Labels
CherryPickCandidate Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@FiloSottile requested issue #27251 to be considered for backport to the next 1.11 minor release.

@gopherbot please file this for backport against 1.11. This is a regression.

@dr2chase please make a cherry-pick CL once your change is merged in master.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 30, 2018
@gopherbot gopherbot added this to the Go1.11.1 milestone Aug 30, 2018
@gopherbot
Copy link
Author

Change https://golang.org/cl/132575 mentions this issue: [release-branch.go1.11] cmd/compile: in prove, fix fence-post implications for unsigned domain

@gopherbot
Copy link
Author

Closed by merging ebf5d98 to release-branch.go1.11.

gopherbot pushed a commit that referenced this issue Sep 1, 2018
…tions for unsigned domain

Fence-post implications of the form "x-1 >= w && x > min ⇒ x > w"
were not correctly handling unsigned domain, by always checking signed
limits.

This bug was uncovered once we taught prove that len(x) is always
>= 0 in the signed domain.

In the code being miscompiled (s[len(s)-1]), prove checks
whether len(s)-1 >= len(s) in the unsigned domain; if it proves
that this is always false, it can remove the bound check.

Notice that len(s)-1 >= len(s) can be true for len(s) = 0 because
of the wrap-around, so this is something prove should not be
able to deduce.

But because of the bug, the gate condition for the fence-post
implication was len(s) > MinInt64 instead of len(s) > 0; that
condition would be good in the signed domain but not in the
unsigned domain. And since in CL105635 we taught prove that
len(s) >= 0, the condition incorrectly triggered
(len(s) >= 0 > MinInt64) and things were going downfall.

Fixes #27378

Change-Id: I3dbcb1955ac5a66a0dcbee500f41e8d219409be5
Reviewed-on: https://go-review.googlesource.com/132495
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 09ea3c0)
Reviewed-on: https://go-review.googlesource.com/132575
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Sep 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickCandidate Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

1 participant