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 #5406.

R=golang-dev, iant
CC=golang-dev
https://golang.org/cl/13367052

1 files changed, 12 insertions, 1 deletions

diff --git a/src/pkg/runtime/stack.c b/src/pkg/runtime/stack.c
index 6b34f091e1..011c616bac 100644
--- a/src/pkg/runtime/stack.c
+++ b/src/pkg/runtime/stack.c
@@ -174,6 +174,7 @@ runtime·oldstack(void)
 	gp->stackbase = top->stackbase;
 	gp->stackguard = top->stackguard;
 	gp->stackguard0 = gp->stackguard;
+	gp->panicwrap = top->panicwrap;
 
 	if(top->free != 0) {
 		gp->stacksize -= top->free;
@@ -195,7 +196,7 @@ void
 runtime·newstack(void)
 {
 	int32 framesize, argsize, oldstatus;
-	Stktop *top;
+	Stktop *top, *oldtop;
 	byte *stk;
 	uintptr sp;
 	uintptr *src, *dst, *dstend;
@@ -316,6 +317,16 @@ runtime·newstack(void)
 	// copy flag from panic
 	top->panic = gp->ispanic;
 	gp->ispanic = false;
+	
+	// if this isn't a panic, maybe we're splitting the stack for a panic.
+	// if we're splitting in the top frame, propagate the panic flag
+	// forward so that recover will know we're in a panic.
+	oldtop = (Stktop*)top->stackbase;
+	if(oldtop != nil && oldtop->panic && top->argp == (byte*)oldtop - oldtop->argsize - gp->panicwrap)
+		top->panic = true;
+
+	top->panicwrap = gp->panicwrap;
+	gp->panicwrap = 0;
 
 	gp->stackbase = (uintptr)top;
 	gp->stackguard = (uintptr)stk + StackGuard;