proc: replace SavedRegisters interface with a Copy method

Fncall.go was written with the assumption that the object returned by
proc.Thread.Registers does not change after we call
proc.Thread.SetPC/etc.

This is true for the native backend but not for gdbserial. I had
anticipated this problem and introduced the Save/SavedRegisters
mechanism during the first implementation of fncall.go but that's
insufficient.

Instead:

1. clarify that the object returned by proc.Thread.Registers could
   change when the CPU registers are modified.
2. add a Copy method to Registers that returns a copy of the registers
   that are guaranteed not to change when the CPU registers change.
3. remove the Save/SavedRegisters mechanism.

This solution leaves us the option, in the future, to cache the output
of proc.(Thread).Registers, avoiding a system call every time it's
called.

pkg/proc/core/core.go

@@ -239,7 +239,7 @@ func (t *Thread) Registers(floatingPoint bool) (proc.Registers, error) {
 	return r, nil
 }
 
-func (t *Thread) RestoreRegisters(proc.SavedRegisters) error {
+func (t *Thread) RestoreRegisters(proc.Registers) error {
 	return errors.New("not supported")
 }
 
@@ -426,6 +426,6 @@ func (r *Registers) Slice() []proc.Register {
 	return out
 }
 
-func (r *Registers) Save() proc.SavedRegisters {
+func (r *Registers) Copy() proc.Registers {
 	return r
 }

pkg/proc/fncall.go

@@ -58,7 +58,7 @@ type functionCallState struct {
 	// finished is true if the function call terminated
 	finished bool
 	// savedRegs contains the saved registers
-	savedRegs SavedRegisters
+	savedRegs Registers
 	// expr contains an expression describing the current function call
 	expr string
 	// err contains a saved error
@@ -112,6 +112,7 @@ func CallFunction(p Process, expr string, retLoadCfg *LoadConfig) error {
 	if err != nil {
 		return err
 	}
+	regs = regs.Copy()
 	if regs.SP()-256 <= g.stacklo {
 		return ErrNotEnoughStack
 	}
@@ -139,7 +140,7 @@ func CallFunction(p Process, expr string, retLoadCfg *LoadConfig) error {
 	}
 
 	fncall.inProgress = true
-	fncall.savedRegs = regs.Save()
+	fncall.savedRegs = regs
 	fncall.expr = expr
 	fncall.fn = fn
 	fncall.closureAddr = closureAddr
@@ -155,7 +156,7 @@ func fncallLog(fmtstr string, args ...interface{}) {
 	if !logflags.FnCall() {
 		return
 	}
-	logrus.WithFields(logrus.Fields{"layer": "proc", "kind": "fncall"}).Debugf(fmtstr, args...)
+	logrus.WithFields(logrus.Fields{"layer": "proc", "kind": "fncall"}).Infof(fmtstr, args...)
 }
 
 // writePointer writes val as an architecture pointer at addr in mem.
@@ -401,6 +402,7 @@ func (fncall *functionCallState) step(p Process) {
 		fncall.inProgress = false
 		return
 	}
+	regs = regs.Copy()
 
 	rax, _ := regs.Get(int(x86asm.RAX))
 

pkg/proc/gdbserial/gdbserver.go

@@ -187,6 +187,7 @@ func New(process *os.Process) *Process {
 		gcmdok:         true,
 		threadStopInfo: true,
 		process:        process,
+		common:         proc.NewCommonProcess(true),
 	}
 
 	if process != nil {
@@ -1188,9 +1189,8 @@ func (t *Thread) Registers(floatingPoint bool) (proc.Registers, error) {
 	return &t.regs, nil
 }
 
-func (t *Thread) RestoreRegisters(regs proc.SavedRegisters) error {
+func (t *Thread) RestoreRegisters(savedRegs proc.Registers) error {
-	gdbregs := regs.(*gdbRegisters)
+	copy(t.regs.buf, savedRegs.(*gdbRegisters).buf)
-	t.regs = *gdbregs
 	return t.writeRegisters()
 }
 
@@ -1804,7 +1804,7 @@ func (regs *gdbRegisters) Slice() []proc.Register {
 	return r
 }
 
-func (regs *gdbRegisters) Save() proc.SavedRegisters {
+func (regs *gdbRegisters) Copy() proc.Registers {
 	savedRegs := &gdbRegisters{}
 	savedRegs.init(regs.regsInfo)
 	copy(savedRegs.buf, regs.buf)

pkg/proc/gdbserial/gdbserver_conn.go

@@ -866,6 +866,10 @@ func writeAsciiBytes(w io.Writer, data []byte) {
 
 // executes 'M' (write memory) command
 func (conn *gdbConn) writeMemory(addr uintptr, data []byte) (written int, err error) {
+	if len(data) == 0 {
+		// LLDB can't parse requests for 0-length writes and hangs if we emit them
+		return 0, nil
+	}
 	conn.outbuf.Reset()
 	//TODO(aarzilli): do not send packets larger than conn.PacketSize
 	fmt.Fprintf(&conn.outbuf, "$M%x,%x:", addr, len(data))

pkg/proc/native/registers_darwin_amd64.go

@@ -367,10 +367,7 @@ func registers(thread *Thread, floatingPoint bool) (proc.Registers, error) {
 	return regs, nil
 }
 
-type savedRegisters struct {
+func (r *Regs) Copy() proc.Registers {
-}
-
-func (r *Regs) Save() proc.SavedRegisters {
 	//TODO(aarzilli): implement this to support function calls
 	return nil
 }

pkg/proc/native/registers_linux_amd64.go

@@ -324,14 +324,6 @@ func (thread *Thread) fpRegisters() (regs []proc.Register, fpregs proc.LinuxX86X
 	return
 }
 
-type savedRegisters struct {
+func (r *Regs) Copy() proc.Registers {
-	regs   sys.PtraceRegs
+	return r
-	fpregs proc.LinuxX86Xstate
-}
-
-func (r *Regs) Save() proc.SavedRegisters {
-	savedRegs := &savedRegisters{}
-	savedRegs.regs = *r.regs
-	savedRegs.fpregs = *r.fpregset
-	return savedRegs
 }

pkg/proc/native/registers_windows_amd64.go

@@ -378,10 +378,7 @@ func registers(thread *Thread, floatingPoint bool) (proc.Registers, error) {
 	return regs, nil
 }
 
-type savedRegisters struct {
+func (r *Regs) Copy() proc.Registers {
-}
-
-func (r *Regs) Save() proc.SavedRegisters {
 	//TODO(aarzilli): implement this to support function calls
 	return nil
 }

pkg/proc/native/threads.go

@@ -1,7 +1,6 @@
 package native
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/derekparker/delve/pkg/proc"
@@ -151,12 +150,8 @@ func (t *Thread) Registers(floatingPoint bool) (proc.Registers, error) {
 	return registers(t, floatingPoint)
 }
 
-func (t *Thread) RestoreRegisters(savedRegs proc.SavedRegisters) error {
+func (t *Thread) RestoreRegisters(savedRegs proc.Registers) error {
-	sr, ok := savedRegs.(*savedRegisters)
+	return t.restoreRegisters(savedRegs)
-	if !ok {
-		return errors.New("unknown saved register set")
-	}
-	return t.restoreRegisters(sr)
 }
 
 func (t *Thread) PC() (uint64, error) {

pkg/proc/native/threads_darwin.go

@@ -146,6 +146,6 @@ func (t *Thread) ReadMemory(buf []byte, addr uintptr) (int, error) {
 	return len(buf), nil
 }
 
-func (t *Thread) restoreRegisters(sr *savedRegisters) error {
+func (t *Thread) restoreRegisters(sr proc.Registers) error {
 	return errors.New("not implemented")
 }

pkg/proc/native/threads_linux.go

@@ -82,20 +82,22 @@ func (t *Thread) Blocked() bool {
 	return false
 }
 
-func (t *Thread) restoreRegisters(sr *savedRegisters) error {
+func (t *Thread) restoreRegisters(savedRegs proc.Registers) error {
+	sr := savedRegs.(*Regs)
+
 	var restoreRegistersErr error
 	t.dbp.execPtraceFunc(func() {
-		restoreRegistersErr = sys.PtraceSetRegs(t.ID, &sr.regs)
+		restoreRegistersErr = sys.PtraceSetRegs(t.ID, sr.regs)
 		if restoreRegistersErr != nil {
 			return
 		}
-		if sr.fpregs.Xsave != nil {
+		if sr.fpregset.Xsave != nil {
-			iov := sys.Iovec{Base: &sr.fpregs.Xsave[0], Len: uint64(len(sr.fpregs.Xsave))}
+			iov := sys.Iovec{Base: &sr.fpregset.Xsave[0], Len: uint64(len(sr.fpregset.Xsave))}
 			_, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETREGSET, uintptr(t.ID), _NT_X86_XSTATE, uintptr(unsafe.Pointer(&iov)), 0, 0)
 			return
 		}
 
-		_, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETFPREGS, uintptr(t.ID), uintptr(0), uintptr(unsafe.Pointer(&sr.fpregs.PtraceFpRegs)), 0, 0)
+		_, _, restoreRegistersErr = syscall.Syscall6(syscall.SYS_PTRACE, sys.PTRACE_SETFPREGS, uintptr(t.ID), uintptr(0), uintptr(unsafe.Pointer(&sr.fpregset.PtraceFpRegs)), 0, 0)
 		return
 	})
 	if restoreRegistersErr == syscall.Errno(0) {

pkg/proc/native/threads_windows.go

@@ -150,6 +150,6 @@ func (t *Thread) ReadMemory(buf []byte, addr uintptr) (int, error) {
 	return int(count), err
 }
 
-func (t *Thread) restoreRegisters(sr *savedRegisters) error {
+func (t *Thread) restoreRegisters(savedRegs proc.Registers) error {
 	return errors.New("not implemented")
 }

pkg/proc/registers.go

@@ -24,8 +24,9 @@ type Registers interface {
 	GAddr() (uint64, bool)
 	Get(int) (uint64, error)
 	Slice() []Register
-	// Save saves a copy of this object that will survive restarts
+	// Copy returns a copy of this registers that is guaranteed not to change
-	Save() SavedRegisters
+	// when the registers of the associated thread change.
+	Copy() Registers
 }
 
 type Register struct {
@@ -34,9 +35,6 @@ type Register struct {
 	Value string
 }
 
-type SavedRegisters interface {
-}
-
 // AppendWordReg appends a word (16 bit) register to regs.
 func AppendWordReg(regs []Register, name string, value uint16) []Register {
 	var buf bytes.Buffer

pkg/proc/test/support.go

@@ -250,7 +250,19 @@ func MustSupportFunctionCalls(t *testing.T, testBackend string) {
 	if !goversion.VersionAfterOrEqual(runtime.Version(), 1, 11) {
 		t.Skip("this version of Go does not support function calls")
 	}
-	if runtime.GOOS == "windows" {
+
-		t.Skip("this backend does not support function calls")
+	const unsupported = "this backend does not support function calls"
+
+	if testBackend == "rr" {
+		t.Skip(unsupported)
+	}
+
+	switch runtime.GOOS {
+	case "darwin":
+		if testBackend == "native" {
+			t.Skip(unsupported)
+		}
+	case "windows":
+		t.Skip(unsupported)
 	}
 }

pkg/proc/threads.go

@@ -22,9 +22,17 @@ type Thread interface {
 	// nil if the thread is not stopped at any breakpoint.
 	Breakpoint() BreakpointState
 	ThreadID() int
+
+	// Registers returns the CPU registers of this thread. The contents of the
+	// variable returned may or may not change to reflect the new CPU status
+	// when the thread is resumed or the registers are changed by calling
+	// SetPC/SetSP/etc.
+	// To insure that the the returned variable won't change call the Copy
+	// method of Registers.
 	Registers(floatingPoint bool) (Registers, error)
+
 	// RestoreRegisters restores saved registers
-	RestoreRegisters(SavedRegisters) error
+	RestoreRegisters(Registers) error
 	Arch() Arch
 	BinInfo() *BinaryInfo
 	StepInstruction() error