From d310b2a6b8a66eeb5953b1e682cf27669c8a08c2 Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Thu, 22 Apr 2021 17:14:10 +0700 Subject: [PATCH] cmd/compile: set correct Defn for inlined vars Currently, when copying definition node of an inlined var, we do not update var Defn field to point to new copied node. That causes all inlined vars point to the same Defn, and ir.StaticValue can not find inlined var in the lhs of its definition. clovar creates new ONAME node for local variables or params of closure inside inlined function, by copying most of the old node fields. So the new Node.Defn is not modified, its lhs still refer to old node instead of new one. To fix this, we need to do two things: - In subst.clovar, set a dummy Defn node for inlvar - During subst.node, when seeing OAS/OAS2 nodes, after substituting, we check if any node in lhs has the dummy Defn, then set it to the current OAS/OAS2 node. Fixes #45606 Change-Id: Ib517b753a7643756dcd61d36deae60f1a0fc53c5 Reviewed-on: https://go-review.googlesource.com/c/go/+/312630 Trust: Cuong Manh Le Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/inline/inl.go | 31 +++++++++++++++++++++++++- test/fixedbugs/issue45606.go | 17 ++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/fixedbugs/issue45606.go diff --git a/src/cmd/compile/internal/inline/inl.go b/src/cmd/compile/internal/inline/inl.go index 3cbe932d552..54fcb2b830d 100644 --- a/src/cmd/compile/internal/inline/inl.go +++ b/src/cmd/compile/internal/inline/inl.go @@ -1002,6 +1002,7 @@ func mkinlcall(n *ir.CallExpr, fn *ir.Func, maxCost int32, inlMap map[*ir.Func]b retvars: retvars, delayretvars: delayretvars, inlvars: inlvars, + defnMarker: ir.NilExpr{}, bases: make(map[*src.PosBase]*src.PosBase), newInlIndex: newIndex, fn: fn, @@ -1103,6 +1104,10 @@ type inlsubst struct { delayretvars bool inlvars map[*ir.Name]*ir.Name + // defnMarker is used to mark a Node for reassignment. + // inlsubst.clovar set this during creating new ONAME. + // inlsubst.node will set the correct Defn for inlvar. + defnMarker ir.NilExpr // bases maps from original PosBase to PosBase with an extra // inlined call frame. @@ -1160,7 +1165,11 @@ func (subst *inlsubst) clovar(n *ir.Name) *ir.Name { m := &ir.Name{} *m = *n m.Curfn = subst.newclofn - if n.Defn != nil && n.Defn.Op() == ir.ONAME { + + switch defn := n.Defn.(type) { + case nil: + // ok + case *ir.Name: if !n.IsClosureVar() { base.FatalfAt(n.Pos(), "want closure variable, got: %+v", n) } @@ -1182,7 +1191,13 @@ func (subst *inlsubst) clovar(n *ir.Name) *ir.Name { if subst.inlvars[n.Defn.(*ir.Name)] != nil { m.Defn = subst.node(n.Defn) } + case *ir.AssignStmt, *ir.AssignListStmt: + // Mark node for reassignment at the end of inlsubst.node. + m.Defn = &subst.defnMarker + default: + base.FatalfAt(n.Pos(), "unexpected Defn: %+v", defn) } + if n.Outer != nil { // Either the outer variable is defined in function being inlined, // and we will replace it with the substituted variable, or it is @@ -1406,6 +1421,20 @@ func (subst *inlsubst) node(n ir.Node) ir.Node { m := ir.Copy(n) m.SetPos(subst.updatedPos(m.Pos())) ir.EditChildren(m, subst.edit) + + switch m := m.(type) { + case *ir.AssignStmt: + if lhs, ok := m.X.(*ir.Name); ok && lhs.Defn == &subst.defnMarker { + lhs.Defn = m + } + case *ir.AssignListStmt: + for _, lhs := range m.Lhs { + if lhs, ok := lhs.(*ir.Name); ok && lhs.Defn == &subst.defnMarker { + lhs.Defn = m + } + } + } + return m } diff --git a/test/fixedbugs/issue45606.go b/test/fixedbugs/issue45606.go new file mode 100644 index 00000000000..1b52b4e7d5a --- /dev/null +++ b/test/fixedbugs/issue45606.go @@ -0,0 +1,17 @@ +// 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. + +package p + +func x() { + func() func() { + return func() { + f := func() {} + g, _ := f, 0 + g() + } + }()() +}