cmd/compile: fix constant index bounds check and error message

While here, rename nonnegintconst to indexconst (because that's
what it is) and add Fatalf calls where we are not expecting the
indexconst call to fail, and fixed wrong comparison in smallintconst.

Fixes #23781.

Change-Id: I86eb13081c450943b1806dfe3ae368872f76639a
Reviewed-on: https://go-review.googlesource.com/c/151599
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/const.go b/src/cmd/compile/internal/gc/const.go
index c01820506deb63729c628a2c185a79c803803ba1..3a9080e67d874d45ee21c5d0c94ccc4325d6583c 100644 (file)
--- a/src/cmd/compile/internal/gc/const.go
+++ b/src/cmd/compile/internal/gc/const.go
@@ -1229,6 +1229,7 @@ func strlit(n *Node) string {
        return n.Val().U.(string)
 }
 
+// TODO(gri) smallintconst is only used in one place - can we used indexconst?
 func smallintconst(n *Node) bool {
        if n.Op == OLITERAL && Isconst(n, CTINT) && n.Type != nil {
                switch simtype[n.Type.Etype] {
@@ -1243,7 +1244,7 @@ func smallintconst(n *Node) bool {
 
                case TIDEAL, TINT64, TUINT64, TPTR:
                        v, ok := n.Val().U.(*Mpint)
-                       if ok && v.Cmp(minintval[TINT32]) > 0 && v.Cmp(maxintval[TINT32]) < 0 {
+                       if ok && v.Cmp(minintval[TINT32]) >= 0 && v.Cmp(maxintval[TINT32]) <= 0 {
                                return true
                        }
                }
@@ -1252,21 +1253,24 @@ func smallintconst(n *Node) bool {
        return false
 }
 
-// nonnegintconst checks if Node n contains a constant expression
-// representable as a non-negative small integer, and returns its
-// (integer) value if that's the case. Otherwise, it returns -1.
-func nonnegintconst(n *Node) int64 {
+// indexconst checks if Node n contains a constant expression
+// representable as a non-negative int and returns its value.
+// If n is not a constant expression, not representable as an
+// integer, or negative, it returns -1. If n is too large, it
+// returns -2.
+func indexconst(n *Node) int64 {
        if n.Op != OLITERAL {
                return -1
        }
 
-       // toint will leave n.Val unchanged if it's not castable to an
-       // Mpint, so we still have to guard the conversion.
-       v := toint(n.Val())
+       v := toint(n.Val()) // toint returns argument unchanged if not representable as an *Mpint
        vi, ok := v.U.(*Mpint)
-       if !ok || vi.CmpInt64(0) < 0 || vi.Cmp(maxintval[TINT32]) > 0 {
+       if !ok || vi.CmpInt64(0) < 0 {
                return -1
        }
+       if vi.Cmp(maxintval[TINT]) > 0 {
+               return -2
+       }
 
        return vi.Int64()
 }

diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go
index acd8550ee3608294f71a655e07c7bf79ed2f4f34..56c63065b222ae4d2374864bf6351ed0be0fc8e4 100644 (file)
--- a/src/cmd/compile/internal/gc/sinit.go
+++ b/src/cmd/compile/internal/gc/sinit.go
@@ -711,7 +711,10 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
                var k int64
                splitnode = func(r *Node) (*Node, *Node) {
                        if r.Op == OKEY {
-                               k = nonnegintconst(r.Left)
+                               k = indexconst(r.Left)
+                               if k < 0 {
+                                       Fatalf("fixedlit: invalid index %v", r.Left)
+                               }
                                r = r.Right
                        }
                        a := nod(OINDEX, var_, nodintconst(k))
@@ -893,7 +896,10 @@ func slicelit(ctxt initContext, n *Node, var_ *Node, init *Nodes) {
        var index int64
        for _, value := range n.List.Slice() {
                if value.Op == OKEY {
-                       index = nonnegintconst(value.Left)
+                       index = indexconst(value.Left)
+                       if index < 0 {
+                               Fatalf("slicelit: invalid index %v", value.Left)
+                       }
                        value = value.Right
                }
                a := nod(OINDEX, vauto, nodintconst(index))
@@ -1250,7 +1256,10 @@ func initplan(n *Node) {
                var k int64
                for _, a := range n.List.Slice() {
                        if a.Op == OKEY {
-                               k = nonnegintconst(a.Left)
+                               k = indexconst(a.Left)
+                               if k < 0 {
+                                       Fatalf("initplan arraylit: invalid index %v", a.Left)
+                               }
                                a = a.Right
                        }
                        addvalue(p, k*n.Type.Elem().Width, a)
@@ -1260,7 +1269,7 @@ func initplan(n *Node) {
        case OSTRUCTLIT:
                for _, a := range n.List.Slice() {
                        if a.Op != OSTRUCTKEY {
-                               Fatalf("initplan fixedlit")
+                               Fatalf("initplan structlit")
                        }
                        addvalue(p, a.Xoffset, a.Left)
                }

diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go
index cbca68541577e878cd7ea0a4c6fb1e5021e3e3d8..633c5e5061a5ac97557ba072f6acc999656cbe93 100644 (file)
--- a/src/cmd/compile/internal/gc/typecheck.go
+++ b/src/cmd/compile/internal/gc/typecheck.go
@@ -3073,10 +3073,16 @@ func typecheckcomplit(n *Node) (res *Node) {
                        if l.Op == OKEY {
                                l.Left = typecheck(l.Left, ctxExpr)
                                evconst(l.Left)
-                               i = nonnegintconst(l.Left)
-                               if i < 0 && !l.Left.Diag() {
-                                       yyerror("index must be non-negative integer constant")
-                                       l.Left.SetDiag(true)
+                               i = indexconst(l.Left)
+                               if i < 0 {
+                                       if !l.Left.Diag() {
+                                               if i == -2 {
+                                                       yyerror("index too large")
+                                               } else {
+                                                       yyerror("index must be non-negative integer constant")
+                                               }
+                                               l.Left.SetDiag(true)
+                                       }
                                        i = -(1 << 30) // stay negative for a while
                                }
                                vp = &l.Right

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index e3fd71e389938d8464c76c3086479f5707c08c43..528aacb213c4b4ef9e0dd7ea1a7671a0175e4869 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -1305,7 +1305,11 @@ opswitch:
                        }
                        // var arr [r]T
                        // n = arr[:l]
-                       t = types.NewArray(t.Elem(), nonnegintconst(r)) // [r]T
+                       i := indexconst(r)
+                       if i < 0 {
+                               Fatalf("walkexpr: invalid index %v", r)
+                       }
+                       t = types.NewArray(t.Elem(), i) // [r]T
                        var_ := temp(t)
                        a := nod(OAS, var_, nil) // zero temp
                        a = typecheck(a, ctxStmt)

diff --git a/test/fixedbugs/issue23781.go b/test/fixedbugs/issue23781.go
new file mode 100644 (file)
index 0000000..5c03cf7
--- /dev/null
+++ b/test/fixedbugs/issue23781.go
@@ -0,0 +1,10 @@
+// +build amd64
+// compile
+
+// Copyright 2009 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.
+
+package p
+
+var _ = []int{1 << 31: 1} // ok on machines with 64bit int