debugger,terminal: Allow setting and clearing breakpoints after process exit (#3246)

This patch allows users to set a breakpoint even when the process has
exited. It will be left in a pending state until the process is
restarted.

Fixes #3242

pkg/terminal/command.go

@@ -1753,14 +1753,18 @@ func setBreakpoint(t *Term, ctx callContext, tracepoint bool, argstr string) ([]
 	}
 	if findLocErr != nil && shouldAskToSuspendBreakpoint(t) {
 		fmt.Fprintf(os.Stderr, "Command failed: %s\n", findLocErr.Error())
-		findLocErr = nil
+		question := "Set a suspended breakpoint (Delve will try to set this breakpoint when a plugin is loaded) [Y/n]?"
-		answer, err := yesno(t.line, "Set a suspended breakpoint (Delve will try to set this breakpoint when a plugin is loaded) [Y/n]?", "yes")
+		if isErrProcessExited(findLocErr) {
+			question = "Set a suspended breakpoint (Delve will try to set this breakpoint when the process is restarted) [Y/n]?"
+		}
+		answer, err := yesno(t.line, question, "yes")
 		if err != nil {
 			return nil, err
 		}
 		if !answer {
 			return nil, nil
 		}
+		findLocErr = nil
 		bp, err := t.client.CreateBreakpointWithExpr(requestedBp, spec, t.substitutePathRules(), true)
 		if err != nil {
 			return nil, err
@@ -3220,5 +3224,6 @@ func (t *Term) formatBreakpointLocation(bp *api.Breakpoint) string {
 
 func shouldAskToSuspendBreakpoint(t *Term) bool {
 	fns, _ := t.client.ListFunctions(`^plugin\.Open$`)
-	return len(fns) > 0
+	_, err := t.client.GetState()
+	return len(fns) > 0 || isErrProcessExited(err)
 }

service/debugger/debugger.go

@@ -789,6 +789,18 @@ func createLogicalBreakpoint(d *Debugger, requestedBp *api.Breakpoint, setbp *pr
 
 	lbp.Set = *setbp
 
+	if lbp.Set.Expr != nil {
+		addrs := lbp.Set.Expr(d.Target())
+		if len(addrs) > 0 {
+			f, l, fn := d.Target().BinInfo().PCToLine(addrs[0])
+			lbp.File = f
+			lbp.Line = l
+			if fn != nil {
+				lbp.FunctionName = fn.Name
+			}
+		}
+	}
+
 	err = d.target.EnableBreakpoint(lbp)
 	if err != nil {
 		if suspended {
@@ -965,10 +977,6 @@ func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint
 
 // clearBreakpoint clears a breakpoint, we can consume this function to avoid locking a goroutine
 func (d *Debugger) clearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) {
-	if _, err := d.target.Valid(); err != nil {
-		return nil, err
-	}
-
 	if requestedBp.ID <= 0 {
 		if len(d.target.Targets()) != 1 {
 			return nil, ErrNotImplementedWithMultitarget

service/test/integration1_test.go

@@ -806,7 +806,7 @@ func Test1ClientServer_FullStacktrace(t *testing.T) {
 func Test1Issue355(t *testing.T) {
 	// After the target process has terminated should return an error but not crash
 	withTestClient1("continuetestprog", t, func(c *rpc1.RPCClient) {
-		bp, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: -1})
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: -1})
 		assertNoError(err, t, "CreateBreakpoint()")
 		ch := c.Continue()
 		state := <-ch
@@ -835,10 +835,6 @@ func Test1Issue355(t *testing.T) {
 		assertErrorOrExited(s, err, t, "SwitchGoroutine()")
 		s, err = c.Halt()
 		assertErrorOrExited(s, err, t, "Halt()")
-		_, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1})
-		assertError(err, t, "CreateBreakpoint()")
-		_, err = c.ClearBreakpoint(bp.ID)
-		assertError(err, t, "ClearBreakpoint()")
 		_, err = c.ListThreads()
 		assertError(err, t, "ListThreads()")
 		_, err = c.GetThread(tid)

service/test/integration2_test.go

@@ -996,7 +996,7 @@ func TestClientServer_FindLocations(t *testing.T) {
 		if strings.Contains(locsNoSubst[0].File, "\\") {
 			sep = "\\"
 		}
-		substRules := [][2]string{[2]string{strings.Replace(locsNoSubst[0].File, "locationsprog.go", "", 1), strings.Replace(locsNoSubst[0].File, "_fixtures"+sep+"locationsprog.go", "nonexistent", 1)}}
+		substRules := [][2]string{{strings.Replace(locsNoSubst[0].File, "locationsprog.go", "", 1), strings.Replace(locsNoSubst[0].File, "_fixtures"+sep+"locationsprog.go", "nonexistent", 1)}}
 		t.Logf("substitute rules: %q -> %q", substRules[0][0], substRules[0][1])
 		locsSubst, err := c.FindLocation(api.EvalScope{GoroutineID: -1}, "nonexistent/locationsprog.go:35", false, substRules)
 		if err != nil {
@@ -1305,7 +1305,7 @@ func TestIssue355(t *testing.T) {
 	// After the target process has terminated should return an error but not crash
 	protest.AllowRecording(t)
 	withTestClient2("continuetestprog", t, func(c service.Client) {
-		bp, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: -1})
+		_, err := c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.sayhi", Line: -1})
 		assertNoError(err, t, "CreateBreakpoint()")
 		ch := c.Continue()
 		state := <-ch
@@ -1334,14 +1334,6 @@ func TestIssue355(t *testing.T) {
 		assertErrorOrExited(s, err, t, "SwitchGoroutine()")
 		s, err = c.Halt()
 		assertErrorOrExited(s, err, t, "Halt()")
-		_, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1})
-		if testBackend != "rr" {
-			assertError(err, t, "CreateBreakpoint()")
-		}
-		_, err = c.ClearBreakpoint(bp.ID)
-		if testBackend != "rr" {
-			assertError(err, t, "ClearBreakpoint()")
-		}
 		_, err = c.ListThreads()
 		assertError(err, t, "ListThreads()")
 		_, err = c.GetThread(tid)
@@ -2959,6 +2951,48 @@ func TestPluginSuspendedBreakpoint(t *testing.T) {
 	})
 }
 
+// Tests that breakpoint set after the process has exited will be hit when the process is restarted.
+func TestBreakpointAfterProcessExit(t *testing.T) {
+	withTestClient2("continuetestprog", t, func(c service.Client) {
+		state := <-c.Continue()
+		if !state.Exited {
+			t.Fatal("process should have exited")
+		}
+		bp, err := c.CreateBreakpointWithExpr(&api.Breakpoint{ID: 2, FunctionName: "main.main", Line: 1}, "main.main", nil, true)
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, err = c.Restart(false)
+		if err != nil {
+			t.Fatal(err)
+		}
+		state = <-c.Continue()
+		if state.CurrentThread == nil {
+			t.Fatal("no current thread")
+		}
+		if state.CurrentThread.Breakpoint == nil {
+			t.Fatal("no breakpoint")
+		}
+		if state.CurrentThread.Breakpoint.ID != bp.ID {
+			t.Fatal("did not hit correct breakpoint")
+		}
+		if state.CurrentThread.Function == nil {
+			t.Fatal("no function")
+		}
+		if state.CurrentThread.Function.Name() != "main.main" {
+			t.Fatal("stopped at incorrect function")
+		}
+		state = <-c.Continue()
+		if !state.Exited {
+			t.Fatal("process should have exited")
+		}
+		_, err = c.ClearBreakpoint(bp.ID)
+		if err != nil {
+			t.Fatal(err)
+		}
+	})
+}
+
 func TestClientServer_createBreakpointWithID(t *testing.T) {
 	protest.AllowRecording(t)
 	withTestClient2("continuetestprog", t, func(c service.Client) {