runtime/pprof: prevent a deadlock that SIGPROF might create on mips{,le}

64bit atomics on mips/mipsle are implemented using spinlocks. If SIGPROF
is received while the program is in the critical section, it will try to
write the sample using the same spinlock, creating a deadloop.
Prevent it by creating a counter of SIGPROFs during atomic64 and
postpone writing the sample(s) until called from elsewhere, with
pc set to _LostSIGPROFDuringAtomic64.

Added a test case, per Cherry's suggestion. Works around #20146.

Change-Id: Icff504180bae4ee83d78b19c0d9d6a80097087f9
Reviewed-on: https://go-review.googlesource.com/42652
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/runtime/cpuprof.go b/src/runtime/cpuprof.go
index c761e440b66fc13acb749708f4fe62c75aaf7437..fb841a9f3ddcb12191a009c1cd3b703cc114cf38 100644 (file)
--- a/src/runtime/cpuprof.go
+++ b/src/runtime/cpuprof.go
@@ -163,6 +163,15 @@ func (p *cpuProfile) addExtra() {
        }
 }
 
+func (p *cpuProfile) addLostAtomic64(count uint64) {
+       hdr := [1]uint64{count}
+       lostStk := [2]uintptr{
+               funcPC(_LostSIGPROFDuringAtomic64) + sys.PCQuantum,
+               funcPC(_System) + sys.PCQuantum,
+       }
+       cpuprof.log.write(nil, 0, hdr[:], lostStk[:])
+}
+
 // CPUProfile panics.
 // It formerly provided raw access to chunks of
 // a pprof-format profile generated by the runtime.

diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go
index 22fea0a52f1bab5327bef14f60871857fdba8987..992d2abb6a335c3c5ee253bc37720baea424e67c 100644 (file)
--- a/src/runtime/pprof/pprof_test.go
+++ b/src/runtime/pprof/pprof_test.go
@@ -12,6 +12,7 @@ import (
        "fmt"
        "internal/testenv"
        "io"
+       "io/ioutil"
        "math/big"
        "os"
        "os/exec"
@@ -20,6 +21,7 @@ import (
        "runtime/pprof/internal/profile"
        "strings"
        "sync"
+       "sync/atomic"
        "testing"
        "time"
 )
@@ -713,3 +715,32 @@ func TestCPUProfileLabel(t *testing.T) {
                })
        })
 }
+
+// Check that there is no deadlock when the program receives SIGPROF while in
+// 64bit atomics' critical section. Used to happen on mips{,le}. See #20146.
+func TestAtomicLoadStore64(t *testing.T) {
+       f, err := ioutil.TempFile("", "profatomic")
+       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()
+
+       var flag uint64
+       done := make(chan bool, 1)
+
+       go func() {
+               for atomic.LoadUint64(&flag) == 0 {
+                       runtime.Gosched()
+               }
+               done <- true
+       }()
+       time.Sleep(50 * time.Millisecond)
+       atomic.StoreUint64(&flag, 1)
+       <-done
+}

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index a5ada4f6db4569be5dcd4a2103933e351b6d1887..ed333bb92eeeb915f4b376c56dd608ebb1cdd896 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -3232,10 +3232,14 @@ var prof struct {
        hz         int32
 }
 
-func _System()           { _System() }
-func _ExternalCode()     { _ExternalCode() }
-func _LostExternalCode() { _LostExternalCode() }
-func _GC()               { _GC() }
+func _System()                    { _System() }
+func _ExternalCode()              { _ExternalCode() }
+func _LostExternalCode()          { _LostExternalCode() }
+func _GC()                        { _GC() }
+func _LostSIGPROFDuringAtomic64() { _LostSIGPROFDuringAtomic64() }
+
+// Counts SIGPROFs received while in atomic64 critical section, on mips{,le}
+var lostAtomic64Count uint64
 
 // Called if we receive a SIGPROF signal.
 // Called by the signal handler, may run during STW.
@@ -3245,6 +3249,21 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
                return
        }
 
+       // On mips{,le}, 64bit atomics are emulated with spinlocks, in
+       // runtime/internal/atomic. If SIGPROF arrives while the program is inside
+       // the critical section, it creates a deadlock (when writing the sample).
+       // As a workaround, create a counter of SIGPROFs while in critical section
+       // to store the count, and pass it to sigprof.add() later when SIGPROF is
+       // received from somewhere else (with _LostSIGPROFDuringAtomic64 as pc).
+       if GOARCH == "mips" || GOARCH == "mipsle" {
+               if f := findfunc(pc); f.valid() {
+                       if hasprefix(funcname(f), "runtime/internal/atomic") {
+                               lostAtomic64Count++
+                               return
+                       }
+               }
+       }
+
        // Profiling runs concurrently with GC, so it must not allocate.
        // Set a trap in case the code does allocate.
        // Note that on windows, one thread takes profiles of all the
@@ -3371,6 +3390,10 @@ func sigprof(pc, sp, lr uintptr, gp *g, mp *m) {
        }
 
        if prof.hz != 0 {
+               if (GOARCH == "mips" || GOARCH == "mipsle") && lostAtomic64Count > 0 {
+                       cpuprof.addLostAtomic64(lostAtomic64Count)
+                       lostAtomic64Count = 0
+               }
                cpuprof.add(gp, stk[:n])
        }
        getg().m.mallocing--