diff options
author | Russ Cox <rsc@golang.org> | 2013-09-12 14:00:16 -0400 |
---|---|---|
committer | Russ Cox <rsc@golang.org> | 2013-09-12 14:00:16 -0400 |
commit | 8e733af658ac83ee339cebdea1d51cc82dd487c2 (patch) | |
tree | 405dce320f88d1a44d1eb8a72a018dc00c435f9a /src/cmd/8l | |
parent | 4df23439a9d69efdf986d6712be1577491e54839 (diff) | |
download | go-8e733af658ac83ee339cebdea1d51cc82dd487c2.tar.gz |
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
Diffstat (limited to 'src/cmd/8l')
-rw-r--r-- | src/cmd/8l/pass.c | 32 |
1 files changed, 28 insertions, 4 deletions
diff --git a/src/cmd/8l/pass.c b/src/cmd/8l/pass.c index b558ffaa9..1eaf78fe0 100644 --- a/src/cmd/8l/pass.c +++ b/src/cmd/8l/pass.c @@ -444,11 +444,12 @@ dostkoff(void) q = P; - if(!(p->from.scale & NOSPLIT)) { + if(!(p->from.scale & NOSPLIT) || (p->from.scale & WRAPPER)) { p = appendp(p); p = load_g_cx(p); // load g into CX - p = stacksplit(p, autoffset, &q); // emit split check } + if(!(cursym->text->from.scale & NOSPLIT)) + p = stacksplit(p, autoffset, &q); // emit split check if(autoffset) { p = appendp(p); @@ -456,8 +457,6 @@ dostkoff(void) p->from.type = D_CONST; p->from.offset = autoffset; p->spadj = autoffset; - if(q != P) - q->pcond = p; } else { // zero-byte stack adjustment. // Insert a fake non-zero adjustment so that stkcheck can @@ -469,8 +468,20 @@ dostkoff(void) p->as = ANOP; p->spadj = PtrSize; } + if(q != P) + q->pcond = p; deltasp = autoffset; + if(cursym->text->from.scale & WRAPPER) { + // g->panicwrap += autoffset + PtrSize; + p = appendp(p); + p->as = AADDL; + p->from.type = D_CONST; + p->from.offset = autoffset + PtrSize; + p->to.type = D_INDIR+D_CX; + p->to.offset = 2*PtrSize; + } + if(debug['Z'] && autoffset && !(cursym->text->from.scale&NOSPLIT)) { // 8l -Z means zero the stack frame on entry. // This slows down function calls but can help avoid @@ -540,6 +551,19 @@ dostkoff(void) if(autoffset != deltasp) diag("unbalanced PUSH/POP"); + + if(cursym->text->from.scale & WRAPPER) { + p = load_g_cx(p); + p = appendp(p); + // g->panicwrap -= autoffset + PtrSize; + p->as = ASUBL; + p->from.type = D_CONST; + p->from.offset = autoffset + PtrSize; + p->to.type = D_INDIR+D_CX; + p->to.offset = 2*PtrSize; + p = appendp(p); + p->as = ARET; + } if(autoffset) { p->as = AADJSP; |