cmd/compile: be sure to wrap defer/go calls with arguments

CL 298669 implemented wrapping for defer/go calls so the function
being called with defer or go statement has no arguments. This
simplifies the compiler and the runtime, especially with the
new ABI.

If the called function does not have any argument, we don't need
to wrap. But the code missed the cases of method receiver, as
well as some apparent argumentless builtin calls which may later
be rewritten to having arguments (e.g. recover). This CL makes
sure to wrap those cases. Also add a check to ensure that go and
defer calls are indeed argumentless.

Handle "defer recover()" specially, as recover() is lowered to
runtime.gorecover(FP) where FP is the frame's FP. FP needs to be
evaluated before wrapping.

Updates #40724.

Change-Id: I2758b6c69ab6aa02dd588441a457fe28ddd0d5a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/304771
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>

diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index a1f6d898149478b53b2e495c92a3dd0bdfeae356..92bddd59da0b5204f514775d2a81fba121b40034 100644 (file)
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -4608,6 +4608,10 @@ func (s *state) openDeferRecord(n *ir.CallExpr) {
        var args []*ssa.Value
        var argNodes []*ir.Name
 
+       if objabi.Experiment.RegabiDefer && (len(n.Args) != 0 || n.Op() == ir.OCALLINTER) {
+               s.Fatalf("defer call with arguments: %v", n)
+       }
+
        opendefer := &openDeferInfo{
                n: n,
        }
@@ -4856,6 +4860,10 @@ func (s *state) call(n *ir.CallExpr, k callKind, returnResultAddr bool) *ssa.Val
                }
        }
 
+       if objabi.Experiment.RegabiDefer && k != callNormal && (len(n.Args) != 0 || n.Op() == ir.OCALLINTER) {
+               s.Fatalf("go/defer call with arguments: %v", n)
+       }
+
        switch n.Op() {
        case ir.OCALLFUNC:
                if k == callNormal && fn.Op() == ir.ONAME && fn.(*ir.Name).Class == ir.PFUNC {

diff --git a/src/cmd/compile/internal/walk/builtin.go b/src/cmd/compile/internal/walk/builtin.go
index 97f9de9c1df332e3222f94a48e322bf460ad7f9a..b5b24b26ca6c496f16829daff9cf4ea81382b2db 100644 (file)
--- a/src/cmd/compile/internal/walk/builtin.go
+++ b/src/cmd/compile/internal/walk/builtin.go
@@ -653,6 +653,18 @@ func walkPrint(nn *ir.CallExpr, init *ir.Nodes) ir.Node {
        return walkStmt(typecheck.Stmt(r))
 }
 
+// walkRecover walks an ORECOVER node.
+func walkRecover(nn *ir.CallExpr, init *ir.Nodes) ir.Node {
+       // Call gorecover with the FP of this frame.
+       // FP is equal to caller's SP plus FixedFrameSize().
+       var fp ir.Node = mkcall("getcallersp", types.Types[types.TUINTPTR], init)
+       if off := base.Ctxt.FixedFrameSize(); off != 0 {
+               fp = ir.NewBinaryExpr(fp.Pos(), ir.OADD, fp, ir.NewInt(off))
+       }
+       fp = ir.NewConvExpr(fp.Pos(), ir.OCONVNOP, types.NewPtr(types.Types[types.TINT32]), fp)
+       return mkcall("gorecover", nn.Type(), init, fp)
+}
+
 func badtype(op ir.Op, tl, tr *types.Type) {
        var s string
        if tl != nil {

diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go
index 4cc5b65d3e3990226a021214064842095214b266..0bf531e29d57728d4fb49bfc11bec21bf332c786 100644 (file)
--- a/src/cmd/compile/internal/walk/expr.go
+++ b/src/cmd/compile/internal/walk/expr.go
@@ -157,15 +157,7 @@ func walkExpr1(n ir.Node, init *ir.Nodes) ir.Node {
                return mkcall("gopanic", nil, init, n.X)
 
        case ir.ORECOVER:
-               n := n.(*ir.CallExpr)
-               // Call gorecover with the FP of this frame.
-               // FP is equal to caller's SP plus FixedFrameSize().
-               var fp ir.Node = mkcall("getcallersp", types.Types[types.TUINTPTR], init)
-               if off := base.Ctxt.FixedFrameSize(); off != 0 {
-                       fp = ir.NewBinaryExpr(fp.Pos(), ir.OADD, fp, ir.NewInt(off))
-               }
-               fp = ir.NewConvExpr(fp.Pos(), ir.OCONVNOP, types.NewPtr(types.Types[types.TINT32]), fp)
-               return mkcall("gorecover", n.Type(), init, fp)
+               return walkRecover(n.(*ir.CallExpr), init)
 
        case ir.OCFUNC:
                return n

diff --git a/src/cmd/compile/internal/walk/order.go b/src/cmd/compile/internal/walk/order.go
index 9e6c58054d3ad597b7b918413dabc9d0d0b71c7b..6e3acc624c45ca397628b08972a74905d495ccb2 100644 (file)
--- a/src/cmd/compile/internal/walk/order.go
+++ b/src/cmd/compile/internal/walk/order.go
@@ -732,6 +732,13 @@ func (o *orderState) stmt(n ir.Node) {
                t := o.markTemp()
                o.init(n.Call)
                o.call(n.Call)
+               if n.Call.Op() == ir.ORECOVER {
+                       // Special handling of "defer recover()". We need to evaluate the FP
+                       // argument before wrapping.
+                       var init ir.Nodes
+                       n.Call = walkRecover(n.Call.(*ir.CallExpr), &init)
+                       o.stmtList(init)
+               }
                if objabi.Experiment.RegabiDefer {
                        o.wrapGoDefer(n)
                }
@@ -1481,7 +1488,7 @@ func (o *orderState) wrapGoDefer(n *ir.GoDeferStmt) {
                callArgs = []ir.Node{x.X}
                mkNewCall = func(pos src.XPos, op ir.Op, fun ir.Node, args []ir.Node) ir.Node {
                        if len(args) != 1 {
-                               panic("internal error, expecting single arg to close")
+                               panic("internal error, expecting single arg")
                        }
                        return ir.Node(ir.NewUnaryExpr(pos, op, args[0]))
                }
@@ -1497,11 +1504,17 @@ func (o *orderState) wrapGoDefer(n *ir.GoDeferStmt) {
                panic("unhandled op")
        }
 
-       // No need to wrap if called func has no args. However in the case
-       // of "defer func() { ... }()" we need to protect against the
-       // possibility of directClosureCall rewriting things so that the
-       // call does have arguments.
-       if len(callArgs) == 0 {
+       // No need to wrap if called func has no args and no receiver.
+       // However in the case of "defer func() { ... }()" we need to
+       // protect against the possibility of directClosureCall rewriting
+       // things so that the call does have arguments.
+       //
+       // Do wrap method calls (OCALLMETH, OCALLINTER), because it has
+       // a receiver.
+       //
+       // Also do wrap builtin functions, because they may be expanded to
+       // calls with arguments (e.g. ORECOVER).
+       if len(callArgs) == 0 && call.Op() == ir.OCALLFUNC {
                if c, ok := call.(*ir.CallExpr); ok && callX != nil && callX.Op() == ir.OCLOSURE {
                        cloFunc := callX.(*ir.ClosureExpr).Func
                        cloFunc.SetClosureCalled(false)