From 983ac4b08663ea9655abe99ca30faf47e54fdc16 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Wed, 13 Jan 2021 15:02:16 -0800 Subject: [PATCH] [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 Run-TryBot: Matthew Dempsky TryBot-Result: Go Bot Reviewed-by: Than McIntosh --- src/cmd/compile/internal/ir/expr.go | 3 ++ src/cmd/compile/internal/staticinit/sched.go | 33 +++++++++++--------- test/fixedbugs/issue43677.go | 18 +++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 test/fixedbugs/issue43677.go diff --git a/src/cmd/compile/internal/ir/expr.go b/src/cmd/compile/internal/ir/expr.go index 51425db42d..0639c3b620 100644 --- 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 ac0b6cd87e..64946ad247 100644 --- 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 index 0000000000..1a68c8b8b9 --- /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() -- 2.50.0