From 7276c02b4193edb19bc0d2d36a786238564db03f Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 12 Sep 2013 14:00:16 -0400 Subject: 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 --- src/pkg/runtime/stack.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'src/pkg/runtime/stack.c') 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; -- cgit v1.3