aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRob Findley <rfindley@google.com>2021-02-08 18:04:58 -0500
committerRobert Findley <rfindley@google.com>2021-02-09 14:15:25 +0000
commit1c58fcf7ed917f66e2b7f77f251e7e63ca9630e2 (patch)
tree83deef446bd0e46d40e9e9198e628ccd962e4b7c
parent493363ccff354ab5ed133f6d5fac942ba6cc034a (diff)
downloadgo-1c58fcf7ed917f66e2b7f77f251e7e63ca9630e2.tar.gz
go-1c58fcf7ed917f66e2b7f77f251e7e63ca9630e2.zip
[dev.regabi] go/types: handle untyped constant arithmetic overflow
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>
-rw-r--r--src/go/types/expr.go133
-rw-r--r--src/go/types/stdlib_test.go1
-rw-r--r--src/go/types/testdata/const0.src7
3 files changed, 88 insertions, 53 deletions
diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index f7fb0caedd..2741cc635d 100644
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -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
}
diff --git a/src/go/types/stdlib_test.go b/src/go/types/stdlib_test.go
index 63e31f3d74..8a1e2905a7 100644
--- a/src/go/types/stdlib_test.go
+++ b/src/go/types/stdlib_test.go
@@ -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
diff --git a/src/go/types/testdata/const0.src b/src/go/types/testdata/const0.src
index adbbf2863b..2916af5490 100644
--- a/src/go/types/testdata/const0.src
+++ b/src/go/types/testdata/const0.src
@@ -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
+)