Skip to content

Commit f5c8db9

Browse files
randall77andybons
authored andcommitted
[release-branch.go1.9] 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]> Reviewed-on: https://go-review.googlesource.com/103115 Run-TryBot: Andrew Bonventre <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 9c69ed5 commit f5c8db9

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
@@ -909,6 +909,20 @@ var linuxAMD64Tests = []*asmTest{
909909
`,
910910
[]string{"b\\+40\\(SP\\)"},
911911
},
912+
{
913+
`
914+
func f73(a,b [3]int16) bool {
915+
return a == b
916+
}`,
917+
[]string{"\tCMPL\t[A-Z]"},
918+
},
919+
{
920+
`
921+
func f74(a,b [12]int8) bool {
922+
return a == b
923+
}`,
924+
[]string{"\tCMPQ\t[A-Z]", "\tCMPL\t[A-Z]"},
925+
},
912926
}
913927

914928
var linux386Tests = []*asmTest{

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3390,18 +3390,23 @@ func walkcompare(n *Node, init *Nodes) *Node {
33903390
i++
33913391
remains -= t.Elem().Width
33923392
} else {
3393+
elemType := t.Elem().ToUnsigned()
33933394
cmplw := nod(OINDEX, cmpl, nodintconst(int64(i)))
3394-
cmplw = conv(cmplw, convType)
3395+
cmplw = conv(cmplw, elemType) // convert to unsigned
3396+
cmplw = conv(cmplw, convType) // widen
33953397
cmprw := nod(OINDEX, cmpr, nodintconst(int64(i)))
3398+
cmprw = conv(cmprw, elemType)
33963399
cmprw = conv(cmprw, convType)
33973400
// For code like this: uint32(s[0]) | uint32(s[1])<<8 | uint32(s[2])<<16 ...
33983401
// ssa will generate a single large load.
33993402
for offset := int64(1); offset < step; offset++ {
34003403
lb := nod(OINDEX, cmpl, nodintconst(int64(i+offset)))
3404+
lb = conv(lb, elemType)
34013405
lb = conv(lb, convType)
34023406
lb = nod(OLSH, lb, nodintconst(int64(8*t.Elem().Width*offset)))
34033407
cmplw = nod(OOR, cmplw, lb)
34043408
rb := nod(OINDEX, cmpr, nodintconst(int64(i+offset)))
3409+
rb = conv(rb, elemType)
34053410
rb = conv(rb, convType)
34063411
rb = nod(OLSH, rb, nodintconst(int64(8*t.Elem().Width*offset)))
34073412
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)