Skip to content

Commit bdc7d56

Browse files
dr2chaseFiloSottile
authored andcommitted
[release-branch.go1.11] cmd/compile: check for negative upper bound to IsSliceInBounds
IsSliceInBounds(x, y) asserts that y is not negative, but there were cases where this is not true. Change code generation to ensure that this is true when it's not obviously true. Prove phase cleans a few of these out. With this change the compiler text section is 0.06% larger, that is, not very much. Benchmarking still TBD, may need to wait for access to a benchmarking box (next week). Also corrected run.go to handle '?' in -update_errors output. Fixes #28799. Change-Id: Ia8af90bc50a91ae6e934ef973def8d3f398fac7b Reviewed-on: https://go-review.googlesource.com/c/152477 Run-TryBot: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> (cherry picked from commit ea6259d) Reviewed-on: https://go-review.googlesource.com/c/153638
1 parent b86522f commit bdc7d56

File tree

5 files changed

+131
-57
lines changed

5 files changed

+131
-57
lines changed

src/cmd/compile/internal/gc/ssa.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3863,13 +3863,34 @@ func (s *state) boundsCheck(idx, len *ssa.Value) {
38633863
s.check(cmp, panicindex)
38643864
}
38653865

3866+
func couldBeNegative(v *ssa.Value) bool {
3867+
switch v.Op {
3868+
case ssa.OpSliceLen, ssa.OpSliceCap, ssa.OpStringLen:
3869+
return false
3870+
case ssa.OpConst64:
3871+
return v.AuxInt < 0
3872+
case ssa.OpConst32:
3873+
return int32(v.AuxInt) < 0
3874+
}
3875+
return true
3876+
}
3877+
38663878
// sliceBoundsCheck generates slice bounds checking code. Checks if 0 <= idx <= len, branches to exit if not.
38673879
// Starts a new block on return.
38683880
// idx and len are already converted to full int width.
38693881
func (s *state) sliceBoundsCheck(idx, len *ssa.Value) {
38703882
if Debug['B'] != 0 {
38713883
return
38723884
}
3885+
if couldBeNegative(len) {
3886+
// OpIsSliceInBounds requires second arg not negative; if it's not obviously true, must check.
3887+
cmpop := ssa.OpGeq64
3888+
if len.Type.Size() == 4 {
3889+
cmpop = ssa.OpGeq32
3890+
}
3891+
cmp := s.newValue2(cmpop, types.Types[TBOOL], len, s.zeroVal(len.Type))
3892+
s.check(cmp, panicslice)
3893+
}
38733894

38743895
// bounds check
38753896
cmp := s.newValue2(ssa.OpIsSliceInBounds, types.Types[TBOOL], idx, len)

test/fixedbugs/issue28797.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// run
2+
3+
// Copyright 2018 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"fmt"
11+
)
12+
13+
// test expects f to panic, but not to run out of memory,
14+
// which is a non-panic fatal error. OOM results from failure
15+
// to properly check negative limit.
16+
func test(f func()) {
17+
defer func() {
18+
r := recover()
19+
if r == nil {
20+
panic("panic wasn't recoverable")
21+
}
22+
}()
23+
f()
24+
}
25+
26+
//go:noinline
27+
func id(x int) int {
28+
return x
29+
}
30+
31+
func main() {
32+
test(foo)
33+
test(bar)
34+
}
35+
36+
func foo() {
37+
b := make([]byte, 0)
38+
b = append(b, 1)
39+
id(len(b))
40+
id(len(b) - 2)
41+
s := string(b[1 : len(b)-2])
42+
fmt.Println(s)
43+
}
44+
45+
func bar() {
46+
b := make([]byte, 1)
47+
b = append(b, 1)
48+
i := id(-1)
49+
if i < len(b) { // establish value is not too large.
50+
s := string(b[1:i]) // should check for negative also.
51+
fmt.Println(s)
52+
}
53+
}

test/loopbce.go

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ package main
66
func f0a(a []int) int {
77
x := 0
88
for i := range a { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
9-
x += a[i] // ERROR "Proved IsInBounds$"
9+
x += a[i] // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
1010
}
1111
return x
1212
}
1313

1414
func f0b(a []int) int {
1515
x := 0
1616
for i := range a { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
17-
b := a[i:] // ERROR "Proved IsSliceInBounds$"
17+
b := a[i:] // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
1818
x += b[0]
1919
}
2020
return x
@@ -23,7 +23,7 @@ func f0b(a []int) int {
2323
func f0c(a []int) int {
2424
x := 0
2525
for i := range a { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
26-
b := a[:i+1] // ERROR "Proved IsSliceInBounds$"
26+
b := a[:i+1] // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
2727
x += b[0]
2828
}
2929
return x
@@ -40,15 +40,15 @@ func f1(a []int) int {
4040
func f2(a []int) int {
4141
x := 0
4242
for i := 1; i < len(a); i++ { // ERROR "Induction variable: limits \[1,\?\), increment 1$"
43-
x += a[i] // ERROR "Proved IsInBounds$"
43+
x += a[i] // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
4444
}
4545
return x
4646
}
4747

4848
func f4(a [10]int) int {
4949
x := 0
5050
for i := 0; i < len(a); i += 2 { // ERROR "Induction variable: limits \[0,10\), increment 2$"
51-
x += a[i] // ERROR "Proved IsInBounds$"
51+
x += a[i] // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
5252
}
5353
return x
5454
}
@@ -63,55 +63,55 @@ func f5(a [10]int) int {
6363

6464
func f6(a []int) {
6565
for i := range a { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
66-
b := a[0:i] // ERROR "Proved IsSliceInBounds$"
66+
b := a[0:i] // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$" "(\([0-9]+\) )?Proved Geq64$"
6767
f6(b)
6868
}
6969
}
7070

7171
func g0a(a string) int {
7272
x := 0
7373
for i := 0; i < len(a); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
74-
x += int(a[i]) // ERROR "Proved IsInBounds$"
74+
x += int(a[i]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
7575
}
7676
return x
7777
}
7878

7979
func g0b(a string) int {
8080
x := 0
8181
for i := 0; len(a) > i; i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
82-
x += int(a[i]) // ERROR "Proved IsInBounds$"
82+
x += int(a[i]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
8383
}
8484
return x
8585
}
8686

8787
func g0c(a string) int {
8888
x := 0
8989
for i := len(a); i > 0; i-- { // ERROR "Induction variable: limits \(0,\?\], increment 1$"
90-
x += int(a[i-1]) // ERROR "Proved IsInBounds$"
90+
x += int(a[i-1]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
9191
}
9292
return x
9393
}
9494

9595
func g0d(a string) int {
9696
x := 0
9797
for i := len(a); 0 < i; i-- { // ERROR "Induction variable: limits \(0,\?\], increment 1$"
98-
x += int(a[i-1]) // ERROR "Proved IsInBounds$"
98+
x += int(a[i-1]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
9999
}
100100
return x
101101
}
102102

103103
func g0e(a string) int {
104104
x := 0
105105
for i := len(a) - 1; i >= 0; i-- { // ERROR "Induction variable: limits \[0,\?\], increment 1$"
106-
x += int(a[i]) // ERROR "Proved IsInBounds$"
106+
x += int(a[i]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
107107
}
108108
return x
109109
}
110110

111111
func g0f(a string) int {
112112
x := 0
113113
for i := len(a) - 1; 0 <= i; i-- { // ERROR "Induction variable: limits \[0,\?\], increment 1$"
114-
x += int(a[i]) // ERROR "Proved IsInBounds$"
114+
x += int(a[i]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
115115
}
116116
return x
117117
}
@@ -120,7 +120,7 @@ func g1() int {
120120
a := "evenlength"
121121
x := 0
122122
for i := 0; i < len(a); i += 2 { // ERROR "Induction variable: limits \[0,10\), increment 2$"
123-
x += int(a[i]) // ERROR "Proved IsInBounds$"
123+
x += int(a[i]) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
124124
}
125125
return x
126126
}
@@ -130,7 +130,7 @@ func g2() int {
130130
x := 0
131131
for i := 0; i < len(a); i += 2 { // ERROR "Induction variable: limits \[0,10\), increment 2$"
132132
j := i
133-
if a[i] == 'e' { // ERROR "Proved IsInBounds$"
133+
if a[i] == 'e' { // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
134134
j = j + 1
135135
}
136136
x += int(a[j])
@@ -141,27 +141,27 @@ func g2() int {
141141
func g3a() {
142142
a := "this string has length 25"
143143
for i := 0; i < len(a); i += 5 { // ERROR "Induction variable: limits \[0,25\), increment 5$"
144-
useString(a[i:]) // ERROR "Proved IsSliceInBounds$"
144+
useString(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
145145
useString(a[:i+3])
146146
}
147147
}
148148

149149
func g3b(a string) {
150150
for i := 0; i < len(a); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
151-
useString(a[i+1:]) // ERROR "Proved IsSliceInBounds$"
151+
useString(a[i+1:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
152152
}
153153
}
154154

155155
func g3c(a string) {
156156
for i := 0; i < len(a); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
157-
useString(a[:i+1]) // ERROR "Proved IsSliceInBounds$"
157+
useString(a[:i+1]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
158158
}
159159
}
160160

161161
func h1(a []byte) {
162162
c := a[:128]
163163
for i := range c { // ERROR "Induction variable: limits \[0,128\), increment 1$"
164-
c[i] = byte(i) // ERROR "Proved IsInBounds$"
164+
c[i] = byte(i) // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
165165
}
166166
}
167167

@@ -174,25 +174,25 @@ func h2(a []byte) {
174174
func k0(a [100]int) [100]int {
175175
for i := 10; i < 90; i++ { // ERROR "Induction variable: limits \[10,90\), increment 1$"
176176
a[i-11] = i
177-
a[i-10] = i // ERROR "Proved IsInBounds$"
178-
a[i-5] = i // ERROR "Proved IsInBounds$"
179-
a[i] = i // ERROR "Proved IsInBounds$"
180-
a[i+5] = i // ERROR "Proved IsInBounds$"
181-
a[i+10] = i // ERROR "Proved IsInBounds$"
177+
a[i-10] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
178+
a[i-5] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
179+
a[i] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
180+
a[i+5] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
181+
a[i+10] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
182182
a[i+11] = i
183183
}
184184
return a
185185
}
186186

187187
func k1(a [100]int) [100]int {
188188
for i := 10; i < 90; i++ { // ERROR "Induction variable: limits \[10,90\), increment 1$"
189-
useSlice(a[:i-11])
190-
useSlice(a[:i-10]) // ERROR "Proved IsSliceInBounds$"
191-
useSlice(a[:i-5]) // ERROR "Proved IsSliceInBounds$"
192-
useSlice(a[:i]) // ERROR "Proved IsSliceInBounds$"
193-
useSlice(a[:i+5]) // ERROR "Proved IsSliceInBounds$"
194-
useSlice(a[:i+10]) // ERROR "Proved IsSliceInBounds$"
195-
useSlice(a[:i+11]) // ERROR "Proved IsSliceInBounds$"
189+
useSlice(a[:i-11]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
190+
useSlice(a[:i-10]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
191+
useSlice(a[:i-5]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
192+
useSlice(a[:i]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$" "(\([0-9]+\) )?Proved Geq64$"
193+
useSlice(a[:i+5]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
194+
useSlice(a[:i+10]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
195+
useSlice(a[:i+11]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
196196
useSlice(a[:i+12])
197197

198198
}
@@ -202,12 +202,12 @@ func k1(a [100]int) [100]int {
202202
func k2(a [100]int) [100]int {
203203
for i := 10; i < 90; i++ { // ERROR "Induction variable: limits \[10,90\), increment 1$"
204204
useSlice(a[i-11:])
205-
useSlice(a[i-10:]) // ERROR "Proved IsSliceInBounds$"
206-
useSlice(a[i-5:]) // ERROR "Proved IsSliceInBounds$"
207-
useSlice(a[i:]) // ERROR "Proved IsSliceInBounds$"
208-
useSlice(a[i+5:]) // ERROR "Proved IsSliceInBounds$"
209-
useSlice(a[i+10:]) // ERROR "Proved IsSliceInBounds$"
210-
useSlice(a[i+11:]) // ERROR "Proved IsSliceInBounds$"
205+
useSlice(a[i-10:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
206+
useSlice(a[i-5:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
207+
useSlice(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
208+
useSlice(a[i+5:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
209+
useSlice(a[i+10:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
210+
useSlice(a[i+11:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
211211
useSlice(a[i+12:])
212212
}
213213
return a
@@ -216,7 +216,7 @@ func k2(a [100]int) [100]int {
216216
func k3(a [100]int) [100]int {
217217
for i := -10; i < 90; i++ { // ERROR "Induction variable: limits \[-10,90\), increment 1$"
218218
a[i+9] = i
219-
a[i+10] = i // ERROR "Proved IsInBounds$"
219+
a[i+10] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
220220
a[i+11] = i
221221
}
222222
return a
@@ -225,7 +225,7 @@ func k3(a [100]int) [100]int {
225225
func k3neg(a [100]int) [100]int {
226226
for i := 89; i > -11; i-- { // ERROR "Induction variable: limits \(-11,89\], increment 1$"
227227
a[i+9] = i
228-
a[i+10] = i // ERROR "Proved IsInBounds$"
228+
a[i+10] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
229229
a[i+11] = i
230230
}
231231
return a
@@ -234,7 +234,7 @@ func k3neg(a [100]int) [100]int {
234234
func k3neg2(a [100]int) [100]int {
235235
for i := 89; i >= -10; i-- { // ERROR "Induction variable: limits \[-10,89\], increment 1$"
236236
a[i+9] = i
237-
a[i+10] = i // ERROR "Proved IsInBounds$"
237+
a[i+10] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
238238
a[i+11] = i
239239
}
240240
return a
@@ -243,16 +243,16 @@ func k3neg2(a [100]int) [100]int {
243243
func k4(a [100]int) [100]int {
244244
min := (-1) << 63
245245
for i := min; i < min+50; i++ { // ERROR "Induction variable: limits \[-9223372036854775808,-9223372036854775758\), increment 1$"
246-
a[i-min] = i // ERROR "Proved IsInBounds$"
246+
a[i-min] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
247247
}
248248
return a
249249
}
250250

251251
func k5(a [100]int) [100]int {
252252
max := (1 << 63) - 1
253-
for i := max - 50; i < max; i++ { // ERROR "Induction variable: limits \[9223372036854775757,9223372036854775807\), increment 1"
254-
a[i-max+50] = i // ERROR "Proved IsInBounds$"
255-
a[i-(max-70)] = i // ERROR "Proved IsInBounds$"
253+
for i := max - 50; i < max; i++ { // ERROR "Induction variable: limits \[9223372036854775757,9223372036854775807\), increment 1$"
254+
a[i-max+50] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
255+
a[i-(max-70)] = i // ERROR "(\([0-9]+\) )?Proved IsInBounds$"
256256
}
257257
return a
258258
}
@@ -275,17 +275,17 @@ func nobce1() {
275275

276276
func nobce2(a string) {
277277
for i := int64(0); i < int64(len(a)); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
278-
useString(a[i:]) // ERROR "Proved IsSliceInBounds$"
278+
useString(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
279279
}
280280
for i := int64(0); i < int64(len(a))-31337; i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
281-
useString(a[i:]) // ERROR "Proved IsSliceInBounds$"
281+
useString(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
282282
}
283283
for i := int64(0); i < int64(len(a))+int64(-1<<63); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
284-
useString(a[i:]) // ERROR "Proved IsSliceInBounds$"
284+
useString(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
285285
}
286286
j := int64(len(a)) - 123
287287
for i := int64(0); i < j+123+int64(-1<<63); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
288-
useString(a[i:]) // ERROR "Proved IsSliceInBounds$"
288+
useString(a[i:]) // ERROR "(\([0-9]+\) )?Proved IsSliceInBounds$"
289289
}
290290
for i := int64(0); i < j+122+int64(-1<<63); i++ { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
291291
// len(a)-123+122+MinInt overflows when len(a) == 0, so a bound check is needed here

0 commit comments

Comments
 (0)