runtime: in getTraceback, set gp->m before gogo

    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. Fix this by
    setting up gp->m first.

    Fixes golang/go#29448.

    Reviewed-on: https://go-review.googlesource.com/c/156038

From-SVN: r267658

diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE
index 34845839a95b50a6837e6c05e8b2a92324dc6074..d6fe5f6afb969fb478311d436d849f37e98b0d10 100644 (file)
--- a/gcc/go/gofrontend/MERGE
+++ b/gcc/go/gofrontend/MERGE
@@ -1,4 +1,4 @@
-2ce291eaee427799bfcde256929dab89e0ab61eb
+c257303eaef143663216e483857d5b259e05753f
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.

diff --git a/libgo/go/runtime/pprof/pprof_test.go b/libgo/go/runtime/pprof/pprof_test.go
index 74a77770be184978a6d71e29fd6cedb0596df651..fd05a049b5a11bf020e03a28cb43ca4e854df4de 100644 (file)
--- a/libgo/go/runtime/pprof/pprof_test.go
+++ b/libgo/go/runtime/pprof/pprof_test.go
@@ -946,3 +946,38 @@ func TestAtomicLoadStore64(t *testing.T) {
        atomic.StoreUint64(&flag, 1)
        <-done
 }
+
+func TestTracebackAll(t *testing.T) {
+       // With gccgo, if a profiling signal arrives at the wrong time
+       // during traceback, it may crash or hang. See issue #29448.
+       f, err := ioutil.TempFile("", "proftraceback")
+       if err != nil {
+               t.Fatalf("TempFile: %v", err)
+       }
+       defer os.Remove(f.Name())
+       defer f.Close()
+
+       if err := StartCPUProfile(f); err != nil {
+               t.Fatal(err)
+       }
+       defer StopCPUProfile()
+
+       ch := make(chan int)
+       defer close(ch)
+
+       count := 10
+       for i := 0; i < count; i++ {
+               go func() {
+                       <-ch // block
+               }()
+       }
+
+       N := 10000
+       if testing.Short() {
+               N = 500
+       }
+       buf := make([]byte, 10*1024)
+       for i := 0; i < N; i++ {
+               runtime.Stack(buf, true)
+       }
+}

diff --git a/libgo/runtime/proc.c b/libgo/runtime/proc.c
index 99b2cb166aa960f3e892c26b2b1c9699bb61e18f..4004df48612d6e1f5f4e12fac161b7855e54326b 100644 (file)
--- a/libgo/runtime/proc.c
+++ b/libgo/runtime/proc.c
@@ -442,6 +442,11 @@ void getTraceback(G*, G*) __asm__(GOSYM_PREFIX "runtime.getTraceback");
 // goroutine stored in the traceback field, which is me.
 void getTraceback(G* me, G* gp)
 {
+       M* holdm;
+
+       holdm = gp->m;
+       gp->m = me->m;
+
 #ifdef USING_SPLIT_STACK
        __splitstack_getcontext((void*)(&me->stackcontext[0]));
 #endif
@@ -450,6 +455,8 @@ void getTraceback(G* me, G* gp)
        if (gp->traceback != 0) {
                runtime_gogo(gp);
        }
+
+       gp->m = holdm;
 }
 
 // Do a stack trace of gp, and then restore the context to
@@ -459,17 +466,11 @@ void
 gtraceback(G* gp)
 {
        Traceback* traceback;
-       M* holdm;
 
        traceback = (Traceback*)gp->traceback;
        gp->traceback = 0;
-       holdm = gp->m;
-       if(holdm != nil && holdm != g->m)
-               runtime_throw("gtraceback: m is not nil");
-       gp->m = traceback->gp->m;
        traceback->c = runtime_callers(1, traceback->locbuf,
                sizeof traceback->locbuf / sizeof traceback->locbuf[0], false);
-       gp->m = holdm;
        runtime_gogo(traceback->gp);
 }