runtime, cmd/gc, cmd/ld: ignore method wrappers in recover

Bug #1:

Issue 5406 identified an interesting case:
        defer iface.M()
may end up calling a wrapper that copies an indirect receiver
from the iface value and then calls the real M method. That's
two calls down, not just one, and so recover() == nil always
in the real M method, even during a panic.

[For the purposes of this entire discussion, a wrapper's
implementation is a function containing an ordinary call, not
the optimized tail call form that is somtimes possible. The
tail call does not create a second frame, so it is already
handled correctly.]

Fix this bug by introducing g->panicwrap, which counts the
number of bytes on current stack segment that are due to
wrapper calls that should not count against the recover
check. All wrapper functions must now adjust g->panicwrap up
on entry and back down on exit. This adds slightly to their
expense; on the x86 it is a single instruction at entry and
exit; on the ARM it is three. However, the alternative is to
make a call to recover depend on being able to walk the stack,
which I very much want to avoid. We have enough problems
walking the stack for garbage collection and profiling.
Also, if performance is critical in a specific case, it is already
faster to use a pointer receiver and avoid this kind of wrapper
entirely.

Bug #2:

The old code, which did not consider the possibility of two
calls, already contained a check to see if the call had split
its stack and so the panic-created segment was one behind the
current segment. In the wrapper case, both of the two calls
might split their stacks, so the panic-created segment can be
two behind the current segment.

Fix this by propagating the Stktop.panic flag forward during
stack splits instead of looking backward during recover.

Fixes issue 5406.

R=golang-dev, iant
CC=golang-dev
https://codereview.appspot.com/13367052

4 files changed, 14 insertions, 6 deletions

diff --git a/src/pkg/reflect/asm_386.s b/src/pkg/reflect/asm_386.s
index ef814966e..75413c752 100644
--- a/src/pkg/reflect/asm_386.s
+++ b/src/pkg/reflect/asm_386.s
@@ -8,7 +8,7 @@
 // See the comment on the declaration of makeFuncStub in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·makeFuncStub(SB),NOSPLIT,$8
+TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
 	MOVL	DX, 0(SP)
 	LEAL	argframe+0(FP), CX
 	MOVL	CX, 4(SP)
@@ -19,7 +19,7 @@ TEXT ·makeFuncStub(SB),NOSPLIT,$8
 // See the comment on the declaration of methodValueCall in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·methodValueCall(SB),NOSPLIT,$8
+TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
 	MOVL	DX, 0(SP)
 	LEAL	argframe+0(FP), CX
 	MOVL	CX, 4(SP)
diff --git a/src/pkg/reflect/asm_amd64.s b/src/pkg/reflect/asm_amd64.s
index 1aa10440c..712959843 100644
--- a/src/pkg/reflect/asm_amd64.s
+++ b/src/pkg/reflect/asm_amd64.s
@@ -8,7 +8,7 @@
 // See the comment on the declaration of makeFuncStub in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·makeFuncStub(SB),NOSPLIT,$16
+TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$16
 	MOVQ	DX, 0(SP)
 	LEAQ	argframe+0(FP), CX
 	MOVQ	CX, 8(SP)
@@ -19,7 +19,7 @@ TEXT ·makeFuncStub(SB),NOSPLIT,$16
 // See the comment on the declaration of methodValueCall in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·methodValueCall(SB),NOSPLIT,$16
+TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$16
 	MOVQ	DX, 0(SP)
 	LEAQ	argframe+0(FP), CX
 	MOVQ	CX, 8(SP)
diff --git a/src/pkg/reflect/asm_arm.s b/src/pkg/reflect/asm_arm.s
index 5e456ea2c..68ded4ac6 100644
--- a/src/pkg/reflect/asm_arm.s
+++ b/src/pkg/reflect/asm_arm.s
@@ -8,7 +8,7 @@
 // See the comment on the declaration of makeFuncStub in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·makeFuncStub(SB),NOSPLIT,$8
+TEXT ·makeFuncStub(SB),(NOSPLIT|WRAPPER),$8
 	MOVW	R7, 4(R13)
 	MOVW	$argframe+0(FP), R1
 	MOVW	R1, 8(R13)
@@ -19,7 +19,7 @@ TEXT ·makeFuncStub(SB),NOSPLIT,$8
 // See the comment on the declaration of methodValueCall in makefunc.go
 // for more details.
 // No argsize here, gc generates argsize info at call site.
-TEXT ·methodValueCall(SB),NOSPLIT,$8
+TEXT ·methodValueCall(SB),(NOSPLIT|WRAPPER),$8
 	MOVW	R7, 4(R13)
 	MOVW	$argframe+0(FP), R1
 	MOVW	R1, 8(R13)
diff --git a/src/pkg/reflect/value.go b/src/pkg/reflect/value.go
index dbecc59da..20fc459e5 100644
--- a/src/pkg/reflect/value.go
+++ b/src/pkg/reflect/value.go
@@ -497,6 +497,10 @@ func (v Value) call(op string, in []Value) []Value {
 // frame into a call using Values.
 // It is in this file so that it can be next to the call method above.
 // The remainder of the MakeFunc implementation is in makefunc.go.
+//
+// NOTE: This function must be marked as a "wrapper" in the generated code,
+// so that the linker can make it work correctly for panic and recover.
+// The gc compilers know to do that for the name "reflect.callReflect".
 func callReflect(ctxt *makeFuncImpl, frame unsafe.Pointer) {
 	ftyp := ctxt.typ
 	f := ctxt.fn
@@ -650,6 +654,10 @@ func frameSize(t *rtype, rcvr bool) (total, in, outOffset, out uintptr) {
 // to deal with individual Values for each argument.
 // It is in this file so that it can be next to the two similar functions above.
 // The remainder of the makeMethodValue implementation is in makefunc.go.
+//
+// NOTE: This function must be marked as a "wrapper" in the generated code,
+// so that the linker can make it work correctly for panic and recover.
+// The gc compilers know to do that for the name "reflect.callMethod".
 func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
 	t, fn, rcvr := methodReceiver("call", ctxt.rcvr, ctxt.method)
 	total, in, outOffset, out := frameSize(t, true)