]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.regabi] go/types: handle untyped constant arithmetic overflow
authorRob Findley <rfindley@google.com>
Mon, 8 Feb 2021 23:04:58 +0000 (18:04 -0500)
committerRobert Findley <rfindley@google.com>
Tue, 9 Feb 2021 14:15:25 +0000 (14:15 +0000)
This is a port of CL 287832 for go/types. It differs from that CL in its
handling of position data. Unlike the syntax package, which has a
unified Operation node, go/types checks operations for ast.UnaryExpr,
IncDecStmt, and BinaryExpr. It was simpler to keep the existing position
logic. Notably, this correctly puts the errors on the operator.

Change-Id: Id1e3aefe863da225eb0a9b3628cfc8a5684c0c4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/290569
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Trust: Robert Findley <rfindley@google.com>

src/go/types/expr.go
src/go/types/stdlib_test.go
src/go/types/testdata/const0.src

index f7fb0caeddb0e39bb5d0d2538425666cce6b0572..2741cc635dfabe68b16fa808fcf488b9936f97e1 100644 (file)
@@ -78,13 +78,60 @@ func (check *Checker) op(m opPredicates, x *operand, op token.Token) bool {
        return true
 }
 
+// overflow checks that the constant x is representable by its type.
+// For untyped constants, it checks that the value doesn't become
+// arbitrarily large.
+func (check *Checker) overflow(x *operand, op token.Token, opPos token.Pos) {
+       assert(x.mode == constant_)
+
+       what := "" // operator description, if any
+       if int(op) < len(op2str) {
+               what = op2str[op]
+       }
+
+       if x.val.Kind() == constant.Unknown {
+               // TODO(gri) We should report exactly what went wrong. At the
+               //           moment we don't have the (go/constant) API for that.
+               //           See also TODO in go/constant/value.go.
+               check.errorf(atPos(opPos), _InvalidConstVal, "constant result is not representable")
+               return
+       }
+
+       // Typed constants must be representable in
+       // their type after each constant operation.
+       if typ, ok := x.typ.Underlying().(*Basic); ok && isTyped(typ) {
+               check.representable(x, typ)
+               return
+       }
+
+       // Untyped integer values must not grow arbitrarily.
+       const limit = 4 * 512 // 512 is the constant precision - we need more because old tests had no limits
+       if x.val.Kind() == constant.Int && constant.BitLen(x.val) > limit {
+               check.errorf(atPos(opPos), _InvalidConstVal, "constant %s overflow", what)
+               x.val = constant.MakeUnknown()
+       }
+}
+
+// This is only used for operations that may cause overflow.
+var op2str = [...]string{
+       token.ADD: "addition",
+       token.SUB: "subtraction",
+       token.XOR: "bitwise XOR",
+       token.MUL: "multiplication",
+       token.SHL: "shift",
+}
+
 // The unary expression e may be nil. It's passed in for better error messages only.
-func (check *Checker) unary(x *operand, e *ast.UnaryExpr, op token.Token) {
-       switch op {
+func (check *Checker) unary(x *operand, e *ast.UnaryExpr) {
+       check.expr(x, e.X)
+       if x.mode == invalid {
+               return
+       }
+       switch e.Op {
        case token.AND:
                // spec: "As an exception to the addressability
                // requirement x may also be a composite literal."
-               if _, ok := unparen(x.expr).(*ast.CompositeLit); !ok && x.mode != variable {
+               if _, ok := unparen(e.X).(*ast.CompositeLit); !ok && x.mode != variable {
                        check.invalidOp(x, _UnaddressableOperand, "cannot take address of %s", x)
                        x.mode = invalid
                        return
@@ -111,26 +158,23 @@ func (check *Checker) unary(x *operand, e *ast.UnaryExpr, op token.Token) {
                return
        }
 
-       if !check.op(unaryOpPredicates, x, op) {
+       if !check.op(unaryOpPredicates, x, e.Op) {
                x.mode = invalid
                return
        }
 
        if x.mode == constant_ {
-               typ := x.typ.Underlying().(*Basic)
-               var prec uint
-               if isUnsigned(typ) {
-                       prec = uint(check.conf.sizeof(typ) * 8)
+               if x.val.Kind() == constant.Unknown {
+                       // nothing to do (and don't cause an error below in the overflow check)
+                       return
                }
-               x.val = constant.UnaryOp(op, x.val, prec)
-               // Typed constants must be representable in
-               // their type after each constant operation.
-               if isTyped(typ) {
-                       if e != nil {
-                               x.expr = e // for better error message
-                       }
-                       check.representable(x, typ)
+               var prec uint
+               if isUnsigned(x.typ) {
+                       prec = uint(check.conf.sizeof(x.typ) * 8)
                }
+               x.val = constant.UnaryOp(e.Op, x.val, prec)
+               x.expr = e
+               check.overflow(x, e.Op, x.Pos())
                return
        }
 
@@ -667,7 +711,8 @@ func (check *Checker) comparison(x, y *operand, op token.Token) {
        x.typ = Typ[UntypedBool]
 }
 
-func (check *Checker) shift(x, y *operand, e *ast.BinaryExpr, op token.Token) {
+// If e != nil, it must be the shift expression; it may be nil for non-constant shifts.
+func (check *Checker) shift(x, y *operand, e ast.Expr, op token.Token) {
        untypedx := isUntyped(x.typ)
 
        var xval constant.Value
@@ -735,14 +780,12 @@ func (check *Checker) shift(x, y *operand, e *ast.BinaryExpr, op token.Token) {
                        }
                        // x is a constant so xval != nil and it must be of Int kind.
                        x.val = constant.Shift(xval, op, uint(s))
-                       // Typed constants must be representable in
-                       // their type after each constant operation.
-                       if isTyped(x.typ) {
-                               if e != nil {
-                                       x.expr = e // for better error message
-                               }
-                               check.representable(x, x.typ.Underlying().(*Basic))
+                       x.expr = e
+                       opPos := x.Pos()
+                       if b, _ := e.(*ast.BinaryExpr); b != nil {
+                               opPos = b.OpPos
                        }
+                       check.overflow(x, op, opPos)
                        return
                }
 
@@ -803,8 +846,9 @@ var binaryOpPredicates = opPredicates{
        token.LOR:  isBoolean,
 }
 
-// The binary expression e may be nil. It's passed in for better error messages only.
-func (check *Checker) binary(x *operand, e *ast.BinaryExpr, lhs, rhs ast.Expr, op token.Token, opPos token.Pos) {
+// If e != nil, it must be the binary expression; it may be nil for non-constant expressions
+// (when invoked for an assignment operation where the binary expression is implicit).
+func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token.Token, opPos token.Pos) {
        var y operand
 
        check.expr(x, lhs)
@@ -879,30 +923,19 @@ func (check *Checker) binary(x *operand, e *ast.BinaryExpr, lhs, rhs ast.Expr, o
        }
 
        if x.mode == constant_ && y.mode == constant_ {
-               xval := x.val
-               yval := y.val
-               typ := x.typ.Underlying().(*Basic)
+               // if either x or y has an unknown value, the result is unknown
+               if x.val.Kind() == constant.Unknown || y.val.Kind() == constant.Unknown {
+                       x.val = constant.MakeUnknown()
+                       // x.typ is unchanged
+                       return
+               }
                // force integer division of integer operands
-               if op == token.QUO && isInteger(typ) {
+               if op == token.QUO && isInteger(x.typ) {
                        op = token.QUO_ASSIGN
                }
-               x.val = constant.BinaryOp(xval, op, yval)
-               // report error if valid operands lead to an invalid result
-               if xval.Kind() != constant.Unknown && yval.Kind() != constant.Unknown && x.val.Kind() == constant.Unknown {
-                       // TODO(gri) We should report exactly what went wrong. At the
-                       //           moment we don't have the (go/constant) API for that.
-                       //           See also TODO in go/constant/value.go.
-                       check.errorf(atPos(opPos), _InvalidConstVal, "constant result is not representable")
-                       // TODO(gri) Should we mark operands with unknown values as invalid?
-               }
-               // Typed constants must be representable in
-               // their type after each constant operation.
-               if isTyped(typ) {
-                       if e != nil {
-                               x.expr = e // for better error message
-                       }
-                       check.representable(x, typ)
-               }
+               x.val = constant.BinaryOp(x.val, op, y.val)
+               x.expr = e
+               check.overflow(x, op, opPos)
                return
        }
 
@@ -1538,11 +1571,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                }
 
        case *ast.UnaryExpr:
-               check.expr(x, e.X)
-               if x.mode == invalid {
-                       goto Error
-               }
-               check.unary(x, e, e.Op)
+               check.unary(x, e)
                if x.mode == invalid {
                        goto Error
                }
index 63e31f3d747219c18d6c00d1452ca5c13a0a1558..8a1e2905a7e1fea1115f09b511c4c29e2281dc88 100644 (file)
@@ -171,7 +171,6 @@ func TestStdFixed(t *testing.T) {
        testTestDir(t, filepath.Join(runtime.GOROOT(), "test", "fixedbugs"),
                "bug248.go", "bug302.go", "bug369.go", // complex test instructions - ignore
                "issue6889.go",   // gc-specific test
-               "issue7746.go",   // large constants - consumes too much memory
                "issue11362.go",  // canonical import path check
                "issue16369.go",  // go/types handles this correctly - not an issue
                "issue18459.go",  // go/types doesn't check validity of //go:xxx directives
index adbbf2863b03e55e0dfaeea860551f88dd7f1b1e..2916af54906eea19efeb9eacd653529d89db7f2d 100644 (file)
@@ -348,3 +348,10 @@ const _ = unsafe.Sizeof(func() {
        assert(one == 1)
        assert(iota == 0)
 })
+
+// untyped constants must not get arbitrarily large
+const (
+       huge = 1<<1000
+       _ = huge * huge * /* ERROR constant multiplication overflow */ huge
+       _ = huge << 1000 << /* ERROR constant shift overflow */ 1000
+)