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/runtime.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/pkg/runtime/runtime.h') diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h index 151804f2a6..9974fa3269 100644 --- a/src/pkg/runtime/runtime.h +++ b/src/pkg/runtime/runtime.h @@ -250,6 +250,8 @@ struct G // stackguard0 can be set to StackPreempt as opposed to stackguard uintptr stackguard0; // cannot move - also known to linker, libmach, runtime/cgo uintptr stackbase; // cannot move - also known to libmach, runtime/cgo + uint32 panicwrap; // cannot move - also known to linker + uint32 selgen; // valid sudog pointer Defer* defer; Panic* panic; Gobuf sched; @@ -264,7 +266,6 @@ struct G void* param; // passed parameter on wakeup int16 status; int64 goid; - uint32 selgen; // valid sudog pointer int8* waitreason; // if status==Gwaiting G* schedlink; bool ispanic; @@ -403,6 +404,7 @@ struct Stktop uintptr stackbase; Gobuf gobuf; uint32 argsize; + uint32 panicwrap; uint8* argp; // pointer to arguments in old frame uintptr free; // if free>0, call stackfree using free as size -- cgit v1.3