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/proc.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/pkg/runtime/proc.c') diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c index d37014f3a5..0edd7e0ac9 100644 --- a/src/pkg/runtime/proc.c +++ b/src/pkg/runtime/proc.c @@ -1773,6 +1773,7 @@ runtime·newproc1(FuncVal *fn, byte *argp, int32 narg, int32 nret, void *callerp newg->gopc = (uintptr)callerpc; newg->status = Grunnable; newg->goid = runtime·xadd64(&runtime·sched.goidgen, 1); + newg->panicwrap = 0; if(raceenabled) newg->racectx = runtime·racegostart((void*)callerpc); runqput(m->p, newg); -- cgit v1.3