Skip to content

Commit

Permalink
cmd/compile: set correct Defn for inlined vars
Browse files Browse the repository at this point in the history
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 <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
  • Loading branch information
cuonglm committed Apr 23, 2021
1 parent 1b0a031 commit d310b2a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
31 changes: 30 additions & 1 deletion src/cmd/compile/internal/inline/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
17 changes: 17 additions & 0 deletions test/fixedbugs/issue45606.go
Original file line number Diff line number Diff line change
@@ -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()
}
}()()
}

0 comments on commit d310b2a

Please sign in to comment.