cmd/compile: access to negative slice indices improperly permitted [1.13 backport] #34807
Labels
CherryPickApproved
Used during the release process for point releases
FrozenDueToAge
release-blocker
Milestone
Comments
gopherbot
added
the
CherryPickCandidate
|
Change https://golang.org/cl/201059 mentions this issue: |
|
Change https://golang.org/cl/201060 mentions this issue: |
toothrot
added
CherryPickApproved
|
Closed by merging 2bbb57c to release-branch.go1.13. |
|
Closed by merging c16e37e to release-branch.go1.13. |
gopherbot
pushed a commit
that referenced
this issue
Oct 14, 2019
… reaches The partially ordered set uses a method named 'dominates' to determine whether two nodes are partially ordered. Dominates does a depth-first search of the DAG, beginning at the source node, and returns true as soon as it finds a path to the target node. In the context of the forest-of-DAGs that makes up the poset, dominates is not necessarily checking dominance, but is checking reachability. See the issue tracker for a more detailed discussion of the difference. Fortunately, reachability is logically correct everywhere dominates is currently used in poset.go. Reachability within a DAG is sufficient to establish the partial ordering (source < target). This CL changes the name of the method (dominates -> reaches) and updates all the comments in the file accordingly. Updates #34807 Change-Id: Ia3a34f7b14b363801d75b05099cfc686035f7d96 Reviewed-on: https://go-review.googlesource.com/c/go/+/192617 Reviewed-by: Giovanni Bajo <[email protected]> Run-TryBot: Giovanni Bajo <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/201059 Run-TryBot: Alexander Rakoczy <[email protected]> Reviewed-by: Keith Randall <[email protected]>
gopherbot
pushed a commit
that referenced
this issue
Oct 14, 2019
…ions for OrderedOrEqual When assessing whether A <= B, the poset's OrderedOrEqual has a passing condition which permits A <= B, but is not sufficient to infer that A <= B. This CL removes that incorrect passing condition. Having identified that A and B are in the poset, the method will report that A <= B if any of these three conditions are true: (1) A and B are the same node in the poset. - This means we know that A == B. (2) There is a directed path, strict or not, from A -> B - This means we know that, at least, A <= B, but A < B is possible. (3) There is a directed path from B -> A, AND that path has no strict edges. - This means we know that B <= A, but do not know that B < A. In condition (3), we do not have enough information to say that A <= B, rather we only know that B == A (which satisfies A <= B) is possible. The way I understand it, a strict edge shows a known, strictly-ordered relation (<) but the lack of a strict edge does not show the lack of a strictly-ordered relation. The difference is highlighted by the example in #34802, where a bounds check is incorrectly removed by prove, such that negative indexes into a slice succeed: n := make([]int, 1) for i := -1; i <= 0; i++ { fmt.Printf("i is %d\n", i) n[i] = 1 // No Bounds check, program runs, assignment to n[-1] succeeds!! } When prove is checking the negative/failed branch from the bounds check at n[i], in the signed domain we learn (0 > i || i >= len(n)). Because prove can't learn the OR condition, we check whether we know that i is non-negative so we can learn something, namely that i >= len(n). Prove uses the poset to check whether we know that i is non-negative. At this point the poset holds the following relations as a directed graph: -1 <= i <= 0 -1 < 0 In poset.OrderedOrEqual, we are testing for 0 <= i. In this case, condition (3) above is true because there is a non-strict path from i -> 0, and that path does NOT have any strict edges. Because this condition is true, the poset reports to prove that i is known to be >= 0. Knowing, incorrectly, that i >= 0, prove learns from the failed bounds check that i >= len(n) in the signed domain. When the slice, n, was created, prove learned that len(n) == 1. Because i is also the induction variable for the loop, upon entering the loop, prove previously learned that i is in [-1,0]. So when prove attempts to learn from the failed bounds check, it finds the new fact, i > len(n), unsatisfiable given that it previously learned that i <= 0 and len(n) = 1. Fixes #34807 Change-Id: I235f4224bef97700c3aa5c01edcc595eb9f13afc Reviewed-on: https://go-review.googlesource.com/c/go/+/200759 Run-TryBot: Zach Jones <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Giovanni Bajo <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/201060 Run-TryBot: Alexander Rakoczy <[email protected]>
gopherbot
pushed a commit
that referenced
this issue
Oct 17, 2019
…inates to reaches The partially ordered set uses a method named 'dominates' to determine whether two nodes are partially ordered. Dominates does a depth-first search of the DAG, beginning at the source node, and returns true as soon as it finds a path to the target node. In the context of the forest-of-DAGs that makes up the poset, dominates is not necessarily checking dominance, but is checking reachability. See the issue tracker for a more detailed discussion of the difference. Fortunately, reachability is logically correct everywhere dominates is currently used in poset.go. Reachability within a DAG is sufficient to establish the partial ordering (source < target). This CL changes the name of the method (dominates -> reaches) and updates all the comments in the file accordingly. Updates #34807 Change-Id: Ia3a34f7b14b363801d75b05099cfc686035f7d96 Reviewed-on: https://go-review.googlesource.com/c/go/+/192617 Reviewed-by: Giovanni Bajo <[email protected]> Run-TryBot: Giovanni Bajo <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/201059 Run-TryBot: Alexander Rakoczy <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/575397 Reviewed-by: Filippo Valsorda <[email protected]>
gopherbot
pushed a commit
that referenced
this issue
Oct 17, 2019
…nt conditions for OrderedOrEqual When assessing whether A <= B, the poset's OrderedOrEqual has a passing condition which permits A <= B, but is not sufficient to infer that A <= B. This CL removes that incorrect passing condition. Having identified that A and B are in the poset, the method will report that A <= B if any of these three conditions are true: (1) A and B are the same node in the poset. - This means we know that A == B. (2) There is a directed path, strict or not, from A -> B - This means we know that, at least, A <= B, but A < B is possible. (3) There is a directed path from B -> A, AND that path has no strict edges. - This means we know that B <= A, but do not know that B < A. In condition (3), we do not have enough information to say that A <= B, rather we only know that B == A (which satisfies A <= B) is possible. The way I understand it, a strict edge shows a known, strictly-ordered relation (<) but the lack of a strict edge does not show the lack of a strictly-ordered relation. The difference is highlighted by the example in #34802, where a bounds check is incorrectly removed by prove, such that negative indexes into a slice succeed: n := make([]int, 1) for i := -1; i <= 0; i++ { fmt.Printf("i is %d\n", i) n[i] = 1 // No Bounds check, program runs, assignment to n[-1] succeeds!! } When prove is checking the negative/failed branch from the bounds check at n[i], in the signed domain we learn (0 > i || i >= len(n)). Because prove can't learn the OR condition, we check whether we know that i is non-negative so we can learn something, namely that i >= len(n). Prove uses the poset to check whether we know that i is non-negative. At this point the poset holds the following relations as a directed graph: -1 <= i <= 0 -1 < 0 In poset.OrderedOrEqual, we are testing for 0 <= i. In this case, condition (3) above is true because there is a non-strict path from i -> 0, and that path does NOT have any strict edges. Because this condition is true, the poset reports to prove that i is known to be >= 0. Knowing, incorrectly, that i >= 0, prove learns from the failed bounds check that i >= len(n) in the signed domain. When the slice, n, was created, prove learned that len(n) == 1. Because i is also the induction variable for the loop, upon entering the loop, prove previously learned that i is in [-1,0]. So when prove attempts to learn from the failed bounds check, it finds the new fact, i > len(n), unsatisfiable given that it previously learned that i <= 0 and len(n) = 1. Fixes #34807 Change-Id: I235f4224bef97700c3aa5c01edcc595eb9f13afc Reviewed-on: https://go-review.googlesource.com/c/go/+/200759 Run-TryBot: Zach Jones <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Giovanni Bajo <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/201060 Run-TryBot: Alexander Rakoczy <[email protected]> Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/575398 Reviewed-by: Filippo Valsorda <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
@ianlancetaylor requested issue #34802 to be considered for backport to the next 1.13 minor release.
The text was updated successfully, but these errors were encountered: