]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: don't accept incorrect shift expression arguments
authorgriesemer <gri@golang.org>
Fri, 22 Sep 2017 14:24:25 +0000 (16:24 +0200)
committerRobert Griesemer <gri@golang.org>
Mon, 25 Sep 2017 08:54:28 +0000 (08:54 +0000)
Under certain circumstances involving shifts, go/types didn't verify
that untyped constant values were representable by the relevant type,
leading to the acceptance of incorrect programs (see the issue).

Fixing this code exposed another problem with int-to-string conversions
which suddenly failed because now the type-checker complained that a
(constant) integer argument wasn't representable as a string. Fixed that
as well.

Added many additional tests covering the various scenarious.

Found two cmd/compile bugs in the process (#21979, #21981) and filed
a go/types TODO (#21982).

Fixes #21727.

Change-Id: If443ee0230979cd7d45d2fc669e623648caa70da
Reviewed-on: https://go-review.googlesource.com/65370
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/types/api_test.go
src/go/types/conversions.go
src/go/types/expr.go
src/go/types/testdata/shifts.src

index ab08a2669d521bd8067f5c8c3607d36bd99f19da..4f54f684b8a1459b8040667ba0cc063139bb5808 100644 (file)
@@ -87,6 +87,10 @@ func TestValuesInfo(t *testing.T) {
                {`package c5a; var _ = string("foo")`, `"foo"`, `string`, `"foo"`},
                {`package c5b; var _ = string("foo")`, `string("foo")`, `string`, `"foo"`},
                {`package c5c; type T string; var _ = T("foo")`, `T("foo")`, `c5c.T`, `"foo"`},
+               {`package c5d; var _ = string(65)`, `65`, `untyped int`, `65`},
+               {`package c5e; var _ = string('A')`, `'A'`, `untyped rune`, `65`},
+               {`package c5f; type T string; var _ = T('A')`, `'A'`, `untyped rune`, `65`},
+               {`package c5g; var s uint; var _ = string(1 << s)`, `1 << s`, `untyped int`, ``},
 
                {`package d0; var _ = []byte("foo")`, `"foo"`, `string`, `"foo"`},
                {`package d1; var _ = []byte(string("foo"))`, `"foo"`, `string`, `"foo"`},
@@ -122,7 +126,7 @@ func TestValuesInfo(t *testing.T) {
                }
                name := mustTypecheck(t, "ValuesInfo", test.src, &info)
 
-               // look for constant expression
+               // look for expression
                var expr ast.Expr
                for e := range info.Types {
                        if ExprString(e) == test.expr {
@@ -142,9 +146,15 @@ func TestValuesInfo(t *testing.T) {
                        continue
                }
 
-               // check that value is correct
-               if got := tv.Value.ExactString(); got != test.val {
-                       t.Errorf("package %s: got value %s; want %s", name, got, test.val)
+               // if we have a constant, check that value is correct
+               if tv.Value != nil {
+                       if got := tv.Value.ExactString(); got != test.val {
+                               t.Errorf("package %s: got value %s; want %s", name, got, test.val)
+                       }
+               } else {
+                       if test.val != "" {
+                               t.Errorf("package %s: no constant found; want %s", name, test.val)
+                       }
                }
        }
 }
index 2bf1e2d5e3825829864b337dce32bd8d4d6d1d14..81a65838fe0915ce74c0f6b7a9338aacec4c986d 100644 (file)
@@ -46,16 +46,19 @@ func (check *Checker) conversion(x *operand, T Type) {
        // The conversion argument types are final. For untyped values the
        // conversion provides the type, per the spec: "A constant may be
        // given a type explicitly by a constant declaration or conversion,...".
-       final := x.typ
        if isUntyped(x.typ) {
-               final = T
+               final := T
                // - For conversions to interfaces, use the argument's default type.
                // - For conversions of untyped constants to non-constant types, also
                //   use the default type (e.g., []byte("foo") should report string
                //   not []byte as type for the constant "foo").
                // - Keep untyped nil for untyped nil arguments.
+               // - For integer to string conversions, keep the argument type.
+               //   (See also the TODO below.)
                if IsInterface(T) || constArg && !isConstType(T) {
                        final = Default(x.typ)
+               } else if isInteger(x.typ) && isString(T) {
+                       final = x.typ
                }
                check.updateExprType(x.expr, final, true)
        }
@@ -63,6 +66,16 @@ func (check *Checker) conversion(x *operand, T Type) {
        x.typ = T
 }
 
+// TODO(gri) convertibleTo checks if T(x) is valid. It assumes that the type
+// of x is fully known, but that's not the case for say string(1<<s + 1.0):
+// Here, the type of 1<<s + 1.0 will be UntypedFloat which will lead to the
+// (correct!) refusal of the conversion. But the reported error is essentially
+// "cannot convert untyped float value to string", yet the correct error (per
+// the spec) is that we cannot shift a floating-point value: 1 in 1<<s should
+// be converted to UntypedFloat because of the addition of 1.0. Fixing this
+// is tricky because we'd have to run updateExprType on the argument first.
+// (Issue #21982.)
+
 func (x *operand) convertibleTo(conf *Config, T Type) bool {
        // "x is assignable to T"
        if x.assignableTo(conf, T, nil) {
index 461f0a525ba88fb525262cc8049aa0b9ff398488..01c97afc07898509debaabee3a88ca8561e578ef 100644 (file)
@@ -402,9 +402,10 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
 
        case *ast.UnaryExpr:
                // If x is a constant, the operands were constants.
-               // They don't need to be updated since they never
-               // get "materialized" into a typed value; and they
-               // will be processed at the end of the type check.
+               // The operands don't need to be updated since they
+               // never get "materialized" into a typed value. If
+               // left in the untyped map, they will be processed
+               // at the end of the type check.
                if old.val != nil {
                        break
                }
@@ -443,12 +444,21 @@ func (check *Checker) updateExprType(x ast.Expr, typ Type, final bool) {
        // Remove it from the map of yet untyped expressions.
        delete(check.untyped, x)
 
-       // If x is the lhs of a shift, its final type must be integer.
-       // We already know from the shift check that it is representable
-       // as an integer if it is a constant.
-       if old.isLhs && !isInteger(typ) {
-               check.invalidOp(x.Pos(), "shifted operand %s (type %s) must be integer", x, typ)
-               return
+       if old.isLhs {
+               // If x is the lhs of a shift, its final type must be integer.
+               // We already know from the shift check that it is representable
+               // as an integer if it is a constant.
+               if !isInteger(typ) {
+                       check.invalidOp(x.Pos(), "shifted operand %s (type %s) must be integer", x, typ)
+                       return
+               }
+       } else if old.val != nil {
+               // If x is a constant, it must be representable as a value of typ.
+               c := operand{old.mode, x, old.typ, old.val, 0}
+               check.convertUntyped(&c, typ)
+               if c.mode == invalid {
+                       return
+               }
        }
 
        // Everything's fine, record final type and value for x.
index dc029fc647aac53791aa6a3151c524e6e0cd071e..ca288290d69cea9bd6932f9858d33b017d760517 100644 (file)
@@ -345,3 +345,12 @@ func issue11594() {
        _ = complex64 /* ERROR "must be integer" */ (0) << 3
        _ = complex64 /* ERROR "must be integer" */ (0) >> 4
 }
+
+func issue21727() {
+       var s uint
+       var a = make([]int, 1<<s + 1.2 /* ERROR "truncated to int" */ )
+       var _ = a[1<<s - 2.3 /* ERROR "truncated to int" */ ]
+       var _ int = 1<<s + 3.4 /* ERROR "truncated to int" */
+       var _ = string(1 << s)
+       var _ = string(1.0 /* ERROR "cannot convert" */ << s)
+}