From 835dfef939879b284d4c0f4e1726491f27e4f1ee Mon Sep 17 00:00:00 2001
From: Vladimir Stefanovic <vladimir.stefanovic@imgtec.com>
Date: Thu, 4 May 2017 16:45:29 +0200
Subject: [PATCH] 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>
---
 src/runtime/cpuprof.go          |  9 +++++++++
 src/runtime/pprof/pprof_test.go | 31 +++++++++++++++++++++++++++++++
 src/runtime/proc.go             | 31 +++++++++++++++++++++++++++----
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/src/runtime/cpuprof.go b/src/runtime/cpuprof.go
index c761e440b6..fb841a9f3d 100644
--- 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 22fea0a52f..992d2abb6a 100644
--- 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 a5ada4f6db..ed333bb92e 100644
--- 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--
-- 
2.50.0