Two bugfixes regarding stale executable files, and executables changing between restarts (#689)

* service/debugger: Restore breakpoints using file:line on restart

Restoring by address can cause the breakpoint to be inserted in the
middle of an instruction if the executable file has changed.

* terminal: Warn of stale executable when printing source

proc/proc.go

@@ -14,6 +14,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/derekparker/delve/dwarf/frame"
 	"github.com/derekparker/delve/dwarf/line"
@@ -24,8 +25,9 @@ import (
 // Process represents all of the information the debugger
 // is holding onto regarding the process we are debugging.
 type Process struct {
-	Pid     int         // Process Pid
-	Process *os.Process // Pointer to process struct for the actual process we are debugging
+	Pid          int         // Process Pid
+	Process      *os.Process // Pointer to process struct for the actual process we are debugging
+	LastModified time.Time   // Time the executable of this process was last modified
 
 	// Breakpoint table, holds information on breakpoints.
 	// Maps instruction address to Breakpoint struct.
@@ -157,10 +159,14 @@ func (dbp *Process) Running() bool {
 func (dbp *Process) LoadInformation(path string) error {
 	var wg sync.WaitGroup
 
-	exe, err := dbp.findExecutable(path)
+	exe, path, err := dbp.findExecutable(path)
 	if err != nil {
 		return err
 	}
+	fi, err := os.Stat(path)
+	if err == nil {
+		dbp.LastModified = fi.ModTime()
+	}
 
 	wg.Add(5)
 	go dbp.loadProcessInformation(&wg)

proc/proc_darwin.go

@@ -336,22 +336,22 @@ func (dbp *Process) parseDebugLineInfo(exe *macho.File, wg *sync.WaitGroup) {
 
 var UnsupportedArchErr = errors.New("unsupported architecture - only darwin/amd64 is supported")
 
-func (dbp *Process) findExecutable(path string) (*macho.File, error) {
+func (dbp *Process) findExecutable(path string) (*macho.File, string, error) {
 	if path == "" {
 		path = C.GoString(C.find_executable(C.int(dbp.Pid)))
 	}
 	exe, err := macho.Open(path)
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
 	if exe.Cpu != macho.CpuAmd64 {
-		return nil, UnsupportedArchErr
+		return nil, path, UnsupportedArchErr
 	}
 	dbp.dwarf, err = exe.DWARF()
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
-	return exe, nil
+	return exe, path, nil
 }
 
 func (dbp *Process) trapWait(pid int) (*Thread, error) {

proc/proc_linux.go

@@ -173,26 +173,26 @@ func (dbp *Process) updateThreadList() error {
 
 var UnsupportedArchErr = errors.New("unsupported architecture - only linux/amd64 is supported")
 
-func (dbp *Process) findExecutable(path string) (*elf.File, error) {
+func (dbp *Process) findExecutable(path string) (*elf.File, string, error) {
 	if path == "" {
 		path = fmt.Sprintf("/proc/%d/exe", dbp.Pid)
 	}
 	f, err := os.OpenFile(path, 0, os.ModePerm)
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
 	elfFile, err := elf.NewFile(f)
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
 	if elfFile.Machine != elf.EM_X86_64 {
-		return nil, UnsupportedArchErr
+		return nil, path, UnsupportedArchErr
 	}
 	dbp.dwarf, err = elfFile.DWARF()
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
-	return elfFile, nil
+	return elfFile, path, nil
 }
 
 func (dbp *Process) parseDebugFrame(exe *elf.File, wg *sync.WaitGroup) {

proc/proc_windows.go

@@ -390,19 +390,19 @@ func (dbp *Process) parseDebugLineInfo(exe *pe.File, wg *sync.WaitGroup) {
 
 var UnsupportedArchErr = errors.New("unsupported architecture of windows/386 - only windows/amd64 is supported")
 
-func (dbp *Process) findExecutable(path string) (*pe.File, error) {
+func (dbp *Process) findExecutable(path string) (*pe.File, string, error) {
 	peFile, err := openExecutablePath(path)
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
 	if peFile.Machine != pe.IMAGE_FILE_MACHINE_AMD64 {
-		return nil, UnsupportedArchErr
+		return nil, path, UnsupportedArchErr
 	}
 	dbp.dwarf, err = dwarfFromPE(peFile)
 	if err != nil {
-		return nil, err
+		return nil, path, err
 	}
-	return peFile, nil
+	return peFile, path, nil
 }
 
 func openExecutablePath(path string) (*pe.File, error) {

service/api/types.go

@@ -313,3 +313,8 @@ func (regs Registers) String() string {
 	}
 	return buf.String()
 }
+
+type DiscardedBreakpoint struct {
+	Breakpoint *Breakpoint
+	Reason error
+}

service/client.go

@@ -1,6 +1,8 @@
 package service
 
 import (
+	"time"
+
 	"github.com/derekparker/delve/service/api"
 )
 
@@ -10,11 +12,14 @@ type Client interface {
 	// Returns the pid of the process we are debugging.
 	ProcessPid() int
 
+	// LastModified returns the time that the process' executable was modified.
+	LastModified() time.Time
+
 	// Detach detaches the debugger, optionally killing the process.
 	Detach(killProcess bool) error
 
 	// Restarts program.
-	Restart() error
+	Restart() ([]api.DiscardedBreakpoint, error)
 
 	// GetState returns the current debugger state.
 	GetState() (*api.DebuggerState, error)

service/debugger/debugger.go

@@ -11,6 +11,7 @@ import (
 	"runtime"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/derekparker/delve/proc"
 	"github.com/derekparker/delve/service/api"
@@ -81,6 +82,12 @@ func (d *Debugger) ProcessPid() int {
 	return d.process.Pid
 }
 
+// LastModified returns the time that the process' executable was last
+// modified.
+func (d *Debugger) LastModified() time.Time {
+	return d.process.LastModified
+}
+
 // Detach detaches from the target process.
 // If `kill` is true we will kill the process after
 // detaching.
@@ -100,7 +107,7 @@ func (d *Debugger) detach(kill bool) error {
 
 // Restart will restart the target process, first killing
 // and then exec'ing it again.
-func (d *Debugger) Restart() error {
+func (d *Debugger) Restart() ([]api.DiscardedBreakpoint, error) {
 	d.processMutex.Lock()
 	defer d.processMutex.Unlock()
 
@@ -110,30 +117,38 @@ func (d *Debugger) Restart() error {
 		}
 		// Ensure the process is in a PTRACE_STOP.
 		if err := stopProcess(d.ProcessPid()); err != nil {
-			return err
+			return nil, err
 		}
 		if err := d.detach(true); err != nil {
-			return err
+			return nil, err
 		}
 	}
 	p, err := proc.Launch(d.config.ProcessArgs, d.config.WorkingDir)
 	if err != nil {
-		return fmt.Errorf("could not launch process: %s", err)
+		return nil, fmt.Errorf("could not launch process: %s", err)
 	}
+	discarded := []api.DiscardedBreakpoint{}
 	for _, oldBp := range d.breakpoints() {
 		if oldBp.ID < 0 {
 			continue
 		}
+		if len(oldBp.File) > 0 {
+			oldBp.Addr, err = d.process.FindFileLocation(oldBp.File, oldBp.Line)
+			if err != nil {
+				discarded = append(discarded, api.DiscardedBreakpoint{oldBp, err})
+				continue
+			}
+		}
 		newBp, err := p.SetBreakpoint(oldBp.Addr, proc.UserBreakpoint, nil)
 		if err != nil {
-			return err
+			return nil, err
 		}
 		if err := copyBreakpointInfo(newBp, oldBp); err != nil {
-			return err
+			return nil, err
 		}
 	}
 	d.process = p
-	return nil
+	return discarded, nil
 }
 
 // State returns the current state of the debugger.

service/rpc1/server.go

@@ -36,7 +36,8 @@ func (s *RPCServer) Restart(arg1 interface{}, arg2 *int) error {
 	if s.config.AttachPid != 0 {
 		return errors.New("cannot restart process Delve did not create")
 	}
-	return s.debugger.Restart()
+	_, err := s.debugger.Restart()
+	return err
 }
 
 func (s *RPCServer) State(arg interface{}, state *api.DebuggerState) error {

service/rpc2/client.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"net/rpc"
 	"net/rpc/jsonrpc"
+	"time"
 
 	"github.com/derekparker/delve/service"
 	"github.com/derekparker/delve/service/api"
@@ -37,14 +38,21 @@ func (c *RPCClient) ProcessPid() int {
 	return out.Pid
 }
 
+func (c *RPCClient) LastModified() time.Time {
+	out := new(LastModifiedOut)
+	c.call("LastModified", LastModifiedIn{}, out)
+	return out.Time
+}
+
 func (c *RPCClient) Detach(kill bool) error {
 	out := new(DetachOut)
 	return c.call("Detach", DetachIn{kill}, out)
 }
 
-func (c *RPCClient) Restart() error {
+func (c *RPCClient) Restart() ([]api.DiscardedBreakpoint, error) {
 	out := new(RestartOut)
-	return c.call("Restart", RestartIn{}, out)
+	err := c.call("Restart", RestartIn{}, out)
+	return out.DiscardedBreakpoints, err
 }
 
 func (c *RPCClient) GetState() (*api.DebuggerState, error) {

service/rpc2/server.go

@@ -3,6 +3,7 @@ package rpc2
 import (
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/derekparker/delve/service"
 	"github.com/derekparker/delve/service/api"
@@ -33,6 +34,18 @@ func (s *RPCServer) ProcessPid(arg ProcessPidIn, out *ProcessPidOut) error {
 	return nil
 }
 
+type LastModifiedIn struct {
+}
+
+type LastModifiedOut struct {
+	Time time.Time
+}
+
+func (s *RPCServer) LastModified(arg LastModifiedIn, out *LastModifiedOut) error {
+	out.Time = s.debugger.LastModified()
+	return nil
+}
+
 type DetachIn struct {
 	Kill bool
 }
@@ -49,6 +62,7 @@ type RestartIn struct {
 }
 
 type RestartOut struct {
+	DiscardedBreakpoints []api.DiscardedBreakpoint
 }
 
 // Restart restarts program.
@@ -56,7 +70,9 @@ func (s *RPCServer) Restart(arg RestartIn, out *RestartOut) error {
 	if s.config.AttachPid != 0 {
 		return errors.New("cannot restart process Delve did not create")
 	}
-	return s.debugger.Restart()
+	var err error
+	out.DiscardedBreakpoints, err = s.debugger.Restart()
+	return err
 }
 
 type StateIn struct {

service/test/integration2_test.go

@@ -71,7 +71,7 @@ func TestRestart_afterExit(t *testing.T) {
 		if !state.Exited {
 			t.Fatal("expected initial process to have exited")
 		}
-		if err := c.Restart(); err != nil {
+		if _, err := c.Restart(); err != nil {
 			t.Fatal(err)
 		}
 		if c.ProcessPid() == origPid {
@@ -124,7 +124,7 @@ func TestRestart_duringStop(t *testing.T) {
 		if state.CurrentThread.Breakpoint == nil {
 			t.Fatal("did not hit breakpoint")
 		}
-		if err := c.Restart(); err != nil {
+		if _, err := c.Restart(); err != nil {
 			t.Fatal(err)
 		}
 		if c.ProcessPid() == origPid {

terminal/command.go

@@ -568,10 +568,14 @@ func writeGoroutineLong(w io.Writer, g *api.Goroutine, prefix string) {
 }
 
 func restart(t *Term, ctx callContext, args string) error {
-	if err := t.client.Restart(); err != nil {
+	discarded, err := t.client.Restart()
+	if err != nil {
 		return err
 	}
 	fmt.Println("Process restarted with PID", t.client.ProcessPid())
+	for i := range discarded {
+		fmt.Println("Discarded %s at %s: %v\n", formatBreakpointName(discarded[i].Breakpoint, false), formatBreakpointLocation(discarded[i].Breakpoint), discarded[i].Reason)
+	}
 	return nil
 }
 
@@ -1266,6 +1270,12 @@ func printfile(t *Term, filename string, line int, showArrow bool) error {
 	}
 	defer file.Close()
 
+	fi, _ := file.Stat()
+	lastModExe := t.client.LastModified()
+	if fi.ModTime().After(lastModExe) {
+		fmt.Println("Warning: listing may not match stale executable")
+	}
+
 	buf := bufio.NewScanner(file)
 	l := line
 	for i := 1; i < l-5; i++ {