proc/native/linux: better handling of process death due to signals (#2477)

Handle the signaled status for the thread leader like we handle the
exited status, by returning ErrProcessExited and recording the killer
signal  in it.
Prior to this commit we would find out about the death of the thread
later in the loop, the condition would still be reported as
ErrProcessExited, but without recording the signal number anywhere.

Also fixes a bug in TestAttachStopOnEntry where the test would
inadvertently cause a SIGPIPE to be sent to the target process, making
it terminate early.

pkg/proc/native/proc_linux.go

@@ -342,6 +342,16 @@ func (dbp *nativeProcess) trapWaitInternal(pid int, options trapWaitOptions) (*n
 			delete(dbp.threads, wpid)
 			continue
 		}
+		if status.Signaled() {
+			// Signaled means the thread was terminated due to a signal.
+			if wpid == dbp.pid {
+				dbp.postExit()
+				return nil, proc.ErrProcessExited{Pid: wpid, Status: -int(status.Signal())}
+			}
+			// does this ever happen?
+			delete(dbp.threads, wpid)
+			continue
+		}
 		if status.StopSignal() == sys.SIGTRAP && status.TrapCause() == sys.PTRACE_EVENT_CLONE {
 			// A traced thread has cloned a new thread, grab the pid and
 			// add it to our list of traced threads.

pkg/proc/proc_unix_test.go

@@ -4,11 +4,17 @@ package proc_test
 
 import (
 	"fmt"
+	"os"
+	"os/exec"
+	"runtime"
 	"syscall"
 	"testing"
 	"time"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/go-delve/delve/pkg/proc"
+	"github.com/go-delve/delve/pkg/proc/native"
 	protest "github.com/go-delve/delve/pkg/proc/test"
 )
 
@@ -66,3 +72,31 @@ func TestIssue419(t *testing.T) {
 		}
 	}
 }
+
+func TestSignalDeath(t *testing.T) {
+	if testBackend != "native" || runtime.GOOS != "linux" {
+		t.Skip("skipped on non-linux non-native backends")
+	}
+	var buildFlags protest.BuildFlags
+	if buildMode == "pie" {
+		buildFlags |= protest.BuildModePIE
+	}
+	fixture := protest.BuildFixture("loopprog", buildFlags)
+	cmd := exec.Command(fixture.Path)
+	stdout, err := cmd.StdoutPipe()
+	assertNoError(err, t, "StdoutPipe")
+	cmd.Stderr = os.Stderr
+	assertNoError(cmd.Start(), t, "starting fixture")
+	p, err := native.Attach(cmd.Process.Pid, []string{})
+	assertNoError(err, t, "Attach")
+	stdout.Close() // target will receive SIGPIPE later on
+	err = p.Continue()
+	t.Logf("error is %v", err)
+	exitErr, isexited := err.(proc.ErrProcessExited)
+	if !isexited {
+		t.Fatal("did not exit")
+	}
+	if exitErr.Status != -int(unix.SIGPIPE) {
+		t.Fatalf("expected SIGPIPE got %d\n", exitErr.Status)
+	}
+}

service/dap/server_test.go

@@ -368,39 +368,21 @@ func TestAttachStopOnEntry(t *testing.T) {
 		// 13 >> disconnect, << disconnect
 		client.DisconnectRequestWithKillOption(true)
 
-		// Both of these scenarios are somehow possible.
-		// Even though the program has an infininte loop,
-		// it apears that a halt can cause it to terminate.
-		// Since we are in async mode while running, we might receive messages in either order.
-		msg := client.ExpectMessage(t)
-		switch m := msg.(type) {
-		case *dap.StoppedEvent:
-			if m.Seq != 0 || m.Body.Reason != "pause" { // continue is interrupted
-				t.Errorf("\ngot %#v\nwant Seq=0 Reason='pause'", m)
-			}
-			oed := client.ExpectOutputEventDetachingKill(t)
-			if oed.Seq != 0 || oed.Body.Category != "console" {
-				t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed)
-			}
-		case *dap.TerminatedEvent:
-			if m.Seq != 0 {
-				t.Errorf("\ngot %#v\nwant Seq=0'", m)
-			}
-			oep := client.ExpectOutputEventProcessExited(t, 0)
-			if oep.Seq != 0 || oep.Body.Category != "console" {
-				t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oep)
-			}
-			oed := client.ExpectOutputEventDetaching(t)
-			if oed.Seq != 0 || oed.Body.Category != "console" {
-				t.Errorf("\ngot %#v\nwant Seq=0 Category='console'", oed)
-			}
-		default:
-			t.Fatalf("got %#v, want StoppedEvent or TerminatedEvent", m)
+		msg := expectMessageFilterStopped(t, client)
+		if _, ok := msg.(*dap.OutputEvent); !ok {
+			// want detach kill output message
+			t.Errorf("got %#v, want *dap.OutputEvent", msg)
 		}
-		dResp := client.ExpectDisconnectResponse(t)
-		if dResp.Seq != 0 || dResp.RequestSeq != 13 {
-			t.Errorf("\ngot %#v\nwant Seq=0, RequestSeq=13", dResp)
+		msg = expectMessageFilterStopped(t, client)
+		if _, ok := msg.(*dap.DisconnectResponse); !ok {
+			t.Errorf("got %#v, want *dap.DisconnectResponse", msg)
 		}
+		// If this call to KeepAlive isn't here there's a chance that stdout will
+		// be garbage collected (since it is no longer alive long before this
+		// point), when that happens, on unix-like OSes, the read end of the pipe
+		// will be closed by the finalizer and the target process will die by
+		// SIGPIPE, which the rest of this test does not expect.
+		runtime.KeepAlive(stdout)
 	})
 }
 
@@ -741,6 +723,14 @@ func expectVarRegex(t *testing.T, got *dap.VariablesResponse, i int, name, evalN
 	return expectVar(t, got, i, name, evalName, value, typ, false, hasRef)
 }
 
+func expectMessageFilterStopped(t *testing.T, client *daptest.Client) dap.Message {
+	msg := client.ExpectMessage(t)
+	if _, isStopped := msg.(*dap.StoppedEvent); isStopped {
+		msg = client.ExpectMessage(t)
+	}
+	return msg
+}
+
 // validateEvaluateName issues an evaluate request with evaluateName of a variable and
 // confirms that it succeeds and returns the same variable record as the original.
 func validateEvaluateName(t *testing.T, client *daptest.Client, got *dap.VariablesResponse, i int) {