From 2d41444ad0c1540f064695a9a4678dda6c0d2a51 Mon Sep 17 00:00:00 2001
From: zdjones <zachj1@gmail.com>
Date: Sat, 30 Mar 2019 17:28:05 +0000
Subject: [PATCH] cmd/compile: make prove learn index >= 0 from successful
 bounds checks
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

When branching at a bounds check for indexing or slicing ops, prove currently
only learns from the upper bound. On the positive branch, we currently learn
i < len(a) (or i <= len(a)) in both the signed and unsigned domains.

This CL makes prove also learn from the lower bound. Specifically, on the
positive branch from index or slicing ops, prove will now ALSO learn i >= 0 in
the signed domain (this fact is of no value in the unsigned domain).

The substantive change itself is only an additional call to addRestrictions,
though I've also inverted the nested switch statements around that call for the
sake of clarity.

This CL removes 92 bounds checks from std and cmd. It passes all tests and
shows no deltas on compilecmp.

Fixes #28885

Change-Id: I13eccc36e640eb599fa6dc5aa3be3c7d7abd2d9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/170121
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
---
 src/cmd/compile/internal/ssa/prove.go | 48 ++++++++++++++++-----------
 test/prove.go                         | 10 ++++++
 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go
index f70ec0c830..973e3cd4f2 100644
--- a/src/cmd/compile/internal/ssa/prove.go
+++ b/src/cmd/compile/internal/ssa/prove.go
@@ -931,31 +931,41 @@ func addBranchRestrictions(ft *factsTable, b *Block, br branch) {
 		if d == signed && ft.isNonNegative(c.Args[0]) && ft.isNonNegative(c.Args[1]) {
 			d |= unsigned
 		}
-		switch br {
-		case negative:
-			switch b.Control.Op { // Special cases
-			case OpIsInBounds, OpIsSliceInBounds:
-				// 0 <= a0 < a1 (or 0 <= a0 <= a1)
-				//
-				// On the positive branch, we learn a0 < a1,
-				// both signed and unsigned.
-				//
-				// On the negative branch, we learn (0 > a0 ||
-				// a0 >= a1). In the unsigned domain, this is
-				// simply a0 >= a1 (which is the reverse of the
-				// positive branch, so nothing surprising).
-				// But in the signed domain, we can't express the ||
-				// condition, so check if a0 is non-negative instead,
-				// to be able to learn something.
+		switch b.Control.Op {
+		case OpIsInBounds, OpIsSliceInBounds:
+			// 0 <= a0 < a1 (or 0 <= a0 <= a1)
+			//
+			// On the positive branch, we learn:
+			//   signed: 0 <= a0 < a1 (or 0 <= a0 <= a1)
+			//   unsigned:    a0 < a1 (or a0 <= a1)
+			//
+			// On the negative branch, we learn (0 > a0 ||
+			// a0 >= a1). In the unsigned domain, this is
+			// simply a0 >= a1 (which is the reverse of the
+			// positive branch, so nothing surprising).
+			// But in the signed domain, we can't express the ||
+			// condition, so check if a0 is non-negative instead,
+			// to be able to learn something.
+			switch br {
+			case negative:
 				d = unsigned
 				if ft.isNonNegative(c.Args[0]) {
 					d |= signed
 				}
+				addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
+			case positive:
+				addRestrictions(b, ft, signed, ft.zero, c.Args[0], lt|eq)
+				addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
+			}
+		default:
+			switch br {
+			case negative:
+				addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
+			case positive:
+				addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
 			}
-			addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
-		case positive:
-			addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
 		}
+
 	}
 }
 
diff --git a/test/prove.go b/test/prove.go
index 39b23c5e0a..6e92b9eec2 100644
--- a/test/prove.go
+++ b/test/prove.go
@@ -726,6 +726,16 @@ func signHint2(b []byte, n int) {
 	}
 }
 
+// indexGT0 tests whether prove learns int index >= 0 from bounds check.
+func indexGT0(b []byte, n int) {
+	_ = b[n]
+	_ = b[25]
+
+	for i := n; i <= 25; i++ { // ERROR "Induction variable: limits \[\?,25\], increment 1$"
+		b[i] = 123 // ERROR "Proved IsInBounds$"
+	}
+}
+
 // Induction variable in unrolled loop.
 func unrollUpExcl(a []int) int {
 	var i, x int
-- 
2.50.0