Skip to content

Commit

Permalink
[release-branch.go1.19] cmd/compile: only inline method wrapper if me…
Browse files Browse the repository at this point in the history
…thod don't contain closures

CL 327871 changes methodWrapper to always perform inlining after global
escape analysis. However, inlining the method may reveal closures, which
require walking all function bodies to decide whether to capture free
variables by value or by ref.

To fix it, just not doing inline if the method contains any closures.

Fixes #54726

Change-Id: I4b0255b86257cc6fe7e5fafbc545cc5cff9113e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/426334
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/426160
  • Loading branch information
cuonglm authored and heschi committed Aug 31, 2022
1 parent 4580d6d commit 11033ea
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 3 deletions.
21 changes: 18 additions & 3 deletions src/cmd/compile/internal/reflectdata/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -1890,14 +1890,14 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
// the TOC to the appropriate value for that module. But if it returns
// directly to the wrapper's caller, nothing will reset it to the correct
// value for that function.
var call *ir.CallExpr
if !base.Flag.Cfg.Instrumenting && rcvr.IsPtr() && methodrcvr.IsPtr() && method.Embedded != 0 && !types.IsInterfaceMethod(method.Type) && !(base.Ctxt.Arch.Name == "ppc64le" && base.Ctxt.Flag_dynlink) && !generic {
call := ir.NewCallExpr(base.Pos, ir.OCALL, dot, nil)
call = ir.NewCallExpr(base.Pos, ir.OCALL, dot, nil)
call.Args = ir.ParamNames(fn.Type())
call.IsDDD = fn.Type().IsVariadic()
fn.Body.Append(ir.NewTailCallStmt(base.Pos, call))
} else {
fn.SetWrapper(true) // ignore frame for panic+recover matching
var call *ir.CallExpr

if generic && dot.X != nthis {
// If there is embedding involved, then we should do the
Expand Down Expand Up @@ -1982,7 +1982,22 @@ func methodWrapper(rcvr *types.Type, method *types.Field, forItab bool) *obj.LSy
typecheck.Stmts(fn.Body)

if AfterGlobalEscapeAnalysis {
inline.InlineCalls(fn)
// Inlining the method may reveal closures, which require walking all function bodies
// to decide whether to capture free variables by value or by ref. So we only do inline
// if the method do not contain any closures, otherwise, the escape analysis may make
// dead variables resurrected, and causing liveness analysis confused, see issue #53702.
var canInline bool
switch x := call.X.(type) {
case *ir.Name:
canInline = len(x.Func.Closures) == 0
case *ir.SelectorExpr:
if x.Op() == ir.OMETHEXPR {
canInline = x.FuncName().Func != nil && len(x.FuncName().Func.Closures) == 0
}
}
if canInline {
inline.InlineCalls(fn)
}
escape.Batch([]*ir.Func{fn}, false)
}

Expand Down
39 changes: 39 additions & 0 deletions test/fixedbugs/issue53702.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// run

// Copyright 2022 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 main

type Elem struct{}

func (*Elem) Wait(callback func()) {}

type Base struct {
elem [8]*Elem
}

var g_val = 1

func (s *Base) Do() *int {
resp := &g_val
for _, e := range s.elem {
e.Wait(func() {
*resp = 0
})
}
return resp
}

type Sub struct {
*Base
}

func main() {
a := Sub{new(Base)}
resp := a.Do()
if resp != nil && *resp != 1 {
panic("FAIL")
}
}

0 comments on commit 11033ea

Please sign in to comment.