From 4fc8528997600e71c37a64d3f44f14475aef9872 Mon Sep 17 00:00:00 2001
From: Alessandro Arzilli <alessandro.arzilli@gmail.com>
Date: Tue, 12 Nov 2019 15:58:54 +0100
Subject: [PATCH] proc/linux: do not route signals to threads while stopping
 (#1752)

* proc/linux: do not route signals to threads while stopping

While we are trying to stop the process we should not route signals
sent to threads because that will result in threads being resumed.
Also keep better track of which threads are stopped.

This fixes an incompatibility with Go 1.14, which sends a lot of
signals to its threads to implement non-cooperative preemption,
resulting in Delve hanging waiting for an already-stopped thread to
stop.

In principle however this bug has nothing to do with Go 1.14 and could
manifest in any instance of high signal pressure.

* Makefile: discard stderr of "go list"

In module mode "go" will print messages about downloading modules to
stderr, we shouldn't confuse them for the real command output.
---
 pkg/proc/native/proc_linux.go    | 24 +++++++++++++++++++-----
 pkg/proc/native/threads_linux.go |  9 ++++++---
 pkg/proc/proc_test.go            |  2 +-
 scripts/make.go                  |  2 +-
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/pkg/proc/native/proc_linux.go b/pkg/proc/native/proc_linux.go
index 50ff669d..9a0beca8 100644
--- a/pkg/proc/native/proc_linux.go
+++ b/pkg/proc/native/proc_linux.go
@@ -319,12 +319,23 @@ func (dbp *Process) trapWaitInternal(pid int, halt bool) (*Thread, error) {
 		}
 
 		// TODO(dp) alert user about unexpected signals here.
-		if err := th.resumeWithSig(int(status.StopSignal())); err != nil {
-			if err == sys.ESRCH {
-				dbp.postExit()
-				return nil, proc.ErrProcessExited{Pid: dbp.pid}
+		if halt && !th.os.running {
+			// We are trying to stop the process, queue this signal to be delivered
+			// to the thread when we resume.
+			// Do not do this for threads that were running because we sent them a
+			// STOP signal and we need to observe it so we don't mistakenly deliver
+			// it later.
+			th.os.delayedSignal = int(status.StopSignal())
+			th.os.running = false
+			return th, nil
+		} else {
+			if err := th.resumeWithSig(int(status.StopSignal())); err != nil {
+				if err == sys.ESRCH {
+					dbp.postExit()
+					return nil, proc.ErrProcessExited{Pid: dbp.pid}
+				}
+				return nil, err
 			}
-			return nil, err
 		}
 	}
 }
@@ -429,6 +440,9 @@ func (dbp *Process) stop(trapthread *Thread) (err error) {
 			if err := th.stop(); err != nil {
 				return dbp.exitGuard(err)
 			}
+		} else {
+			// Thread is already in a trace stop but we didn't get the notification yet.
+			th.os.running = false
 		}
 	}
 
diff --git a/pkg/proc/native/threads_linux.go b/pkg/proc/native/threads_linux.go
index 3cd91671..563e1c83 100644
--- a/pkg/proc/native/threads_linux.go
+++ b/pkg/proc/native/threads_linux.go
@@ -16,8 +16,9 @@ type WaitStatus sys.WaitStatus
 // OSSpecificDetails hold Linux specific
 // process details.
 type OSSpecificDetails struct {
-	registers sys.PtraceRegs
-	running   bool
+	delayedSignal int
+	registers     sys.PtraceRegs
+	running       bool
 }
 
 func (t *Thread) stop() (err error) {
@@ -37,7 +38,9 @@ func (t *Thread) Stopped() bool {
 }
 
 func (t *Thread) resume() error {
-	return t.resumeWithSig(0)
+	sig := t.os.delayedSignal
+	t.os.delayedSignal = 0
+	return t.resumeWithSig(sig)
 }
 
 func (t *Thread) resumeWithSig(sig int) (err error) {
diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go
index c37c2f0a..7e966c23 100644
--- a/pkg/proc/proc_test.go
+++ b/pkg/proc/proc_test.go
@@ -953,7 +953,7 @@ func TestStacktraceGoroutine(t *testing.T) {
 					if locations[i].Call.Fn != nil {
 						name = locations[i].Call.Fn.Name
 					}
-					t.Logf("\t%s:%d %s (%#x)\n", locations[i].Call.File, locations[i].Call.Line, name, locations[i].Current.PC)
+					t.Logf("\t%s:%d %s (%#x) %x %v\n", locations[i].Call.File, locations[i].Call.Line, name, locations[i].Current.PC, locations[i].FrameOffset(), locations[i].SystemStack)
 				}
 			}
 		}
diff --git a/scripts/make.go b/scripts/make.go
index 9b1e0b31..bdba288d 100644
--- a/scripts/make.go
+++ b/scripts/make.go
@@ -187,7 +187,7 @@ func quotemaybe(args []string) []string {
 func getoutput(cmd string, args ...interface{}) string {
 	x := exec.Command(cmd, strflatten(args)...)
 	x.Env = os.Environ()
-	out, err := x.CombinedOutput()
+	out, err := x.Output()
 	if err != nil {
 		log.Fatal(err)
 	}