fix(gnovm): correct type for shift expression (#1775)

[shift operator where first operand is an untyped bigint always results
in a bigint](#1462) is not resolved
by #1426, it's fixed by this one.

=================================================================
1. This is a fix to /issues/1462;
3. **NOTE**: This PR should be reviewed following the potential merger
of #1426, from which it is both decoupled and dependent. #1426 serves as
base branch of this one.
4. **NOTE**: Currently, this PR displays all code including that from
#1426, because it is being compared to the master branch instead of
differing against #1426 directly.

---------

Co-authored-by: Morgan <[email protected]>
Co-authored-by: Marc Vertes <[email protected]>

gnovm/pkg/gnolang/eval_test.go

@@ -40,8 +40,8 @@ func TestEvalFiles(t *testing.T) {
 			if wantStacktrace != "" && !strings.Contains(stacktrace, wantStacktrace) {
 				t.Fatalf("unexpected stacktrace\nWant: %s\n Got: %s", wantStacktrace, stacktrace)
 			}
-			if wantOut != "" && out != wantOut {
-				t.Fatalf("unexpected output\nWant: %s\n Got: %s", wantOut, out)
+			if wantOut != "" && strings.TrimSpace(out) != strings.TrimSpace(wantOut) {
+				t.Fatalf("unexpected output\nWant: \"%s\"\n Got: \"%s\"", wantOut, out)
 			}
 		})
 

gnovm/pkg/gnolang/nodes.go

@@ -2119,6 +2119,7 @@ const (
 	ATTR_IOTA         GnoAttribute = "ATTR_IOTA"
 	ATTR_LOCATIONED   GnoAttribute = "ATTR_LOCATIONED"
 	ATTR_INJECTED     GnoAttribute = "ATTR_INJECTED"
+	ATTR_SHIFT_RHS    GnoAttribute = "ATTR_SHIFT_RHS"
 )
 
 var rePkgName = regexp.MustCompile(`^[a-z][a-z0-9_]+$`)

gnovm/pkg/gnolang/op_binary.go

@@ -1097,6 +1097,7 @@ func xorAssign(lv, rv *TypedValue) {
 
 // for doOpShl and doOpShlAssign.
 func shlAssign(lv, rv *TypedValue) {
+	rv.AssertNonNegative("runtime error: negative shift amount")
 	// set the result in lv.
 	// NOTE: baseOf(rv.T) is always UintType.
 	switch baseOf(lv.T) {
@@ -1136,6 +1137,7 @@ func shlAssign(lv, rv *TypedValue) {
 
 // for doOpShr and doOpShrAssign.
 func shrAssign(lv, rv *TypedValue) {
+	rv.AssertNonNegative("runtime error: negative shift amount")
 	// set the result in lv.
 	// NOTE: baseOf(rv.T) is always UintType.
 	switch baseOf(lv.T) {

gnovm/pkg/gnolang/op_call.go

@@ -27,6 +27,11 @@ func (m *Machine) doOpPrecall() {
 	case TypeValue:
 		// Do not pop type yet.
 		// No need for frames.
+		xv := m.PeekValue(1)
+		if cx.GetAttribute(ATTR_SHIFT_RHS) == true {
+			xv.AssertNonNegative("runtime error: negative shift amount")
+		}
+
 		m.PushOp(OpConvert)
 		if debug {
 			if len(cx.Args) != 1 {

gnovm/pkg/gnolang/preprocess.go

@@ -1025,7 +1025,7 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 				isShift := n.Op == SHL || n.Op == SHR
 				if isShift {
 					// check LHS type compatibility
-					n.checkShiftLhs(lt)
+					n.assertShiftExprCompatible1(store, last, lt, rt)
 					// checkOrConvert RHS
 					if baseOf(rt) != UintType {
 						// convert n.Right to (gno) uint type,
@@ -1036,6 +1036,7 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 							Op:    n.Op,
 							Right: rn,
 						}
+						n2.Right.SetAttribute(ATTR_SHIFT_RHS, true)
 						resn := Preprocess(store, last, n2)
 						return resn, TRANS_CONTINUE
 					}
@@ -1097,12 +1098,34 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 							// NOTE: binary operations are always computed in
 							// gno, never with reflect.
 						} else {
-							// convert n.Left to right type.
-							checkOrConvertType(store, last, &n.Left, rt, false)
+							// right is untyped const, left is not const, typed/untyped
+							checkUntypedShiftExpr := func(x Expr) {
+								if bx, ok := x.(*BinaryExpr); ok {
+									slt := evalStaticTypeOf(store, last, bx.Left)
+									if bx.Op == SHL || bx.Op == SHR {
+										srt := evalStaticTypeOf(store, last, bx.Right)
+										bx.assertShiftExprCompatible1(store, last, slt, srt)
+									}
+								}
+							}
+
+							if !isUntyped(rt) { // right is typed
+								checkOrConvertType(store, last, &n.Left, rt, false)
+							} else {
+								if shouldSwapOnSpecificity(lt, rt) {
+									checkUntypedShiftExpr(n.Right)
+								} else {
+									checkUntypedShiftExpr(n.Left)
+								}
+							}
 						}
 					} else if lcx.T == nil { // LHS is nil.
 						// convert n.Left to typed-nil type.
 						checkOrConvertType(store, last, &n.Left, rt, false)
+					} else {
+						if isUntyped(rt) {
+							checkOrConvertType(store, last, &n.Right, lt, false)
+						}
 					}
 				} else if ric { // right is const, left is not
 					if isUntyped(rcx.T) {
@@ -1134,12 +1157,33 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 							// NOTE: binary operations are always computed in
 							// gno, never with reflect.
 						} else {
-							// convert n.Right to left type.
-							checkOrConvertType(store, last, &n.Right, lt, false)
+							// right is untyped const, left is not const, typed or untyped
+							checkUntypedShiftExpr := func(x Expr) {
+								if bx, ok := x.(*BinaryExpr); ok {
+									if bx.Op == SHL || bx.Op == SHR {
+										srt := evalStaticTypeOf(store, last, bx.Right)
+										bx.assertShiftExprCompatible1(store, last, rt, srt)
+									}
+								}
+							}
+							// both untyped, e.g. 1<<s != 1.0
+							if !isUntyped(lt) { // left is typed
+								checkOrConvertType(store, last, &n.Right, lt, false)
+							} else { // if one side is untyped shift expression, check type with lower specificity
+								if shouldSwapOnSpecificity(lt, rt) {
+									checkUntypedShiftExpr(n.Right)
+								} else {
+									checkUntypedShiftExpr(n.Left)
+								}
+							}
 						}
 					} else if rcx.T == nil { // RHS is nil
 						// refer to tests/files/types/eql_0f20.gno
 						checkOrConvertType(store, last, &n.Right, lt, false)
+					} else { // left is not const, right is typed const
+						if isUntyped(lt) {
+							checkOrConvertType(store, last, &n.Left, rt, false)
+						}
 					}
 				} else {
 					// Left not const, Right not const ------------------
@@ -1267,27 +1311,28 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 						panic("type conversion requires single argument")
 					}
 					n.NumArgs = 1
-					if arg0, ok := n.Args[0].(*ConstExpr); ok {
-						var constConverted bool
-						ct := evalStaticType(store, last, n.Func)
+					ct := evalStaticType(store, last, n.Func)
+					at := evalStaticTypeOf(store, last, n.Args[0])
+					var constConverted bool
+					switch arg0 := n.Args[0].(type) {
+					case *ConstExpr:
 						// As a special case, if a decimal cannot
 						// be represented as an integer, it cannot be converted to one,
 						// and the error is handled here.
 						// Out of bounds errors are usually handled during evalConst().
-						switch ct.Kind() {
-						case IntKind, Int8Kind, Int16Kind, Int32Kind, Int64Kind,
-							UintKind, Uint8Kind, Uint16Kind, Uint32Kind, Uint64Kind,
-							BigintKind:
+						if isIntNum(ct) {
 							if bd, ok := arg0.TypedValue.V.(BigdecValue); ok {
 								if !isInteger(bd.V) {
 									panic(fmt.Sprintf(
 										"cannot convert %s to integer type",
 										arg0))
 								}
 							}
-							convertConst(store, last, arg0, ct)
-							constConverted = true
-						case SliceKind:
+							if isNumeric(at) {
+								convertConst(store, last, arg0, ct)
+								constConverted = true
+							}
+						} else if ct.Kind() == SliceKind {
 							if ct.Elem().Kind() == Uint8Kind { // bypass []byte("xxx")
 								n.SetAttribute(ATTR_TYPEOF_VALUE, ct)
 								return n, TRANS_CONTINUE
@@ -1298,18 +1343,34 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 						if !constConverted {
 							convertConst(store, last, arg0, nil)
 						}
-
 						// evaluate the new expression.
 						cx := evalConst(store, last, n)
 						// Though cx may be undefined if ct is interface,
 						// the ATTR_TYPEOF_VALUE is still interface.
 						cx.SetAttribute(ATTR_TYPEOF_VALUE, ct)
 						return cx, TRANS_CONTINUE
-					} else {
-						ct := evalStaticType(store, last, n.Func)
-						n.SetAttribute(ATTR_TYPEOF_VALUE, ct)
-						return n, TRANS_CONTINUE
+					case *BinaryExpr: // special case to evaluate type of binaryExpr/UnaryExpr which has untyped shift nested
+						if isUntyped(at) {
+							switch arg0.Op {
+							case EQL, NEQ, LSS, GTR, LEQ, GEQ:
+								assertAssignableTo(at, ct, false)
+								break
+							default:
+								checkOrConvertType(store, last, &n.Args[0], ct, false)
+							}
+						}
+					case *UnaryExpr:
+						if isUntyped(at) {
+							checkOrConvertType(store, last, &n.Args[0], ct, false)
+						}
+					default:
+						// do nothing
 					}
+					// general case, for non-const untyped && no nested untyped shift
+					// after handling const, and special cases recursively, set the target node type
+					// ct := evalStaticType(store, last, n.Func)
+					n.SetAttribute(ATTR_TYPEOF_VALUE, ct)
+					return n, TRANS_CONTINUE
 				default:
 					panic(fmt.Sprintf(
 						"unexpected func type %v (%v)",
@@ -1479,7 +1540,7 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 						}
 					}
 				} else {
-					for i := range n.Args {
+					for i := range n.Args { // iterate args
 						if hasVarg {
 							if (len(spts) - 1) <= i {
 								if isVarg {
@@ -1929,6 +1990,10 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 							} else {
 								last.Define(ln, anyValue(rt))
 							}
+							// if rhs is untyped
+							if isUntyped(rt) {
+								checkOrConvertType(store, last, &n.Rhs[i], nil, false)
+							}
 						}
 					}
 				} else { // ASSIGN, or assignment operation (+=, -=, <<=, etc.)
@@ -2018,9 +2083,6 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 						}
 					} else { // len(Lhs) == len(Rhs)
 						if n.Op == SHL_ASSIGN || n.Op == SHR_ASSIGN {
-							if len(n.Lhs) != 1 || len(n.Rhs) != 1 {
-								panic("should not happen")
-							}
 							// Special case if shift assign <<= or >>=.
 							convertType(store, last, &n.Rhs[0], UintType)
 						} else if n.Op == ADD_ASSIGN || n.Op == SUB_ASSIGN || n.Op == MUL_ASSIGN || n.Op == QUO_ASSIGN || n.Op == REM_ASSIGN {
@@ -2281,10 +2343,15 @@ func Preprocess(store Store, ctx BlockNode, n Node) Node {
 							vt := evalStaticTypeOf(store, last, vx)
 							sts[i] = vt
 						}
-					} else {
+					} else { // T is nil, n not const
 						// convert n.Value to default type.
 						for i, vx := range n.Values {
-							convertIfConst(store, last, vx)
+							if cx, ok := vx.(*ConstExpr); ok {
+								convertConst(store, last, cx, nil)
+								// convertIfConst(store, last, vx)
+							} else {
+								checkOrConvertType(store, last, &vx, nil, false)
+							}
 							vt := evalStaticTypeOf(store, last, vx)
 							sts[i] = vt
 						}
@@ -2840,9 +2907,25 @@ func checkOrConvertType(store Store, last BlockNode, x *Expr, t Type, autoNative
 			assertAssignableTo(cx.T, t, autoNative)
 		}
 	} else if bx, ok := (*x).(*BinaryExpr); ok && (bx.Op == SHL || bx.Op == SHR) {
-		// "push" expected type into shift binary's left operand. recursively.
-		checkOrConvertType(store, last, &bx.Left, t, autoNative)
-	} else if *x != nil { // XXX if x != nil && t != nil {
+		xt := evalStaticTypeOf(store, last, *x)
+		if debug {
+			debug.Printf("shift, xt: %v, Op: %v, t: %v \n", xt, bx.Op, t)
+		}
+		if isUntyped(xt) {
+			// check assignable first, see: types/shift_b6.gno
+			assertAssignableTo(xt, t, autoNative)
+
+			if t == nil || t.Kind() == InterfaceKind {
+				t = defaultTypeOf(xt)
+			}
+
+			bx.assertShiftExprCompatible2(t)
+			checkOrConvertType(store, last, &bx.Left, t, autoNative)
+		} else {
+			assertAssignableTo(xt, t, autoNative)
+		}
+		return
+	} else if *x != nil {
 		xt := evalStaticTypeOf(store, last, *x)
 		if t != nil {
 			assertAssignableTo(xt, t, autoNative)
@@ -2853,19 +2936,53 @@ func checkOrConvertType(store Store, last BlockNode, x *Expr, t Type, autoNative
 				switch bx.Op {
 				case ADD, SUB, MUL, QUO, REM, BAND, BOR, XOR,
 					BAND_NOT, LAND, LOR:
-					// push t into bx.Left and bx.Right
-					checkOrConvertType(store, last, &bx.Left, t, autoNative)
-					checkOrConvertType(store, last, &bx.Right, t, autoNative)
-					return
-				case SHL, SHR:
-					// push t into bx.Left
-					checkOrConvertType(store, last, &bx.Left, t, autoNative)
+					lt := evalStaticTypeOf(store, last, bx.Left)
+					rt := evalStaticTypeOf(store, last, bx.Right)
+					if t != nil {
+						// push t into bx.Left and bx.Right
+						checkOrConvertType(store, last, &bx.Left, t, autoNative)
+						checkOrConvertType(store, last, &bx.Right, t, autoNative)
+						return
+					} else {
+						if shouldSwapOnSpecificity(lt, rt) {
+							// e.g. 1.0<<s + 1
+							// The expression '1.0<<s' does not trigger assertions of
+							// incompatible types when evaluated alone.
+							// However, when evaluating the full expression '1.0<<s + 1'
+							// without a specific context type, '1.0<<s' is checked against
+							// its default type, the BigDecKind, will trigger assertion failure.
+							// so here in checkOrConvertType, shift expression is "finally" checked.
+							checkOrConvertType(store, last, &bx.Left, lt, autoNative)
+							checkOrConvertType(store, last, &bx.Right, lt, autoNative)
+						} else {
+							checkOrConvertType(store, last, &bx.Left, rt, autoNative)
+							checkOrConvertType(store, last, &bx.Right, rt, autoNative)
+						}
+					}
 					return
 				case EQL, LSS, GTR, NEQ, LEQ, GEQ:
-					// do nothing
+					lt := evalStaticTypeOf(store, last, bx.Left)
+					rt := evalStaticTypeOf(store, last, bx.Right)
+					if shouldSwapOnSpecificity(lt, rt) {
+						checkOrConvertType(store, last, &bx.Left, lt, autoNative)
+						checkOrConvertType(store, last, &bx.Right, lt, autoNative)
+					} else {
+						checkOrConvertType(store, last, &bx.Left, rt, autoNative)
+						checkOrConvertType(store, last, &bx.Right, rt, autoNative)
+					}
 				default:
 					// do nothing
 				}
+			} else if ux, ok := (*x).(*UnaryExpr); ok {
+				xt := evalStaticTypeOf(store, last, *x)
+				// check assignable first
+				assertAssignableTo(xt, t, autoNative)
+
+				if t == nil || t.Kind() == InterfaceKind {
+					t = defaultTypeOf(xt)
+				}
+				checkOrConvertType(store, last, &ux.X, t, autoNative)
+				return
 			}
 		}
 	}
@@ -2891,9 +3008,6 @@ func convertType(store Store, last BlockNode, x *Expr, t Type) {
 			if t == nil {
 				t = defaultTypeOf(xt)
 			}
-			if debug {
-				debug.Printf("default type of t: %v \n", t)
-			}
 			// convert x to destination type t
 			doConvertType(store, last, x, t)
 		} else {