Re: libgo patch committed: set gp->m in getTraceback

[Ian Lance Taylor]

On Mon, Jan 7, 2019 at 12:12 PM Ian Lance Taylor <i...@golang.org> wrote:
>
> This libgo patch by Cherry Zhang moves setting p->m from gtraceback to
> getTraceback.  Currently, when collecting a traceback for another
> goroutine, getTraceback calls gogo(gp) switching to gp, which will
> resume in mcall, which will call gtraceback, which will set up gp->m.
> There is a gap between setting the current running g to gp and setting
> gp->m. If a profiling signal arrives in between, sigtramp will see a
> non-nil gp with a nil m, and will seg fault.  This patch fixes this by
> setting up gp->m first.  This fixes https://golang.org/issue/29448.
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

And here is a follow up patch by Cherry Zhang to make the same fix in
doscanstackswitch.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE     (revision 267660)
+++ gcc/go/gofrontend/MERGE     (working copy)
@@ -1,4 +1,4 @@
-c8a9bccbc524381d150c84907a61ac257c1b07cc
+085ef4556ec810a5a9c422e7b86d98441dc92e86
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: libgo/runtime/proc.c
===================================================================
--- libgo/runtime/proc.c        (revision 267658)
+++ libgo/runtime/proc.c        (working copy)
@@ -482,9 +482,14 @@ void doscanstackswitch(G*, G*) __asm__(G
 void
 doscanstackswitch(G* me, G* gp)
 {
+       M* holdm;
+
        __go_assert(me->entry == nil);
        me->fromgogo = false;
 
+       holdm = gp->m;
+       gp->m = me->m;
+
 #ifdef USING_SPLIT_STACK
        __splitstack_getcontext((void*)(&me->stackcontext[0]));
 #endif
@@ -507,6 +512,8 @@ doscanstackswitch(G* me, G* gp)
 
        if (gp->scang != 0)
                runtime_gogo(gp);
+
+       gp->m = holdm;
 }
 
 // Do a stack scan, then switch back to the g that triggers this scan.
@@ -515,21 +522,15 @@ static void
 gscanstack(G *gp)
 {
        G *oldg, *oldcurg;
-       M* holdm;
 
        oldg = (G*)gp->scang;
        oldcurg = oldg->m->curg;
-       holdm = gp->m;
-       if(holdm != nil && holdm != g->m)
-               runtime_throw("gscanstack: m is not nil");
        oldg->m->curg = gp;
-       gp->m = oldg->m;
        gp->scang = 0;
 
        doscanstack(gp, (void*)gp->scangcw);
 
        gp->scangcw = 0;
-       gp->m = holdm;
        oldg->m->curg = oldcurg;
        runtime_gogo(oldg);
 }