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

7 files changed, 131 insertions, 9 deletions

diff --git a/src/cmd/5l/noop.c b/src/cmd/5l/noop.c
index 7b63aa715..0bd76040d 100644
--- a/src/cmd/5l/noop.c
+++ b/src/cmd/5l/noop.c
@@ -271,6 +271,35 @@ noops(void)
 				p->to.offset = -autosize;
 				p->to.reg = REGSP;
 				p->spadj = autosize;
+				
+				if(cursym->text->reg & WRAPPER) {
+					// g->panicwrap += autosize;
+					// MOVW panicwrap_offset(g), R3
+					// ADD $autosize, R3
+					// MOVW R3 panicwrap_offset(g)
+					p = appendp(p);
+					p->as = AMOVW;
+					p->from.type = D_OREG;
+					p->from.reg = REGG;
+					p->from.offset = 2*PtrSize;
+					p->to.type = D_REG;
+					p->to.reg = 3;
+				
+					p = appendp(p);
+					p->as = AADD;
+					p->from.type = D_CONST;
+					p->from.offset = autosize;
+					p->to.type = D_REG;
+					p->to.reg = 3;
+					
+					p = appendp(p);
+					p->as = AMOVW;
+					p->from.type = D_REG;
+					p->from.reg = 3;
+					p->to.type = D_OREG;
+					p->to.reg = REGG;
+					p->to.offset = 2*PtrSize;
+				}
 				break;
 	
 			case ARET:
@@ -290,6 +319,36 @@ noops(void)
 						break;
 					}
 				}
+
+				if(cursym->text->reg & WRAPPER) {
+					// g->panicwrap -= autosize;
+					// MOVW panicwrap_offset(g), R3
+					// SUB $autosize, R3
+					// MOVW R3 panicwrap_offset(g)
+					p->as = AMOVW;
+					p->from.type = D_OREG;
+					p->from.reg = REGG;
+					p->from.offset = 2*PtrSize;
+					p->to.type = D_REG;
+					p->to.reg = 3;
+					p = appendp(p);
+				
+					p->as = ASUB;
+					p->from.type = D_CONST;
+					p->from.offset = autosize;
+					p->to.type = D_REG;
+					p->to.reg = 3;
+					p = appendp(p);
+
+					p->as = AMOVW;
+					p->from.type = D_REG;
+					p->from.reg = 3;
+					p->to.type = D_OREG;
+					p->to.reg = REGG;
+					p->to.offset = 2*PtrSize;
+					p = appendp(p);
+				}
+
 				p->as = AMOVW;
 				p->scond |= C_PBIT;
 				p->from.type = D_OREG;
diff --git a/src/cmd/6l/pass.c b/src/cmd/6l/pass.c
index d24672432..1be3c18fe 100644
--- a/src/cmd/6l/pass.c
+++ b/src/cmd/6l/pass.c
@@ -511,11 +511,12 @@ dostkoff(void)
 			diag("nosplit func likely to overflow stack");
 
 		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);
@@ -523,8 +524,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
@@ -536,7 +535,19 @@ 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['K'] > 1 && autoffset) {
 			// 6l -KK means double-check for stack overflow
@@ -654,6 +665,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;
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;
diff --git a/src/cmd/gc/go.h b/src/cmd/gc/go.h
index 103aedb41..ba73508b8 100644
--- a/src/cmd/gc/go.h
+++ b/src/cmd/gc/go.h
@@ -272,6 +272,7 @@ struct	Node
 	uchar	implicit;
 	uchar	addrtaken;	// address taken, even if not moved to heap
 	uchar	dupok;	// duplicate definitions ok (for func)
+	uchar	wrapper;	// is method wrapper (for func)
 	schar	likely; // likeliness of if statement
 	uchar	hasbreak;	// has break statement
 	uchar	needzero; // if it contains pointers, needs to be zeroed on function entry
diff --git a/src/cmd/gc/pgen.c b/src/cmd/gc/pgen.c
index d649fc49d..0fbac84cf 100644
--- a/src/cmd/gc/pgen.c
+++ b/src/cmd/gc/pgen.c
@@ -95,7 +95,18 @@ compile(Node *fn)
 	nodconst(&nod1, types[TINT32], 0);
 	ptxt = gins(ATEXT, isblank(curfn->nname) ? N : curfn->nname, &nod1);
 	if(fn->dupok)
-		ptxt->TEXTFLAG = DUPOK;
+		ptxt->TEXTFLAG |= DUPOK;
+	if(fn->wrapper)
+		ptxt->TEXTFLAG |= WRAPPER;
+
+	// Clumsy but important.
+	// See test/recover.go for test cases and src/pkg/reflect/value.go
+	// for the actual functions being considered.
+	if(myimportpath != nil && strcmp(myimportpath, "reflect") == 0) {
+		if(strcmp(curfn->nname->sym->name, "callReflect") == 0 || strcmp(curfn->nname->sym->name, "callMethod") == 0)
+			ptxt->TEXTFLAG |= WRAPPER;
+	}	
+	
 	afunclit(&ptxt->from, curfn->nname);
 
 	ginit();
diff --git a/src/cmd/gc/subr.c b/src/cmd/gc/subr.c
index 54fddbb90..3b3b57631 100644
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -2588,6 +2588,7 @@ genwrapper(Type *rcvr, Type *method, Sym *newnam, int iface)
 		n->left = newname(methodsym(method->sym, methodrcvr, 0));
 		fn->nbody = list(fn->nbody, n);
 	} else {
+		fn->wrapper = 1; // ignore frame for panic+recover matching
 		call = nod(OCALL, dot, N);
 		call->list = args;
 		call->isddd = isddd;
diff --git a/src/cmd/ld/textflag.h b/src/cmd/ld/textflag.h
index 64ae647fb..1d62db736 100644
--- a/src/cmd/ld/textflag.h
+++ b/src/cmd/ld/textflag.h
@@ -17,3 +17,5 @@
 #define RODATA	8
 // This data contains no pointers.
 #define NOPTR	16
+// This is a wrapper function and should not count as disabling 'recover'.
+#define WRAPPER 32