[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 <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ilya Tocar <ilya.tocar@intel.com>
Reviewed-on: https://go-review.googlesource.com/103115
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/gc/asm_test.go b/src/cmd/compile/internal/gc/asm_test.go
index 08ec638f44ef7105d2dad1db5631c17fff5a2f79..95c354380ea970fb6f7315af72bf12e23c0cd3dc 100644 (file)
--- a/src/cmd/compile/internal/gc/asm_test.go
+++ b/src/cmd/compile/internal/gc/asm_test.go
@@ -909,6 +909,20 @@ var linuxAMD64Tests = []*asmTest{
                `,
                []string{"b\\+40\\(SP\\)"},
        },
+       {
+               `
+               func f73(a,b [3]int16) bool {
+                   return a == b
+               }`,
+               []string{"\tCMPL\t[A-Z]"},
+       },
+       {
+               `
+               func f74(a,b [12]int8) bool {
+                   return a == b
+               }`,
+               []string{"\tCMPQ\t[A-Z]", "\tCMPL\t[A-Z]"},
+       },
 }
 
 var linux386Tests = []*asmTest{

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index ab2e38208dd58536d31d1d065e7ef8bc11e5826e..a814ee4bf7fac50a28d4d2ef089fb24da765057c 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -3390,18 +3390,23 @@ func walkcompare(n *Node, init *Nodes) *Node {
                                i++
                                remains -= t.Elem().Width
                        } else {
+                               elemType := t.Elem().ToUnsigned()
                                cmplw := nod(OINDEX, cmpl, nodintconst(int64(i)))
-                               cmplw = conv(cmplw, convType)
+                               cmplw = conv(cmplw, elemType) // convert to unsigned
+                               cmplw = conv(cmplw, convType) // widen
                                cmprw := nod(OINDEX, cmpr, nodintconst(int64(i)))
+                               cmprw = conv(cmprw, elemType)
                                cmprw = conv(cmprw, convType)
                                // For code like this:  uint32(s[0]) | uint32(s[1])<<8 | uint32(s[2])<<16 ...
                                // ssa will generate a single large load.
                                for offset := int64(1); offset < step; offset++ {
                                        lb := nod(OINDEX, cmpl, nodintconst(int64(i+offset)))
+                                       lb = conv(lb, elemType)
                                        lb = conv(lb, convType)
                                        lb = nod(OLSH, lb, nodintconst(int64(8*t.Elem().Width*offset)))
                                        cmplw = nod(OOR, cmplw, lb)
                                        rb := nod(OINDEX, cmpr, nodintconst(int64(i+offset)))
+                                       rb = conv(rb, elemType)
                                        rb = conv(rb, convType)
                                        rb = nod(OLSH, rb, nodintconst(int64(8*t.Elem().Width*offset)))
                                        cmprw = nod(OOR, cmprw, rb)

diff --git a/test/fixedbugs/issue23719.go b/test/fixedbugs/issue23719.go
new file mode 100644 (file)
index 0000000..c97e636
--- /dev/null
+++ b/test/fixedbugs/issue23719.go
@@ -0,0 +1,42 @@
+// run
+
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+func main() {
+       v1 := [2]int32{-1, 88}
+       v2 := [2]int32{-1, 99}
+       if v1 == v2 {
+               panic("bad comparison")
+       }
+
+       w1 := [2]int16{-1, 88}
+       w2 := [2]int16{-1, 99}
+       if w1 == w2 {
+               panic("bad comparison")
+       }
+       x1 := [4]int16{-1, 88, 88, 88}
+       x2 := [4]int16{-1, 99, 99, 99}
+       if x1 == x2 {
+               panic("bad comparison")
+       }
+
+       a1 := [2]int8{-1, 88}
+       a2 := [2]int8{-1, 99}
+       if a1 == a2 {
+               panic("bad comparison")
+       }
+       b1 := [4]int8{-1, 88, 88, 88}
+       b2 := [4]int8{-1, 99, 99, 99}
+       if b1 == b2 {
+               panic("bad comparison")
+       }
+       c1 := [8]int8{-1, 88, 88, 88, 88, 88, 88, 88}
+       c2 := [8]int8{-1, 99, 99, 99, 99, 99, 99, 99}
+       if c1 == c2 {
+               panic("bad comparison")
+       }
+}