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 f5b8d3e3cc894a0e7a341e4be715f0e917b699e7..be0f81879e47e580aa76ee772ac7b33e5f65ce4b 100644 (file)
--- 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 a8b98b73db97f3ea4b3073835d1cb51ed5fd0025..4d74b570b3a9f18503db25be52060c736fc7f9fe 100644 (file)
--- 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 c4beab1b5d88c2bb5b73b7b7ae47dc3fe659c174..e48b58a70e820cdc11bbec600bc5d8abfc797201 100644 (file)
--- 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 78ec5482dbd5d972bb537a12258877aa6b8341a0..74725ba4ca6c7898e1da503586166875e0c658e6 100644 (file)
--- 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 7f89b78fde77bf01482559b91a84327296944761..78f8bd5aad26e87a7cbc6617fce5e1902a50bb28 100644 (file)
--- 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.