cmd/compile: implement shifts by signed amounts

Allow shifts by signed amounts. Panic if the shift amount is negative.

TODO: We end up doing two compares per shift, see Ian's comment
https://github.com/golang/go/issues/19113#issuecomment-443241799 that
we could do it with a single comparison in the normal case.

The prove pass mostly handles this code well. For instance, it removes the
<0 check for cases like this:
    if s >= 0 { _ = x << s }
    _ = x << len(a)

This case isn't handled well yet:
    _ = x << (y & 0xf)
I'll do followon CLs for unhandled cases as needed.

Update #19113

R=go1.13

Change-Id: I839a5933d94b54ab04deb9dd5149f32c51c90fa1
Reviewed-on: https://go-review.googlesource.com/c/158719
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

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

@@ -13,6 +13,7 @@ var runtimeDecls = [...]struct {
 	{"panicindex", funcTag, 5},
 	{"panicslice", funcTag, 5},
 	{"panicdivide", funcTag, 5},
+	{"panicshift", funcTag, 5},
 	{"panicmakeslicelen", funcTag, 5},
 	{"throwinit", funcTag, 5},
 	{"panicwrap", funcTag, 5},

src/cmd/compile/internal/gc/builtin/runtime.go

@@ -18,6 +18,7 @@ func newobject(typ *byte) *any
 func panicindex()
 func panicslice()
 func panicdivide()
+func panicshift()
 func panicmakeslicelen()
 func throwinit()
 func panicwrap()

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

@@ -296,6 +296,7 @@ var (
 	msanwrite,
 	newproc,
 	panicdivide,
+	panicshift,
 	panicdottypeE,
 	panicdottypeI,
 	panicindex,

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

@@ -84,6 +84,7 @@ func initssaconfig() {
 	panicnildottype = sysfunc("panicnildottype")
 	panicoverflow = sysfunc("panicoverflow")
 	panicslice = sysfunc("panicslice")
+	panicshift = sysfunc("panicshift")
 	raceread = sysfunc("raceread")
 	racereadrange = sysfunc("racereadrange")
 	racewrite = sysfunc("racewrite")
@@ -2128,7 +2129,13 @@ func (s *state) expr(n *Node) *ssa.Value {
 	case OLSH, ORSH:
 		a := s.expr(n.Left)
 		b := s.expr(n.Right)
-		return s.newValue2(s.ssaShiftOp(n.Op, n.Type, n.Right.Type), a.Type, a, b)
+		bt := b.Type
+		if bt.IsSigned() {
+			cmp := s.newValue2(s.ssaOp(OGE, bt), types.Types[TBOOL], b, s.zeroVal(bt))
+			s.check(cmp, panicshift)
+			bt = bt.ToUnsigned()
+		}
+		return s.newValue2(s.ssaShiftOp(n.Op, n.Type, bt), a.Type, a, b)
 	case OANDAND, OOROR:
 		// To implement OANDAND (and OOROR), we introduce a
 		// new temporary variable to hold the result. The

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

@@ -660,8 +660,8 @@ func typecheck1(n *Node, top int) (res *Node) {
 			r = defaultlit(r, types.Types[TUINT])
 			n.Right = r
 			t := r.Type
-			if !t.IsInteger() || t.IsSigned() {
-				yyerror("invalid operation: %v (shift count type %v, must be unsigned integer)", n, r.Type)
+			if !t.IsInteger() {
+				yyerror("invalid operation: %v (shift count type %v, must be integer)", n, r.Type)
 				n.Type = nil
 				return n
 			}

src/cmd/compile/internal/ssa/rewrite.go

@@ -1115,7 +1115,8 @@ func needRaceCleanup(sym interface{}, v *Value) bool {
 			case OpStaticCall:
 				switch v.Aux.(fmt.Stringer).String() {
 				case "runtime.racefuncenter", "runtime.racefuncexit", "runtime.panicindex",
-					"runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap":
+					"runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap",
+					"runtime.panicshift":
 				// Check for racefuncenter will encounter racefuncexit and vice versa.
 				// Allow calls to panic*
 				default:

src/cmd/internal/obj/x86/obj6.go

@@ -968,7 +968,7 @@ func isZeroArgRuntimeCall(s *obj.LSym) bool {
 		return false
 	}
 	switch s.Name {
-	case "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap":
+	case "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap", "runtime.panicshift":
 		return true
 	}
 	return false

src/runtime/panic.go

@@ -23,16 +23,16 @@ func panicCheckMalloc(err error) {
 
 var indexError = error(errorString("index out of range"))
 
-// The panicindex, panicslice, and panicdivide functions are called by
+// The panic{index,slice,divide,shift} functions are called by
 // code generated by the compiler for out of bounds index expressions,
-// out of bounds slice expressions, and division by zero. The
-// panicdivide (again), panicoverflow, panicfloat, and panicmem
+// out of bounds slice expressions, division by zero, and shift by negative.
+// The panicdivide (again), panicoverflow, panicfloat, and panicmem
 // functions are called by the signal handler when a signal occurs
 // indicating the respective problem.
 //
-// Since panicindex and panicslice are never called directly, and
+// Since panic{index,slice,shift} are never called directly, and
 // since the runtime package should never have an out of bounds slice
-// or array reference, if we see those functions called from the
+// or array reference or negative shift, if we see those functions called from the
 // runtime package we turn the panic into a throw. That will dump the
 // entire runtime stack for easier debugging.
 
@@ -68,6 +68,16 @@ func panicoverflow() {
 	panic(overflowError)
 }
 
+var shiftError = error(errorString("negative shift amount"))
+
+func panicshift() {
+	if hasPrefix(funcname(findfunc(getcallerpc())), "runtime.") {
+		throw(string(shiftError.(errorString)))
+	}
+	panicCheckMalloc(shiftError)
+	panic(shiftError)
+}
+
 var floatError = error(errorString("floating point error"))
 
 func panicfloat() {

test/fixedbugs/bug073.go

@@ -1,4 +1,4 @@
-// errorcheck
+// compile
 
 // Copyright 2009 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
@@ -7,8 +7,8 @@
 package main
 
 func main() {
-	var s int = 0;
-	var x int = 0;
-	x = x << s;  // ERROR "illegal|inval|shift"
-	x = x >> s;  // ERROR "illegal|inval|shift"
+	var s int = 0
+	var x int = 0
+	x = x << s // as of 1.13, these are ok
+	x = x >> s // as of 1.13, these are ok
 }

test/fixedbugs/issue19113.go

@@ -0,0 +1,108 @@
+// run
+
+// Copyright 2019 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
+
+import "reflect"
+
+var tests = []interface{}{
+	func(x int, s int) int {
+		return x << s
+	},
+	func(x int, s int64) int {
+		return x << s
+	},
+	func(x int, s int32) int {
+		return x << s
+	},
+	func(x int, s int16) int {
+		return x << s
+	},
+	func(x int, s int8) int {
+		return x << s
+	},
+	func(x int, s int) int {
+		return x >> s
+	},
+	func(x int, s int64) int {
+		return x >> s
+	},
+	func(x int, s int32) int {
+		return x >> s
+	},
+	func(x int, s int16) int {
+		return x >> s
+	},
+	func(x int, s int8) int {
+		return x >> s
+	},
+	func(x uint, s int) uint {
+		return x << s
+	},
+	func(x uint, s int64) uint {
+		return x << s
+	},
+	func(x uint, s int32) uint {
+		return x << s
+	},
+	func(x uint, s int16) uint {
+		return x << s
+	},
+	func(x uint, s int8) uint {
+		return x << s
+	},
+	func(x uint, s int) uint {
+		return x >> s
+	},
+	func(x uint, s int64) uint {
+		return x >> s
+	},
+	func(x uint, s int32) uint {
+		return x >> s
+	},
+	func(x uint, s int16) uint {
+		return x >> s
+	},
+	func(x uint, s int8) uint {
+		return x >> s
+	},
+}
+
+func main() {
+	for _, t := range tests {
+		runTest(reflect.ValueOf(t))
+	}
+}
+
+func runTest(f reflect.Value) {
+	xt := f.Type().In(0)
+	st := f.Type().In(1)
+
+	for _, x := range []int{1, 0, -1} {
+		for _, s := range []int{-99, -64, -63, -32, -31, -16, -15, -8, -7, -1, 0, 1, 7, 8, 15, 16, 31, 32, 63, 64, 99} {
+			args := []reflect.Value{
+				reflect.ValueOf(x).Convert(xt),
+				reflect.ValueOf(s).Convert(st),
+			}
+			if s < 0 {
+				shouldPanic(func() {
+					f.Call(args)
+				})
+			} else {
+				f.Call(args) // should not panic
+			}
+		}
+	}
+}
+
+func shouldPanic(f func()) {
+	defer func() {
+		if recover() == nil {
+			panic("did not panic")
+		}
+	}()
+	f()
+}