From 575eb8f58b06657c9cadfe6f7ddfd9e530fe5dea Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 7 Jan 2019 20:12:39 +0000 Subject: [PATCH] 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 --- gcc/go/gofrontend/MERGE | 2 +- libgo/go/runtime/pprof/pprof_test.go | 35 ++++++++++++++++++++++++++++ libgo/runtime/proc.c | 13 ++++++----- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/gcc/go/gofrontend/MERGE b/gcc/go/gofrontend/MERGE index 34845839a95..d6fe5f6afb9 100644 --- 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 74a77770be1..fd05a049b5a 100644 --- 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 99b2cb166aa..4004df48612 100644 --- 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); } -- 2.30.2