go/ssa: removes conversion of index value in Index and IndexAddr to int

Removes the forced conversion of the Index field in Index and IndexAddr to type int. ssa/interp now intprets indices as int64s instead of ints.

Converts untyped indices for strings to ints before the lookup.

Fixes golang/go#50949

Change-Id: Ib5d7f1ad28728d16c8e0a8411014d1209c62a3f2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/387996
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Tim King <taking@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Tim King <taking@google.com>

go/ssa/builder.go

@@ -407,9 +407,13 @@ func (b *builder) addr(fn *Function, e ast.Expr, escaping bool) lvalue {
 		default:
 			panic("unexpected container type in IndexExpr: " + t.String())
 		}
+		index := b.expr(fn, e.Index)
+		if isUntyped(index.Type()) {
+			index = emitConv(fn, index, tInt)
+		}
 		v := &IndexAddr{
 			X:     x,
-			Index: emitConv(fn, b.expr(fn, e.Index), tInt),
+			Index: index,
 		}
 		v.setPos(e.Lbrack)
 		v.setType(et)
@@ -748,9 +752,13 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value {
 		switch t := fn.typeOf(e.X).Underlying().(type) {
 		case *types.Array:
 			// Non-addressable array (in a register).
+			index := b.expr(fn, e.Index)
+			if isUntyped(index.Type()) {
+				index = emitConv(fn, index, tInt)
+			}
 			v := &Index{
 				X:     b.expr(fn, e.X),
-				Index: emitConv(fn, b.expr(fn, e.Index), tInt),
+				Index: index,
 			}
 			v.setPos(e.Lbrack)
 			v.setType(t.Elem())
@@ -769,9 +777,13 @@ func (b *builder) expr0(fn *Function, e ast.Expr, tv types.TypeAndValue) Value {
 
 		case *types.Basic: // => string
 			// Strings are not addressable.
+			index := b.expr(fn, e.Index)
+			if isUntyped(index.Type()) {
+				index = emitConv(fn, index, tInt)
+			}
 			v := &Lookup{
 				X:     b.expr(fn, e.X),
-				Index: b.expr(fn, e.Index),
+				Index: index,
 			}
 			v.setPos(e.Lbrack)
 			v.setType(tByte)

go/ssa/emit.go

@@ -78,7 +78,7 @@ func emitArith(f *Function, op token.Token, x, y Value, t types.Type, pos token.
 		// 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 {
+		if isUntyped(y.Type().Underlying()) {
 			// 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

go/ssa/interp/interp.go

@@ -279,7 +279,7 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 		}()
 
 	case *ssa.MakeChan:
-		fr.env[instr] = make(chan value, asInt(fr.get(instr.Size)))
+		fr.env[instr] = make(chan value, asInt64(fr.get(instr.Size)))
 
 	case *ssa.Alloc:
 		var addr *value
@@ -294,17 +294,20 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 		*addr = zero(deref(instr.Type()))
 
 	case *ssa.MakeSlice:
-		slice := make([]value, asInt(fr.get(instr.Cap)))
+		slice := make([]value, asInt64(fr.get(instr.Cap)))
 		tElt := instr.Type().Underlying().(*types.Slice).Elem()
 		for i := range slice {
 			slice[i] = zero(tElt)
 		}
-		fr.env[instr] = slice[:asInt(fr.get(instr.Len))]
+		fr.env[instr] = slice[:asInt64(fr.get(instr.Len))]
 
 	case *ssa.MakeMap:
-		reserve := 0
+		var reserve int64
 		if instr.Reserve != nil {
-			reserve = asInt(fr.get(instr.Reserve))
+			reserve = asInt64(fr.get(instr.Reserve))
+		}
+		if !fitsInt(reserve, fr.i.sizes) {
+			panic(fmt.Sprintf("ssa.MakeMap.Reserve value %d does not fit in int", reserve))
 		}
 		fr.env[instr] = makeMap(instr.Type().Underlying().(*types.Map).Key(), reserve)
 
@@ -325,15 +328,15 @@ func visitInstr(fr *frame, instr ssa.Instruction) continuation {
 		idx := fr.get(instr.Index)
 		switch x := x.(type) {
 		case []value:
-			fr.env[instr] = &x[asInt(idx)]
+			fr.env[instr] = &x[asInt64(idx)]
 		case *value: // *array
-			fr.env[instr] = &(*x).(array)[asInt(idx)]
+			fr.env[instr] = &(*x).(array)[asInt64(idx)]
 		default:
 			panic(fmt.Sprintf("unexpected x type in IndexAddr: %T", x))
 		}
 
 	case *ssa.Index:
-		fr.env[instr] = fr.get(instr.X).(array)[asInt(fr.get(instr.Index))]
+		fr.env[instr] = fr.get(instr.X).(array)[asInt64(fr.get(instr.Index))]
 
 	case *ssa.Lookup:
 		fr.env[instr] = lookup(instr, fr.get(instr.X), fr.get(instr.Index))

go/ssa/interp/interp_test.go

@@ -123,6 +123,13 @@ var testdataTests = []string{
 	"recover.go",
 	"reflect.go",
 	"static.go",
+	"width32.go",
+}
+
+// Specific GOARCH to use for a test case in go.tools/go/ssa/interp/testdata/.
+// Defaults to amd64 otherwise.
+var testdataArchs = map[string]string{
+	"width32.go": "386",
 }
 
 func run(t *testing.T, input string) bool {
@@ -140,6 +147,9 @@ func run(t *testing.T, input string) bool {
 	ctx.GOROOT = "testdata" // fake goroot
 	ctx.GOOS = "linux"
 	ctx.GOARCH = "amd64"
+	if arch, ok := testdataArchs[filepath.Base(input)]; ok {
+		ctx.GOARCH = arch
+	}
 
 	conf := loader.Config{Build: &ctx}
 	if _, err := conf.FromArgs([]string{input}, true); err != nil {
@@ -180,8 +190,9 @@ func run(t *testing.T, input string) bool {
 
 	interp.CapturedOutput = new(bytes.Buffer)
 
+	sizes := types.SizesFor("gc", ctx.GOARCH)
 	hint = fmt.Sprintf("To trace execution, run:\n%% go build golang.org/x/tools/cmd/ssadump && ./ssadump -build=C -test -run --interp=T %s\n", input)
-	exitCode := interp.Interpret(mainPkg, 0, &types.StdSizes{WordSize: 8, MaxAlign: 8}, input, []string{})
+	exitCode := interp.Interpret(mainPkg, 0, sizes, input, []string{})
 	if exitCode != 0 {
 		t.Fatalf("interpreting %s: exit code was %d", input, exitCode)
 	}

go/ssa/interp/map.go

@@ -38,7 +38,7 @@ type hashmap struct {
 
 // makeMap returns an empty initialized map of key type kt,
 // preallocating space for reserve elements.
-func makeMap(kt types.Type, reserve int) value {
+func makeMap(kt types.Type, reserve int64) value {
 	if usesBuiltinMap(kt) {
 		return make(map[value]value, reserve)
 	}

go/ssa/interp/ops.go

@@ -87,34 +87,46 @@ func constValue(c *ssa.Const) value {
 	panic(fmt.Sprintf("constValue: %s", c))
 }
 
-// asInt converts x, which must be an integer, to an int suitable for
-// use as a slice or array index or operand to make().
-func asInt(x value) int {
+// fitsInt returns true if x fits in type int according to sizes.
+func fitsInt(x int64, sizes types.Sizes) bool {
+	intSize := sizes.Sizeof(types.Typ[types.Int])
+	if intSize < sizes.Sizeof(types.Typ[types.Int64]) {
+		maxInt := int64(1)<<(intSize-1) - 1
+		minInt := -int64(1) << (intSize - 1)
+		return minInt <= x && x <= maxInt
+	}
+	return true
+}
+
+// asInt64 converts x, which must be an integer, to an int64.
+//
+// Callers that need a value directly usable as an int should combine this with fitsInt().
+func asInt64(x value) int64 {
 	switch x := x.(type) {
 	case int:
-		return x
+		return int64(x)
 	case int8:
-		return int(x)
+		return int64(x)
 	case int16:
-		return int(x)
+		return int64(x)
 	case int32:
-		return int(x)
+		return int64(x)
 	case int64:
-		return int(x)
+		return x
 	case uint:
-		return int(x)
+		return int64(x)
 	case uint8:
-		return int(x)
+		return int64(x)
 	case uint16:
-		return int(x)
+		return int64(x)
 	case uint32:
-		return int(x)
+		return int64(x)
 	case uint64:
-		return int(x)
+		return int64(x)
 	case uintptr:
-		return int(x)
+		return int64(x)
 	}
-	panic(fmt.Sprintf("cannot convert %T to int", x))
+	panic(fmt.Sprintf("cannot convert %T to int64", x))
 }
 
 // asUint64 converts x, which must be an unsigned integer, to a uint64
@@ -268,19 +280,19 @@ func slice(x, lo, hi, max value) value {
 		Cap = cap(a)
 	}
 
-	l := 0
+	l := int64(0)
 	if lo != nil {
-		l = asInt(lo)
+		l = asInt64(lo)
 	}
 
-	h := Len
+	h := int64(Len)
 	if hi != nil {
-		h = asInt(hi)
+		h = asInt64(hi)
 	}
 
-	m := Cap
+	m := int64(Cap)
 	if max != nil {
-		m = asInt(max)
+		m = asInt64(max)
 	}
 
 	switch x := x.(type) {
@@ -316,7 +328,7 @@ func lookup(instr *ssa.Lookup, x, idx value) value {
 		}
 		return v
 	case string:
-		return x[asInt(idx)]
+		return x[asInt64(idx)]
 	}
 	panic(fmt.Sprintf("unexpected x type in Lookup: %T", x))
 }

go/ssa/interp/testdata/convert.go

@@ -22,6 +22,15 @@ func main() {
 		},
 		"runtime error: negative shift amount",
 	)
+	wantPanic(
+		func() {
+			const maxInt32 = 1<<31 - 1
+			var idx int64 = maxInt32*2 + 8
+			x := make([]int, 16)
+			_ = x[idx]
+		},
+		"runtime error: runtime error: index out of range [4294967302] with length 16",
+	)
 }
 
 func wantPanic(fn func(), s string) {

go/ssa/interp/testdata/width32.go

@@ -0,0 +1,42 @@
+// 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 interpretation on 32 bit widths.
+
+package main
+
+func main() {
+	mapSize()
+}
+
+func mapSize() {
+	// Tests for the size argument of make on a map type.
+	const tooBigFor32 = 1<<33 - 1
+	wantPanic(
+		func() {
+			_ = make(map[int]int, int64(tooBigFor32))
+		},
+		"runtime error: ssa.MakeMap.Reserve value 8589934591 does not fit in int",
+	)
+
+	// TODO: Enable the following if sizeof(int) can be different for host and target.
+	// _ = make(map[int]int, tooBigFor32)
+	//
+	// Second arg to make in `make(map[int]int, tooBigFor32)` is an untyped int and
+	// is converted into an int explicitly in ssa.
+	// This has a different value on 32 and 64 bit systems.
+}
+
+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()
+}

go/ssa/util.go

@@ -64,6 +64,12 @@ func recvType(obj *types.Func) types.Type {
 	return obj.Type().(*types.Signature).Recv().Type()
 }
 
+// isUntyped returns true for types that are untyped.
+func isUntyped(typ types.Type) bool {
+	b, ok := typ.(*types.Basic)
+	return ok && b.Info()&types.IsUntyped != 0
+}
+
 // logStack prints the formatted "start" message to stderr and
 // returns a closure that prints the corresponding "end" message.
 // Call using 'defer logStack(...)()' to show builder stack on panic.