go/ssa: allows right operand of a shift to be signed.

Removes the forced conversion of the right operand of a shift to an unsigned type. This allows for clients to correctly model the runtime panic when a signed shift count is negative.

Fixes golang/go#51363

Change-Id: If59000eeb503fd45cdc6d4143dcc249242e7a957
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387995
Trust: Tim King <[email protected]>
Run-TryBot: Tim King <[email protected]>
gopls-CI: kokoro <[email protected]>
Reviewed-by: Zvonimir Pavlinovic <[email protected]>
Reviewed-by: Dominik Honnef <[email protected]>
Trust: Dominik Honnef <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: timothy-king

go/ssa/emit.go

@@ -74,9 +74,16 @@ func emitArith(f *Function, op token.Token, x, y Value, t types.Type, pos token.
 	case token.SHL, token.SHR:
 		x = emitConv(f, x, t)
 		// y may be signed or an 'untyped' constant.
-		// TODO(adonovan): whence signed values?
-		if b, ok := y.Type().Underlying().(*types.Basic); ok && b.Info()&types.IsUnsigned == 0 {
-			y = emitConv(f, y, types.Typ[types.Uint64])
+
+		// There is a runtime panic if y is signed and <0. Instead of inserting a check for y<0
+		// and converting to an unsigned value (like the compiler) leave y as is.
+
+		if b, ok := y.Type().Underlying().(*types.Basic); ok && b.Info()&types.IsUntyped != 0 {
+			// Untyped conversion:
+			// Spec https://go.dev/ref/spec#Operators:
+			// The right operand in a shift expression must have integer type or be an untyped constant
+			// representable by a value of type uint.
+			y = emitConv(f, y, types.Typ[types.Uint])
 		}
 
 	case token.ADD, token.SUB, token.MUL, token.QUO, token.REM, token.AND, token.OR, token.XOR, token.AND_NOT:

go/ssa/interp/interp_test.go

@@ -109,6 +109,7 @@ var gorootTestTests = []string{
 var testdataTests = []string{
 	"boundmeth.go",
 	"complit.go",
+	"convert.go",
 	"coverage.go",
 	"defer.go",
 	"fieldprom.go",

go/ssa/interp/ops.go

@@ -137,6 +137,26 @@ func asUint64(x value) uint64 {
 	panic(fmt.Sprintf("cannot convert %T to uint64", x))
 }
 
+// asUnsigned returns the value of x, which must be an integer type, as its equivalent unsigned type,
+// and returns true if x is non-negative.
+func asUnsigned(x value) (value, bool) {
+	switch x := x.(type) {
+	case int:
+		return uint(x), x >= 0
+	case int8:
+		return uint8(x), x >= 0
+	case int16:
+		return uint16(x), x >= 0
+	case int32:
+		return uint32(x), x >= 0
+	case int64:
+		return uint64(x), x >= 0
+	case uint, uint8, uint32, uint64, uintptr:
+		return x, true
+	}
+	panic(fmt.Sprintf("cannot convert %T to unsigned", x))
+}
+
 // zero returns a new "zero" value of the specified type.
 func zero(t types.Type) value {
 	switch t := t.(type) {
@@ -576,7 +596,11 @@ func binop(op token.Token, t types.Type, x, y value) value {
 		}
 
 	case token.SHL:
-		y := asUint64(y)
+		u, ok := asUnsigned(y)
+		if !ok {
+			panic("negative shift amount")
+		}
+		y := asUint64(u)
 		switch x.(type) {
 		case int:
 			return x.(int) << y
@@ -603,7 +627,11 @@ func binop(op token.Token, t types.Type, x, y value) value {
 		}
 
 	case token.SHR:
-		y := asUint64(y)
+		u, ok := asUnsigned(y)
+		if !ok {
+			panic("negative shift amount")
+		}
+		y := asUint64(u)
 		switch x.(type) {
 		case int:
 			return x.(int) >> y

go/ssa/interp/testdata/convert.go

@@ -0,0 +1,38 @@
+// Copyright 2022 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.
+
+// Test conversion operations.
+
+package main
+
+func left(x int)  { _ = 1 << x }
+func right(x int) { _ = 1 >> x }
+
+func main() {
+	wantPanic(
+		func() {
+			left(-1)
+		},
+		"runtime error: negative shift amount",
+	)
+	wantPanic(
+		func() {
+			right(-1)
+		},
+		"runtime error: negative shift amount",
+	)
+}
+
+func wantPanic(fn func(), s string) {
+	defer func() {
+		err := recover()
+		if err == nil {
+			panic("expected panic")
+		}
+		if got := err.(error).Error(); got != s {
+			panic("expected panic " + s + " got " + got)
+		}
+	}()
+	fn()
+}