diff options
author | Keith Randall <khr@google.com> | 2018-02-06 09:44:34 -0800 |
---|---|---|
committer | Andrew Bonventre <andybons@golang.org> | 2018-03-29 06:05:52 +0000 |
commit | f5c8db9643005344b9d1306588afea35c18140cc (patch) | |
tree | bfa49dcfca1deb1c78aec0116729139cbc0bd241 | |
parent | 9c69ed5d094bb8806b71076ab986c35df0a3f8a3 (diff) | |
download | go-f5c8db9643005344b9d1306588afea35c18140cc.tar.gz go-f5c8db9643005344b9d1306588afea35c18140cc.zip |
[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>
-rw-r--r-- | src/cmd/compile/internal/gc/asm_test.go | 14 | ||||
-rw-r--r-- | src/cmd/compile/internal/gc/walk.go | 7 | ||||
-rw-r--r-- | test/fixedbugs/issue23719.go | 42 |
3 files changed, 62 insertions, 1 deletions
diff --git a/src/cmd/compile/internal/gc/asm_test.go b/src/cmd/compile/internal/gc/asm_test.go index 08ec638f44..95c354380e 100644 --- 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 ab2e38208d..a814ee4bf7 100644 --- 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 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") + } +} |