From 23e8e197b0cd40312d96dd7576a44796f65dfb50 Mon Sep 17 00:00:00 2001
From: Keith Randall <khr@google.com>
Date: Tue, 6 Feb 2018 09:44:34 -0800
Subject: [PATCH] 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>
---
 src/cmd/compile/internal/gc/asm_test.go | 14 +++++++++
 src/cmd/compile/internal/gc/walk.go     |  7 ++++-
 test/fixedbugs/issue23719.go            | 42 +++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 test/fixedbugs/issue23719.go

diff --git a/src/cmd/compile/internal/gc/asm_test.go b/src/cmd/compile/internal/gc/asm_test.go
index 8eb3d07f2c..50857e6533 100644
--- a/src/cmd/compile/internal/gc/asm_test.go
+++ b/src/cmd/compile/internal/gc/asm_test.go
@@ -1002,6 +1002,20 @@ var linuxAMD64Tests = []*asmTest{
 		}`,
 		pos: []string{"\tCMPL\t[A-Z]"},
 	},
+	{
+		fn: `
+		func $(a,b [3]int16) bool {
+		    return a == b
+		}`,
+		pos: []string{"\tCMPL\t[A-Z]"},
+	},
+	{
+		fn: `
+		func $(a,b [12]int8) bool {
+		    return a == b
+		}`,
+		pos: []string{"\tCMPQ\t[A-Z]", "\tCMPL\t[A-Z]"},
+	},
 	{
 		fn: `
 		func f70(a,b [15]byte) bool {
diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index 34c73acce0..f48513dc73 100644
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -3415,18 +3415,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
index 0000000000..c97e63636c
--- /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")
+	}
+}