Skip to content

Commit 23e8e19

Browse files
committed
cmd/compile: use unsigned loads for multi-element comparisons
When loading multiple elements of an array into a single register, make sure we treat them as unsigned. When treated as signed, the upper bits might all be set, causing the shift-or combo to clobber the values higher in the register. Fixes #23719. Change-Id: Ic87da03e9bd0fe2c60bb214b99f846e4e9446052 Reviewed-on: https://go-review.googlesource.com/92335 Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ilya Tocar <[email protected]>
1 parent 85bdd05 commit 23e8e19

File tree

3 files changed

+62
-1
lines changed

3 files changed

+62
-1
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,20 @@ var linuxAMD64Tests = []*asmTest{
10021002
}`,
10031003
pos: []string{"\tCMPL\t[A-Z]"},
10041004
},
1005+
{
1006+
fn: `
1007+
func $(a,b [3]int16) bool {
1008+
return a == b
1009+
}`,
1010+
pos: []string{"\tCMPL\t[A-Z]"},
1011+
},
1012+
{
1013+
fn: `
1014+
func $(a,b [12]int8) bool {
1015+
return a == b
1016+
}`,
1017+
pos: []string{"\tCMPQ\t[A-Z]", "\tCMPL\t[A-Z]"},
1018+
},
10051019
{
10061020
fn: `
10071021
func f70(a,b [15]byte) bool {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3415,18 +3415,23 @@ func walkcompare(n *Node, init *Nodes) *Node {
34153415
i++
34163416
remains -= t.Elem().Width
34173417
} else {
3418+
elemType := t.Elem().ToUnsigned()
34183419
cmplw := nod(OINDEX, cmpl, nodintconst(int64(i)))
3419-
cmplw = conv(cmplw, convType)
3420+
cmplw = conv(cmplw, elemType) // convert to unsigned
3421+
cmplw = conv(cmplw, convType) // widen
34203422
cmprw := nod(OINDEX, cmpr, nodintconst(int64(i)))
3423+
cmprw = conv(cmprw, elemType)
34213424
cmprw = conv(cmprw, convType)
34223425
// For code like this: uint32(s[0]) | uint32(s[1])<<8 | uint32(s[2])<<16 ...
34233426
// ssa will generate a single large load.
34243427
for offset := int64(1); offset < step; offset++ {
34253428
lb := nod(OINDEX, cmpl, nodintconst(int64(i+offset)))
3429+
lb = conv(lb, elemType)
34263430
lb = conv(lb, convType)
34273431
lb = nod(OLSH, lb, nodintconst(int64(8*t.Elem().Width*offset)))
34283432
cmplw = nod(OOR, cmplw, lb)
34293433
rb := nod(OINDEX, cmpr, nodintconst(int64(i+offset)))
3434+
rb = conv(rb, elemType)
34303435
rb = conv(rb, convType)
34313436
rb = nod(OLSH, rb, nodintconst(int64(8*t.Elem().Width*offset)))
34323437
cmprw = nod(OOR, cmprw, rb)

test/fixedbugs/issue23719.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
func main() {
10+
v1 := [2]int32{-1, 88}
11+
v2 := [2]int32{-1, 99}
12+
if v1 == v2 {
13+
panic("bad comparison")
14+
}
15+
16+
w1 := [2]int16{-1, 88}
17+
w2 := [2]int16{-1, 99}
18+
if w1 == w2 {
19+
panic("bad comparison")
20+
}
21+
x1 := [4]int16{-1, 88, 88, 88}
22+
x2 := [4]int16{-1, 99, 99, 99}
23+
if x1 == x2 {
24+
panic("bad comparison")
25+
}
26+
27+
a1 := [2]int8{-1, 88}
28+
a2 := [2]int8{-1, 99}
29+
if a1 == a2 {
30+
panic("bad comparison")
31+
}
32+
b1 := [4]int8{-1, 88, 88, 88}
33+
b2 := [4]int8{-1, 99, 99, 99}
34+
if b1 == b2 {
35+
panic("bad comparison")
36+
}
37+
c1 := [8]int8{-1, 88, 88, 88, 88, 88, 88, 88}
38+
c2 := [8]int8{-1, 99, 99, 99, 99, 99, 99, 99}
39+
if c1 == c2 {
40+
panic("bad comparison")
41+
}
42+
}

0 commit comments

Comments
 (0)