From: Dmitriy Vyukov <dvyukov@google.com>
Date: Tue, 6 Aug 2013 09:38:44 +0000 (+0400)
Subject: runtime: use gcpc/gcsp during traceback of goroutines in syscalls
X-Git-Tag: go1.2rc2~769
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9c0500b466196388ab40e03c94759066bb1c7fe6;p=gostls13.git

runtime: use gcpc/gcsp during traceback of goroutines in syscalls
gcpc/gcsp are used by GC in similar situation.
gcpc/gcsp are also more stable than gp->sched,
because gp->sched is mutated by entersyscall/exitsyscall
in morestack and mcall. So it has higher chances of being inconsistent.
Also, rename gcpc/gcsp to syscallpc/syscallsp.

This is the same as reverted change 12250043
with save marked as textflag 7.
The problem was that if save calls morestack,
then subsequent lessstack spoils g->sched.pc/sp.
And that bad values were remembered in g->syscallpc/sp.
Entersyscallblock had the same problem,
but it was never triggered to date.

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/12478043
---

diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c
index f5b8d3e3cc..be0f81879e 100644
--- a/src/pkg/runtime/mgc0.c
+++ b/src/pkg/runtime/mgc0.c
@@ -1457,17 +1457,17 @@ addstackroots(G *gp)
 		runtime·throw("can't scan our own stack");
 	if((mp = gp->m) != nil && mp->helpgc)
 		runtime·throw("can't scan gchelper stack");
-	if(gp->gcstack != (uintptr)nil) {
+	if(gp->syscallstack != (uintptr)nil) {
 		// Scanning another goroutine that is about to enter or might
 		// have just exited a system call. It may be executing code such
 		// as schedlock and may have needed to start a new stack segment.
 		// Use the stack segment and stack pointer at the time of
 		// the system call instead, since that won't change underfoot.
-		sp = gp->gcsp;
-		pc = gp->gcpc;
+		sp = gp->syscallsp;
+		pc = gp->syscallpc;
 		lr = 0;
-		stk = (Stktop*)gp->gcstack;
-		guard = gp->gcguard;
+		stk = (Stktop*)gp->syscallstack;
+		guard = gp->syscallguard;
 	} else {
 		// Scanning another goroutine's stack.
 		// The goroutine is usually asleep (the world is stopped).
diff --git a/src/pkg/runtime/proc.c b/src/pkg/runtime/proc.c
index a8b98b73db..4d74b570b3 100644
--- a/src/pkg/runtime/proc.c
+++ b/src/pkg/runtime/proc.c
@@ -1355,11 +1355,10 @@ goexit0(G *gp)
 	schedule();
 }
 
+#pragma textflag 7
 static void
 save(void *pc, uintptr sp)
 {
-	g->gcpc = (uintptr)pc;
-	g->gcsp = sp;
 	g->sched.pc = (uintptr)pc;
 	g->sched.sp = sp;
 	g->sched.lr = 0;
@@ -1384,15 +1383,16 @@ void
 	// but can have inconsistent g->sched, do not let GC observe it.
 	m->locks++;
 
-	// Leave SP around for gc and traceback.
+	// Leave SP around for GC and traceback.
 	save(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
-
-	g->gcstack = g->stackbase;
-	g->gcguard = g->stackguard;
+	g->syscallsp = g->sched.sp;
+	g->syscallpc = g->sched.pc;
+	g->syscallstack = g->stackbase;
+	g->syscallguard = g->stackguard;
 	g->status = Gsyscall;
-	if(g->gcsp < g->gcguard-StackGuard || g->gcstack < g->gcsp) {
+	if(g->syscallsp < g->syscallguard-StackGuard || g->syscallstack < g->syscallsp) {
 		// runtime·printf("entersyscall inconsistent %p [%p,%p]\n",
-		//	g->gcsp, g->gcguard-StackGuard, g->gcstack);
+		//	g->syscallsp, g->syscallguard-StackGuard, g->syscallstack);
 		runtime·throw("entersyscall");
 	}
 
@@ -1436,16 +1436,16 @@ void
 
 	m->locks++;  // see comment in entersyscall
 
-	// Leave SP around for gc and traceback.
+	// Leave SP around for GC and traceback.
 	save(runtime·getcallerpc(&dummy), runtime·getcallersp(&dummy));
-	g->gcsp = g->sched.sp;
-	g->gcpc = g->sched.pc;
-	g->gcstack = g->stackbase;
-	g->gcguard = g->stackguard;
+	g->syscallsp = g->sched.sp;
+	g->syscallpc = g->sched.pc;
+	g->syscallstack = g->stackbase;
+	g->syscallguard = g->stackguard;
 	g->status = Gsyscall;
-	if(g->gcsp < g->gcguard-StackGuard || g->gcstack < g->gcsp) {
-		// runtime·printf("entersyscallblock inconsistent %p [%p,%p]\n",
-		//	g->gcsp, g->gcguard-StackGuard, g->gcstack);
+	if(g->syscallsp < g->syscallguard-StackGuard || g->syscallstack < g->syscallsp) {
+		// runtime·printf("entersyscall inconsistent %p [%p,%p]\n",
+		//	g->syscallsp, g->syscallguard-StackGuard, g->syscallstack);
 		runtime·throw("entersyscallblock");
 	}
 
@@ -1480,8 +1480,8 @@ runtime·exitsyscall(void)
 		g->status = Grunning;
 		// Garbage collector isn't running (since we are),
 		// so okay to clear gcstack and gcsp.
-		g->gcstack = (uintptr)nil;
-		g->gcsp = (uintptr)nil;
+		g->syscallstack = (uintptr)nil;
+		g->syscallsp = (uintptr)nil;
 		m->locks--;
 		if(g->preempt) {
 			// restore the preemption request in case we've cleared it in newstack
@@ -1504,8 +1504,8 @@ runtime·exitsyscall(void)
 	// Must wait until now because until gosched returns
 	// we don't know for sure that the garbage collector
 	// is not running.
-	g->gcstack = (uintptr)nil;
-	g->gcsp = (uintptr)nil;
+	g->syscallstack = (uintptr)nil;
+	g->syscallsp = (uintptr)nil;
 }
 
 #pragma textflag 7
diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h
index c4beab1b5d..e48b58a70e 100644
--- a/src/pkg/runtime/runtime.h
+++ b/src/pkg/runtime/runtime.h
@@ -253,10 +253,10 @@ struct	G
 	Defer*	defer;
 	Panic*	panic;
 	Gobuf	sched;
-	uintptr	gcstack;		// if status==Gsyscall, gcstack = stackbase to use during gc
-	uintptr	gcsp;		// if status==Gsyscall, gcsp = sched.sp to use during gc
-	uintptr	gcpc;		// if status==Gsyscall, gcpc = sched.pc to use during gc
-	uintptr	gcguard;		// if status==Gsyscall, gcguard = stackguard to use during gc
+	uintptr	syscallstack;		// if status==Gsyscall, syscallstack = stackbase to use during gc
+	uintptr	syscallsp;		// if status==Gsyscall, syscallsp = sched.sp to use during gc
+	uintptr	syscallpc;		// if status==Gsyscall, syscallpc = sched.pc to use during gc
+	uintptr	syscallguard;		// if status==Gsyscall, syscallguard = stackguard to use during gc
 	uintptr	stackguard;	// same as stackguard0, but not set to StackPreempt
 	uintptr	stack0;
 	G*	alllink;	// on allg
diff --git a/src/pkg/runtime/traceback_arm.c b/src/pkg/runtime/traceback_arm.c
index 78ec5482db..74725ba4ca 100644
--- a/src/pkg/runtime/traceback_arm.c
+++ b/src/pkg/runtime/traceback_arm.c
@@ -221,8 +221,8 @@ runtime·traceback(uintptr pc, uintptr sp, uintptr lr, G *gp)
 {
 	if(gp->status == Gsyscall) {
 		// Override signal registers if blocked in system call.
-		pc = gp->sched.pc;
-		sp = gp->sched.sp;
+		pc = gp->syscallpc;
+		sp = gp->syscallsp;
 		lr = 0;
 	}
 
diff --git a/src/pkg/runtime/traceback_x86.c b/src/pkg/runtime/traceback_x86.c
index 7f89b78fde..78f8bd5aad 100644
--- a/src/pkg/runtime/traceback_x86.c
+++ b/src/pkg/runtime/traceback_x86.c
@@ -229,8 +229,8 @@ runtime·traceback(uintptr pc, uintptr sp, uintptr lr, G *gp)
 
 	if(gp->status == Gsyscall) {
 		// Override signal registers if blocked in system call.
-		pc = gp->sched.pc;
-		sp = gp->sched.sp;
+		pc = gp->syscallpc;
+		sp = gp->syscallsp;
 	}
 	
 	// Print traceback. By default, omits runtime frames.