[dev.regabi] cmd/compile: fix ICE when initializing blank vars

CL 278914 introduced NameOffsetExpr to avoid copying ONAME nodes and
hacking up their offsets, but evidently staticinit subtly depended on
the prior behavior to allow dynamic initialization of blank variables.

This CL refactors the code somewhat to avoid using NameOffsetExpr with
blank variables, and to instead create dynamic assignments directly to
the global blank node. It also adds a check to NewNameOffsetExpr to
guard against misuse like this, since I suspect there could be other
cases still lurking within staticinit. (This code is overdue for an
makeover anyway.)

Thanks to thanm@ for bisect and test case minimization.

Fixes #43677.

Change-Id: Ic71cb5d6698382feb9548dc3bb9fd606b207a172
Reviewed-on: https://go-review.googlesource.com/c/go/+/283537
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>

diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go
index 51425db42d43100d18cfca75d2fc6a135fdff4cd..0639c3b620a32a77a5f363c80a4023bcbe841556 100644 (file)
--- a/src/cmd/compile/internal/ir/expr.go
+++ b/src/cmd/compile/internal/ir/expr.go
@@ -473,6 +473,9 @@ type NameOffsetExpr struct {
 }
 
 func NewNameOffsetExpr(pos src.XPos, name *Name, offset int64, typ *types.Type) *NameOffsetExpr {
+       if name == nil || IsBlank(name) {
+               base.FatalfAt(pos, "cannot take offset of nil or blank name: %v", name)
+       }
        n := &NameOffsetExpr{Name_: name, Offset_: offset}
        n.typ = typ
        n.op = ONAMEOFFSET

diff --git a/src/cmd/compile/internal/staticinit/sched.go b/src/cmd/compile/internal/staticinit/sched.go
index ac0b6cd87efecf7af48ff544bcfd373b05539053..64946ad2476d8211a4130bc733d7c0f5b070f931 100644 (file)
--- a/src/cmd/compile/internal/staticinit/sched.go
+++ b/src/cmd/compile/internal/staticinit/sched.go
@@ -15,6 +15,7 @@ import (
        "cmd/compile/internal/typecheck"
        "cmd/compile/internal/types"
        "cmd/internal/obj"
+       "cmd/internal/src"
 )
 
 type Entry struct {
@@ -199,6 +200,20 @@ func (s *Schedule) StaticAssign(l *ir.Name, loff int64, r ir.Node, typ *types.Ty
                r = r.(*ir.ConvExpr).X
        }
 
+       assign := func(pos src.XPos, a *ir.Name, aoff int64, v ir.Node) {
+               if s.StaticAssign(a, aoff, v, v.Type()) {
+                       return
+               }
+               var lhs ir.Node
+               if ir.IsBlank(a) {
+                       // Don't use NameOffsetExpr with blank (#43677).
+                       lhs = ir.BlankNode
+               } else {
+                       lhs = ir.NewNameOffsetExpr(pos, a, aoff, v.Type())
+               }
+               s.append(ir.NewAssignStmt(pos, lhs, v))
+       }
+
        switch r.Op() {
        case ir.ONAME:
                r := r.(*ir.Name)
@@ -237,9 +252,7 @@ func (s *Schedule) StaticAssign(l *ir.Name, loff int64, r ir.Node, typ *types.Ty
                        staticdata.InitAddr(l, loff, a, 0)
 
                        // Init underlying literal.
-                       if !s.StaticAssign(a, 0, r.X, a.Type()) {
-                               s.append(ir.NewAssignStmt(base.Pos, a, r.X))
-                       }
+                       assign(base.Pos, a, 0, r.X)
                        return true
                }
                //dump("not static ptrlit", r);
@@ -278,10 +291,7 @@ func (s *Schedule) StaticAssign(l *ir.Name, loff int64, r ir.Node, typ *types.Ty
                                continue
                        }
                        ir.SetPos(e.Expr)
-                       if !s.StaticAssign(l, loff+e.Xoffset, e.Expr, e.Expr.Type()) {
-                               a := ir.NewNameOffsetExpr(base.Pos, l, loff+e.Xoffset, e.Expr.Type())
-                               s.append(ir.NewAssignStmt(base.Pos, a, e.Expr))
-                       }
+                       assign(base.Pos, l, loff+e.Xoffset, e.Expr)
                }
 
                return true
@@ -345,17 +355,12 @@ func (s *Schedule) StaticAssign(l *ir.Name, loff int64, r ir.Node, typ *types.Ty
                        }
                        // Copy val directly into n.
                        ir.SetPos(val)
-                       if !s.StaticAssign(l, loff+int64(types.PtrSize), val, val.Type()) {
-                               a := ir.NewNameOffsetExpr(base.Pos, l, loff+int64(types.PtrSize), val.Type())
-                               s.append(ir.NewAssignStmt(base.Pos, a, val))
-                       }
+                       assign(base.Pos, l, loff+int64(types.PtrSize), val)
                } else {
                        // Construct temp to hold val, write pointer to temp into n.
                        a := StaticName(val.Type())
                        s.Temps[val] = a
-                       if !s.StaticAssign(a, 0, val, val.Type()) {
-                               s.append(ir.NewAssignStmt(base.Pos, a, val))
-                       }
+                       assign(base.Pos, a, 0, val)
                        staticdata.InitAddr(l, loff+int64(types.PtrSize), a, 0)
                }
 

diff --git a/test/fixedbugs/issue43677.go b/test/fixedbugs/issue43677.go
new file mode 100644 (file)
index 0000000..1a68c8b
--- /dev/null
+++ b/test/fixedbugs/issue43677.go
@@ -0,0 +1,18 @@
+// compile
+
+// Copyright 2021 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.
+
+// Issue #43677: ICE during compilation of dynamic initializers for
+// composite blank variables.
+
+package p
+
+func f() *int
+
+var _ = [2]*int{nil, f()}
+
+var _ = struct{ x, y *int }{nil, f()}
+
+var _ interface{} = f()