Skip to content

Commit

Permalink
cmd/compile: fix mishandling of unsafe-uintptr arguments with call me…
Browse files Browse the repository at this point in the history
…thod in go/defer

In CL 253457, we did the same fix for direct function calls. But for
method calls, the receiver argument also need to be passed through the
wrapper function, which we are not doing so the compiler crashes with
the code in #44415.

It will be nicer if we can rewrite OCALLMETHOD to normal OCALLFUNC, but
that will be for future CL. The passing receiver argument to wrapper
function is easier for backporting to go1.16 branch.

Fixes #44415

Change-Id: I03607a64429042c6066ce673931db9769deb3124
Reviewed-on: https://go-review.googlesource.com/c/go/+/296490
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
  • Loading branch information
cuonglm committed Feb 26, 2021
1 parent e25040d commit 23943a6
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/cmd/compile/internal/walk/stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,22 @@ func wrapCall(n *ir.CallExpr, init *ir.Nodes) ir.Node {
}
}

wrapArgs := n.Args
// If there's a receiver argument, it needs to be passed through the wrapper too.
if n.Op() == ir.OCALLMETH || n.Op() == ir.OCALLINTER {
recv := n.X.(*ir.SelectorExpr).X
wrapArgs = append([]ir.Node{recv}, wrapArgs...)
}

// origArgs keeps track of what argument is uintptr-unsafe/unsafe-uintptr conversion.
origArgs := make([]ir.Node, len(n.Args))
origArgs := make([]ir.Node, len(wrapArgs))
var funcArgs []*ir.Field
for i, arg := range n.Args {
for i, arg := range wrapArgs {
s := typecheck.LookupNum("a", i)
if !isBuiltinCall && arg.Op() == ir.OCONVNOP && arg.Type().IsUintptr() && arg.(*ir.ConvExpr).X.Type().IsUnsafePtr() {
origArgs[i] = arg
arg = arg.(*ir.ConvExpr).X
n.Args[i] = arg
wrapArgs[i] = arg
}
funcArgs = append(funcArgs, ir.NewField(base.Pos, s, nil, arg.Type()))
}
Expand All @@ -278,6 +285,12 @@ func wrapCall(n *ir.CallExpr, init *ir.Nodes) ir.Node {
}
args[i] = ir.NewConvExpr(base.Pos, origArg.Op(), origArg.Type(), args[i])
}
if n.Op() == ir.OCALLMETH || n.Op() == ir.OCALLINTER {
// Move wrapped receiver argument back to its appropriate place.
recv := typecheck.Expr(args[0])
n.X.(*ir.SelectorExpr).X = recv
args = args[1:]
}
call := ir.NewCallExpr(base.Pos, n.Op(), n.X, args)
if !isBuiltinCall {
call.SetOp(ir.OCALL)
Expand All @@ -291,6 +304,6 @@ func wrapCall(n *ir.CallExpr, init *ir.Nodes) ir.Node {
typecheck.Stmts(fn.Body)
typecheck.Target.Decls = append(typecheck.Target.Decls, fn)

call = ir.NewCallExpr(base.Pos, ir.OCALL, fn.Nname, n.Args)
call = ir.NewCallExpr(base.Pos, ir.OCALL, fn.Nname, wrapArgs)
return walkExpr(typecheck.Stmt(call), init)
}
31 changes: 31 additions & 0 deletions test/fixedbugs/issue24491a.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ func f() int {
return test("return", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
}

type S struct{}

//go:noinline
//go:uintptrescapes
func (S) test(s string, p, q uintptr, rest ...uintptr) int {
return test(s, p, q, rest...)
}

func main() {
test("normal", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
<-done
Expand All @@ -60,6 +68,29 @@ func main() {
}()
<-done

func() {
for {
defer test("defer in for loop", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
break
}
}()

<-done
func() {
s := &S{}
defer s.test("method call", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
}()
<-done

func() {
s := &S{}
for {
defer s.test("defer method loop", uintptr(setup()), uintptr(setup()), uintptr(setup()), uintptr(setup()))
break
}
}()
<-done

f()
<-done
}

0 comments on commit 23943a6

Please sign in to comment.