Repository: go-delve/delve
Pull request #2064 (2020-05-26 12:16:40)

proc/*: move Set/Clear Breakpoint methods to Target
```
proc/*: move Set/Clear Breakpoint methods to Target

```



Patch:
diff --git a/_fixtures/issue2023.go b/_fixtures/issue2023.go
new file mode 100644
index 00000000..b8ab8448
--- /dev/null
+++ b/_fixtures/issue2023.go
@@ -0,0 +1,22 @@
+package main
+
+import (
+	"fmt"
+	"time"
+)
+
+func A() {
+	fmt.Printf("hello delve\n")
+}
+
+func main() {
+	count := 0
+	for {
+		A()
+		time.Sleep(time.Millisecond * time.Duration(100))
+		if count >= 30 {
+			break
+		}
+		count++
+	}
+}
diff --git a/cmd/dlv/cmds/commands.go b/cmd/dlv/cmds/commands.go
index 081c4f6e..2e6f1236 100644
--- a/cmd/dlv/cmds/commands.go
+++ b/cmd/dlv/cmds/commands.go
@@ -466,20 +466,19 @@ func traceCmd(cmd *cobra.Command, args []string) {
 		var processArgs []string
 
 		dlvArgs, targetArgs := splitArgs(cmd, args)
+		var dlvArgsLen = len(dlvArgs)
+		if dlvArgsLen == 1 {
+			regexp = args[0]
+			dlvArgs = dlvArgs[0:0]
+		} else if dlvArgsLen >= 2 {
+			regexp = dlvArgs[dlvArgsLen-1]
+			dlvArgs = dlvArgs[:dlvArgsLen-1]
+		}
 
 		if traceAttachPid == 0 {
-			var dlvArgsLen = len(dlvArgs)
-
-			if dlvArgsLen == 1 {
-				regexp = args[0]
-				dlvArgs = dlvArgs[0:0]
-			} else if dlvArgsLen >= 2 {
-				if traceExecFile != "" {
-					fmt.Fprintln(os.Stderr, "Cannot specify package when using exec.")
-					return 1
-				}
-				regexp = dlvArgs[dlvArgsLen-1]
-				dlvArgs = dlvArgs[:dlvArgsLen-1]
+			if dlvArgsLen >= 2 && traceExecFile != "" {
+				fmt.Fprintln(os.Stderr, "Cannot specify package when using exec.")
+				return 1
 			}
 
 			debugname := traceExecFile
diff --git a/pkg/proc/breakpoints.go b/pkg/proc/breakpoints.go
index 3e01bea5..b331dbf0 100644
--- a/pkg/proc/breakpoints.go
+++ b/pkg/proc/breakpoints.go
@@ -237,21 +237,13 @@ func NewBreakpointMap() BreakpointMap {
 	}
 }
 
-// ResetBreakpointIDCounter resets the breakpoint ID counter of bpmap.
-func (bpmap *BreakpointMap) ResetBreakpointIDCounter() {
-	bpmap.breakpointIDCounter = 0
-}
-
-// WriteBreakpointFn is a type that represents a function to be used for
-// writting breakpoings into the target.
-type WriteBreakpointFn func(addr uint64) (file string, line int, fn *Function, originalData []byte, err error)
-
-type clearBreakpointFn func(*Breakpoint) error
-
-// Set creates a breakpoint at addr calling writeBreakpoint. Do not call this
-// function, call proc.Process.SetBreakpoint instead, this function exists
-// to implement proc.Process.SetBreakpoint.
-func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr, writeBreakpoint WriteBreakpointFn) (*Breakpoint, error) {
+// SetBreakpoint sets a breakpoint at addr, and stores it in the process wide
+// break point table.
+func (t *Target) SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error) {
+	if valid, err := t.Valid(); !valid {
+		return nil, err
+	}
+	bpmap := t.Breakpoints()
 	if bp, ok := bpmap.M[addr]; ok {
 		// We can overlap one internal breakpoint with one user breakpoint, we
 		// need to support this otherwise a conditional breakpoint can mask a
@@ -268,7 +260,7 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr,
 		return bp, nil
 	}
 
-	f, l, fn, originalData, err := writeBreakpoint(addr)
+	f, l, fn, originalData, err := t.proc.WriteBreakpoint(addr)
 	if err != nil {
 		return nil, err
 	}
@@ -303,9 +295,10 @@ func (bpmap *BreakpointMap) Set(addr uint64, kind BreakpointKind, cond ast.Expr,
 	return newBreakpoint, nil
 }
 
-// SetWithID creates a breakpoint at addr, with the specified logical ID.
-func (bpmap *BreakpointMap) SetWithID(id int, addr uint64, writeBreakpoint WriteBreakpointFn) (*Breakpoint, error) {
-	bp, err := bpmap.Set(addr, UserBreakpoint, nil, writeBreakpoint)
+// setBreakpointWithID creates a breakpoint at addr, with the specified logical ID.
+func (t *Target) setBreakpointWithID(id int, addr uint64) (*Breakpoint, error) {
+	bpmap := t.Breakpoints()
+	bp, err := t.SetBreakpoint(addr, UserBreakpoint, nil)
 	if err == nil {
 		bp.LogicalID = id
 		bpmap.breakpointIDCounter--
@@ -313,9 +306,12 @@ func (bpmap *BreakpointMap) SetWithID(id int, addr uint64, writeBreakpoint Write
 	return bp, err
 }
 
-// Clear clears the breakpoint at addr.
-// Do not call this function call proc.Process.ClearBreakpoint instead.
-func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn) (*Breakpoint, error) {
+// ClearBreakpoint clears the breakpoint at addr.
+func (t *Target) ClearBreakpoint(addr uint64) (*Breakpoint, error) {
+	if valid, err := t.Valid(); !valid {
+		return nil, err
+	}
+	bpmap := t.Breakpoints()
 	bp, ok := bpmap.M[addr]
 	if !ok {
 		return nil, NoBreakpointError{Addr: addr}
@@ -327,7 +323,7 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn
 		return bp, nil
 	}
 
-	if err := clearBreakpoint(bp); err != nil {
+	if err := t.proc.EraseBreakpoint(bp); err != nil {
 		return nil, err
 	}
 
@@ -338,9 +334,9 @@ func (bpmap *BreakpointMap) Clear(addr uint64, clearBreakpoint clearBreakpointFn
 
 // ClearInternalBreakpoints removes all internal breakpoints from the map,
 // calling clearBreakpoint on each one.
-// Do not call this function, call proc.Process.ClearInternalBreakpoints
-// instead, this function is used to implement that.
-func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakpointFn) error {
+func (t *Target) ClearInternalBreakpoints() error {
+	bpmap := t.Breakpoints()
+	threads := t.ThreadList()
 	for addr, bp := range bpmap.M {
 		bp.Kind = bp.Kind & UserBreakpoint
 		bp.internalCond = nil
@@ -348,9 +344,14 @@ func (bpmap *BreakpointMap) ClearInternalBreakpoints(clearBreakpoint clearBreakp
 		if bp.Kind != 0 {
 			continue
 		}
-		if err := clearBreakpoint(bp); err != nil {
+		if err := t.proc.EraseBreakpoint(bp); err != nil {
 			return err
 		}
+		for _, thread := range threads {
+			if thread.Breakpoint().Breakpoint == bp {
+				thread.Breakpoint().Clear()
+			}
+		}
 		delete(bpmap.M, addr)
 	}
 	return nil
diff --git a/pkg/proc/core/core.go b/pkg/proc/core/core.go
index 41c7df79..984ffdf3 100644
--- a/pkg/proc/core/core.go
+++ b/pkg/proc/core/core.go
@@ -3,7 +3,6 @@ package core
 import (
 	"errors"
 	"fmt"
-	"go/ast"
 	"io"
 
 	"github.com/go-delve/delve/pkg/proc"
@@ -216,7 +215,6 @@ func OpenCore(corePath, exePath string, debugInfoDirs []string) (*proc.Target, e
 	return proc.NewTarget(p, proc.NewTargetConfig{
 		Path:                exePath,
 		DebugInfoDirs:       debugInfoDirs,
-		WriteBreakpoint:     p.writeBreakpoint,
 		DisableAsyncPreempt: false,
 		StopReason:          proc.StopAttached})
 }
@@ -231,9 +229,9 @@ func (p *process) EntryPoint() (uint64, error) {
 	return p.entryPoint, nil
 }
 
-// writeBreakpoint is a noop function since you
+// WriteBreakpoint is a noop function since you
 // cannot write breakpoints into core files.
-func (p *process) writeBreakpoint(addr uint64) (file string, line int, fn *proc.Function, originalData []byte, err error) {
+func (p *process) WriteBreakpoint(addr uint64) (file string, line int, fn *proc.Function, originalData []byte, err error) {
 	return "", 0, nil, nil, errors.New("cannot write a breakpoint to a core file")
 }
 
@@ -365,10 +363,10 @@ func (p *process) Breakpoints() *proc.BreakpointMap {
 	return &p.breakpoints
 }
 
-// ClearBreakpoint will always return an error as you cannot set or clear
+// EraseBreakpoint will always return an error as you cannot set or clear
 // breakpoints on core files.
-func (p *process) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) {
-	return nil, proc.NoBreakpointError{Addr: addr}
+func (p *process) EraseBreakpoint(bp *proc.Breakpoint) error {
+	return proc.NoBreakpointError{Addr: bp.Addr}
 }
 
 // ClearInternalBreakpoints will always return nil and have no
@@ -430,11 +428,6 @@ func (p *process) Pid() int {
 func (p *process) ResumeNotify(chan<- struct{}) {
 }
 
-// SetBreakpoint will always return an error for core files as you cannot write memory or control execution.
-func (p *process) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) {
-	return nil, ErrWriteCore
-}
-
 // ThreadList will return a list of all threads currently in the process.
 func (p *process) ThreadList() []proc.Thread {
 	r := make([]proc.Thread, 0, len(p.Threads))
diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go
index c88ed334..dbfb82c8 100644
--- a/pkg/proc/gdbserial/gdbserver.go
+++ b/pkg/proc/gdbserial/gdbserver.go
@@ -67,7 +67,6 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"go/ast"
 	"net"
 	"os"
 	"os/exec"
@@ -555,7 +554,6 @@ func (p *gdbProcess) initialize(path string, debugInfoDirs []string, stopReason
 	tgt, err := proc.NewTarget(p, proc.NewTargetConfig{
 		Path:                path,
 		DebugInfoDirs:       debugInfoDirs,
-		WriteBreakpoint:     p.writeBreakpoint,
 		DisableAsyncPreempt: runtime.GOOS == "darwin",
 		StopReason:          stopReason})
 	if err != nil {
@@ -1083,7 +1081,7 @@ func (p *gdbProcess) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) {
 	return nil, false
 }
 
-func (p *gdbProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) {
+func (p *gdbProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) {
 	f, l, fn := p.bi.PCToLine(uint64(addr))
 
 	if err := p.conn.setBreakpoint(addr); err != nil {
@@ -1093,37 +1091,8 @@ func (p *gdbProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function,
 	return f, l, fn, nil, nil
 }
 
-// SetBreakpoint creates a new breakpoint.
-func (p *gdbProcess) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) {
-	if p.exited {
-		return nil, &proc.ErrProcessExited{Pid: p.conn.pid}
-	}
-	return p.breakpoints.Set(addr, kind, cond, p.writeBreakpoint)
-}
-
-// ClearBreakpoint clears a breakpoint at the given address.
-func (p *gdbProcess) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) {
-	if p.exited {
-		return nil, &proc.ErrProcessExited{Pid: p.conn.pid}
-	}
-	return p.breakpoints.Clear(addr, func(bp *proc.Breakpoint) error {
-		return p.conn.clearBreakpoint(bp.Addr)
-	})
-}
-
-// ClearInternalBreakpoints clear all internal use breakpoints like those set by 'next'.
-func (p *gdbProcess) ClearInternalBreakpoints() error {
-	return p.breakpoints.ClearInternalBreakpoints(func(bp *proc.Breakpoint) error {
-		if err := p.conn.clearBreakpoint(bp.Addr); err != nil {
-			return err
-		}
-		for _, thread := range p.threads {
-			if thread.CurrentBreakpoint.Breakpoint == bp {
-				thread.clearBreakpointState()
-			}
-		}
-		return nil
-	})
+func (p *gdbProcess) EraseBreakpoint(bp *proc.Breakpoint) error {
+	return p.conn.clearBreakpoint(bp.Addr)
 }
 
 type threadUpdater struct {
diff --git a/pkg/proc/interface.go b/pkg/proc/interface.go
index d5f7a729..cd1ad0c8 100644
--- a/pkg/proc/interface.go
+++ b/pkg/proc/interface.go
@@ -1,9 +1,5 @@
 package proc
 
-import (
-	"go/ast"
-)
-
 // Process represents the target of the debugger. This
 // target could be a system process, core file, etc.
 //
@@ -14,8 +10,9 @@ import (
 type Process interface {
 	Info
 	ProcessManipulation
-	BreakpointManipulation
 	RecordingManipulation
+
+	Breakpoints() *BreakpointMap
 }
 
 // ProcessInternal holds a set of methods that are not meant to be called by
@@ -32,6 +29,9 @@ type ProcessInternal interface {
 	Restart(pos string) error
 	Detach(bool) error
 	ContinueOnce() (trapthread Thread, stopReason StopReason, err error)
+
+	WriteBreakpoint(addr uint64) (file string, line int, fn *Function, originalData []byte, err error)
+	EraseBreakpoint(*Breakpoint) error
 }
 
 // RecordingManipulation is an interface for manipulating process recordings.
@@ -101,11 +101,3 @@ type ProcessManipulation interface {
 	// after a call to RequestManualStop.
 	CheckAndClearManualStopRequest() bool
 }
-
-// BreakpointManipulation is an interface for managing breakpoints.
-type BreakpointManipulation interface {
-	Breakpoints() *BreakpointMap
-	SetBreakpoint(addr uint64, kind BreakpointKind, cond ast.Expr) (*Breakpoint, error)
-	ClearBreakpoint(addr uint64) (*Breakpoint, error)
-	ClearInternalBreakpoints() error
-}
diff --git a/pkg/proc/native/proc.go b/pkg/proc/native/proc.go
index 5381c506..9e4aeb61 100644
--- a/pkg/proc/native/proc.go
+++ b/pkg/proc/native/proc.go
@@ -1,7 +1,6 @@
 package native
 
 import (
-	"go/ast"
 	"os"
 	"runtime"
 	"sync"
@@ -115,15 +114,6 @@ func (dbp *nativeProcess) Detach(kill bool) (err error) {
 		dbp.bi.Close()
 		return nil
 	}
-	// Clean up any breakpoints we've set.
-	for _, bp := range dbp.breakpoints.M {
-		if bp != nil {
-			_, err := dbp.ClearBreakpoint(bp.Addr)
-			if err != nil {
-				return err
-			}
-		}
-	}
 	dbp.execPtraceFunc(func() {
 		err = dbp.detach(kill)
 		if err != nil {
@@ -215,7 +205,7 @@ func (dbp *nativeProcess) CheckAndClearManualStopRequest() bool {
 	return msr
 }
 
-func (dbp *nativeProcess) writeBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) {
+func (dbp *nativeProcess) WriteBreakpoint(addr uint64) (string, int, *proc.Function, []byte, error) {
 	f, l, fn := dbp.bi.PCToLine(uint64(addr))
 
 	originalData := make([]byte, dbp.bi.Arch.BreakpointSize())
@@ -230,18 +220,8 @@ func (dbp *nativeProcess) writeBreakpoint(addr uint64) (string, int, *proc.Funct
 	return f, l, fn, originalData, nil
 }
 
-// SetBreakpoint sets a breakpoint at addr, and stores it in the process wide
-// break point table.
-func (dbp *nativeProcess) SetBreakpoint(addr uint64, kind proc.BreakpointKind, cond ast.Expr) (*proc.Breakpoint, error) {
-	return dbp.breakpoints.Set(addr, kind, cond, dbp.writeBreakpoint)
-}
-
-// ClearBreakpoint clears the breakpoint at addr.
-func (dbp *nativeProcess) ClearBreakpoint(addr uint64) (*proc.Breakpoint, error) {
-	if dbp.exited {
-		return nil, &proc.ErrProcessExited{Pid: dbp.Pid()}
-	}
-	return dbp.breakpoints.Clear(addr, dbp.currentThread.ClearBreakpoint)
+func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error {
+	return dbp.currentThread.ClearBreakpoint(bp)
 }
 
 // ContinueOnce will continue the target until it stops.
@@ -305,27 +285,10 @@ func (dbp *nativeProcess) initialize(path string, debugInfoDirs []string) (*proc
 	return proc.NewTarget(dbp, proc.NewTargetConfig{
 		Path:                path,
 		DebugInfoDirs:       debugInfoDirs,
-		WriteBreakpoint:     dbp.writeBreakpoint,
 		DisableAsyncPreempt: runtime.GOOS == "windows" || runtime.GOOS == "freebsd",
 		StopReason:          stopReason})
 }
 
-// ClearInternalBreakpoints will clear all non-user set breakpoints. These
-// breakpoints are set for internal operations such as 'next'.
-func (dbp *nativeProcess) ClearInternalBreakpoints() error {
-	return dbp.breakpoints.ClearInternalBreakpoints(func(bp *proc.Breakpoint) error {
-		if err := dbp.currentThread.ClearBreakpoint(bp); err != nil {
-			return err
-		}
-		for _, thread := range dbp.threads {
-			if thread.CurrentBreakpoint.Breakpoint == bp {
-				thread.CurrentBreakpoint.Clear()
-			}
-		}
-		return nil
-	})
-}
-
 func (dbp *nativeProcess) handlePtraceFuncs() {
 	// We must ensure here that we are running on the same thread during
 	// while invoking the ptrace(2) syscall. This is due to the fact that ptrace(2) expects
diff --git a/pkg/proc/target.go b/pkg/proc/target.go
index 5aedcf5e..9711275e 100644
--- a/pkg/proc/target.go
+++ b/pkg/proc/target.go
@@ -80,11 +80,10 @@ const (
 
 // NewTargetConfig contains the configuration for a new Target object,
 type NewTargetConfig struct {
-	Path                string            // path of the main executable
-	DebugInfoDirs       []string          // Directories to search for split debug info
-	WriteBreakpoint     WriteBreakpointFn // Function to write a breakpoint to the target process
-	DisableAsyncPreempt bool              // Go 1.14 asynchronous preemption should be disabled
-	StopReason          StopReason        // Initial stop reason
+	Path                string     // path of the main executable
+	DebugInfoDirs       []string   // Directories to search for split debug info
+	DisableAsyncPreempt bool       // Go 1.14 asynchronous preemption should be disabled
+	StopReason          StopReason // Initial stop reason
 }
 
 // DisableAsyncPreemptEnv returns a process environment (like os.Environ)
@@ -128,8 +127,8 @@ func NewTarget(p Process, cfg NewTargetConfig) (*Target, error) {
 	g, _ := GetG(p.CurrentThread())
 	t.selectedGoroutine = g
 
-	createUnrecoveredPanicBreakpoint(p, cfg.WriteBreakpoint)
-	createFatalThrowBreakpoint(p, cfg.WriteBreakpoint)
+	t.createUnrecoveredPanicBreakpoint()
+	t.createFatalThrowBreakpoint()
 
 	t.gcache.init(p.BinInfo())
 
@@ -216,8 +215,18 @@ func (p *Target) SwitchThread(tid int) error {
 // we were previously debugging.
 // If kill is true then the process will be killed when we detach.
 func (t *Target) Detach(kill bool) error {
-	if !kill && t.asyncPreemptChanged {
-		setAsyncPreemptOff(t, t.asyncPreemptOff)
+	if !kill {
+		if t.asyncPreemptChanged {
+			setAsyncPreemptOff(t, t.asyncPreemptOff)
+		}
+		for _, bp := range t.Breakpoints().M {
+			if bp != nil {
+				_, err := t.ClearBreakpoint(bp.Addr)
+				if err != nil {
+					return err
+				}
+			}
+		}
 	}
 	t.StopReason = StopUnknown
 	return t.proc.Detach(kill)
@@ -255,13 +264,13 @@ func setAsyncPreemptOff(p *Target, v int64) {
 }
 
 // createUnrecoveredPanicBreakpoint creates the unrecoverable-panic breakpoint.
-func createUnrecoveredPanicBreakpoint(p Process, writeBreakpoint WriteBreakpointFn) {
-	panicpcs, err := FindFunctionLocation(p, "runtime.startpanic", 0)
+func (t *Target) createUnrecoveredPanicBreakpoint() {
+	panicpcs, err := FindFunctionLocation(t.Process, "runtime.startpanic", 0)
 	if _, isFnNotFound := err.(*ErrFunctionNotFound); isFnNotFound {
-		panicpcs, err = FindFunctionLocation(p, "runtime.fatalpanic", 0)
+		panicpcs, err = FindFunctionLocation(t.Process, "runtime.fatalpanic", 0)
 	}
 	if err == nil {
-		bp, err := p.Breakpoints().SetWithID(unrecoveredPanicID, panicpcs[0], writeBreakpoint)
+		bp, err := t.setBreakpointWithID(unrecoveredPanicID, panicpcs[0])
 		if err == nil {
 			bp.Name = UnrecoveredPanic
 			bp.Variables = []string{"runtime.curg._panic.arg"}
@@ -270,10 +279,10 @@ func createUnrecoveredPanicBreakpoint(p Process, writeBreakpoint WriteBreakpoint
 }
 
 // createFatalThrowBreakpoint creates the a breakpoint as runtime.fatalthrow.
-func createFatalThrowBreakpoint(p Process, writeBreakpoint WriteBreakpointFn) {
-	fatalpcs, err := FindFunctionLocation(p, "runtime.fatalthrow", 0)
+func (t *Target) createFatalThrowBreakpoint() {
+	fatalpcs, err := FindFunctionLocation(t.Process, "runtime.fatalthrow", 0)
 	if err == nil {
-		bp, err := p.Breakpoints().SetWithID(fatalThrowID, fatalpcs[0], writeBreakpoint)
+		bp, err := t.setBreakpointWithID(fatalThrowID, fatalpcs[0])
 		if err == nil {
 			bp.Name = FatalThrow
 		}
diff --git a/pkg/proc/target_exec.go b/pkg/proc/target_exec.go
index 792a66ca..8ac620b5 100644
--- a/pkg/proc/target_exec.go
+++ b/pkg/proc/target_exec.go
@@ -625,7 +625,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 	return nil
 }
 
-func setStepIntoBreakpoints(dbp Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
+func setStepIntoBreakpoints(dbp *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
 	for _, instr := range text {
 		if instr.Loc.File != topframe.Current.File || instr.Loc.Line != topframe.Current.Line || !instr.IsCall() {
 			continue
@@ -645,7 +645,7 @@ func setStepIntoBreakpoints(dbp Process, text []AsmInstruction, topframe Stackfr
 	return nil
 }
 
-func setStepIntoBreakpointsReverse(dbp Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
+func setStepIntoBreakpointsReverse(dbp *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr) error {
 	// Set a breakpoint after every CALL instruction
 	for i, instr := range text {
 		if instr.Loc.File != topframe.Current.File || !instr.IsCall() || instr.DestLoc == nil || instr.DestLoc.Fn == nil {
@@ -708,7 +708,7 @@ func removePCsBetween(pcs []uint64, start, end uint64) []uint64 {
 	return out
 }
 
-func setStepIntoBreakpoint(dbp Process, text []AsmInstruction, cond ast.Expr) error {
+func setStepIntoBreakpoint(dbp *Target, text []AsmInstruction, cond ast.Expr) error {
 	if len(text) <= 0 {
 		return nil
 	}
@@ -873,7 +873,7 @@ func skipAutogeneratedWrappersOut(g *G, thread Thread, startTopframe, startRetfr
 
 // setDeferBreakpoint is a helper function used by next and StepOut to set a
 // breakpoint on the first deferred function.
-func setDeferBreakpoint(p Process, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr, stepInto bool) (uint64, error) {
+func setDeferBreakpoint(p *Target, text []AsmInstruction, topframe Stackframe, sameGCond ast.Expr, stepInto bool) (uint64, error) {
 	// Set breakpoint on the most recently deferred function (if any)
 	var deferpc uint64
 	if topframe.TopmostDefer != nil && topframe.TopmostDefer.DeferredPC != 0 {
diff --git a/pkg/terminal/command.go b/pkg/terminal/command.go
index 419ebbbb..5c780aa1 100644
--- a/pkg/terminal/command.go
+++ b/pkg/terminal/command.go
@@ -2269,7 +2269,7 @@ func printTracepoint(th *api.Thread, bpname string, fn *api.Function, args strin
 		fmt.Fprintf(os.Stderr, " => (%s)\n", strings.Join(retVals, ","))
 	}
 	if th.Breakpoint.TraceReturn || !hasReturnValue {
-		if th.BreakpointInfo.Stacktrace != nil {
+		if th.BreakpointInfo != nil && th.BreakpointInfo.Stacktrace != nil {
 			fmt.Fprintf(os.Stderr, "\tStack:\n")
 			printStack(os.Stderr, th.BreakpointInfo.Stacktrace, "\t\t", false)
 		}
diff --git a/service/api/conversions.go b/service/api/conversions.go
index 321b7a4e..c22035e1 100644
--- a/service/api/conversions.go
+++ b/service/api/conversions.go
@@ -257,7 +257,10 @@ func ConvertGoroutine(g *proc.G) *Goroutine {
 	if th != nil {
 		tid = th.ThreadID()
 	}
-	r := &Goroutine{
+	if g.Unreadable != nil {
+		return &Goroutine{Unreadable: g.Unreadable.Error()}
+	}
+	return &Goroutine{
 		ID:             g.ID,
 		CurrentLoc:     ConvertLocation(g.CurrentLoc),
 		UserCurrentLoc: ConvertLocation(g.UserCurrent()),
@@ -266,10 +269,6 @@ func ConvertGoroutine(g *proc.G) *Goroutine {
 		ThreadID:       tid,
 		Labels:         g.Labels(),
 	}
-	if g.Unreadable != nil {
-		r.Unreadable = g.Unreadable.Error()
-	}
-	return r
 }
 
 // ConvertLocation converts from proc.Location to api.Location.
diff --git a/service/dap/server.go b/service/dap/server.go
index eb140818..c2fd0f20 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -402,9 +402,16 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 			return
 		}
 
-		buildFlags, ok := request.Arguments["buildFlags"].(string)
-		if !ok {
-			buildFlags = ""
+		buildFlags := ""
+		buildFlagsArg, ok := request.Arguments["buildFlags"]
+		if ok {
+			buildFlags, ok = buildFlagsArg.(string)
+			if !ok {
+				s.sendErrorResponse(request.Request,
+					FailedToContinue, "Failed to launch",
+					fmt.Sprintf("'buildFlags' attribute '%v' in debug configuration is not a string.", buildFlagsArg))
+				return
+			}
 		}
 
 		switch mode {
diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go
index 2bdcb8ca..c83eedad 100644
--- a/service/debugger/debugger.go
+++ b/service/debugger/debugger.go
@@ -581,7 +581,7 @@ func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoin
 
 // createLogicalBreakpoint creates one physical breakpoint for each address
 // in addrs and associates all of them with the same logical breakpoint.
-func createLogicalBreakpoint(p proc.Process, addrs []uint64, requestedBp *api.Breakpoint) (*api.Breakpoint, error) {
+func createLogicalBreakpoint(p *proc.Target, addrs []uint64, requestedBp *api.Breakpoint) (*api.Breakpoint, error) {
 	bps := make([]*proc.Breakpoint, len(addrs))
 	var err error
 	for i := range addrs {


Test patch:
diff --git a/_fixtures/buildflagtest.go b/_fixtures/buildflagtest.go
new file mode 100644
index 00000000..92ae8c5e
--- /dev/null
+++ b/_fixtures/buildflagtest.go
@@ -0,0 +1,14 @@
+package main
+
+import "fmt"
+
+// To be set via
+//   go build -ldflags '-X main.Hello=World'
+var Hello string
+
+func main() {
+	if len(Hello) < 1 {
+		panic("global main.Hello not set via build flags")
+	}
+	fmt.Printf("Hello %s!\n", Hello)
+}
diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go
index ef49b582..f4725801 100644
--- a/cmd/dlv/dlv_test.go
+++ b/cmd/dlv/dlv_test.go
@@ -628,6 +628,52 @@ func TestTrace(t *testing.T) {
 	cmd.Wait()
 }
 
+func TestTracePid(t *testing.T) {
+	if runtime.GOOS == "linux" {
+		bs, _ := ioutil.ReadFile("/proc/sys/kernel/yama/ptrace_scope")
+		if bs == nil || strings.TrimSpace(string(bs)) != "0" {
+			t.Logf("can not run TestAttachDetach: %v\n", bs)
+			return
+		}
+	}
+
+	dlvbin, tmpdir := getDlvBin(t)
+	defer os.RemoveAll(tmpdir)
+
+	expected := []byte("goroutine(1): main.A() => ()\n")
+
+	// make process run
+	fix := protest.BuildFixture("issue2023", 0)
+	targetCmd := exec.Command(fix.Path)
+	if err := targetCmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	if targetCmd.Process == nil || targetCmd.Process.Pid == 0 {
+		t.Fatal("expected target process runninng")
+	}
+	defer targetCmd.Process.Kill()
+
+	// dlv attach the process by pid
+	cmd := exec.Command(dlvbin, "trace", "-p", strconv.Itoa(targetCmd.Process.Pid), "main.A")
+	rdr, err := cmd.StderrPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = cmd.Start()
+	if err != nil {
+		t.Fatalf("error running trace: %#v", err)
+	}
+	output, err := ioutil.ReadAll(rdr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Contains(output, expected) {
+		t.Fatalf("expected:\n%s\ngot:\n%s", string(expected), string(output))
+	}
+
+	cmd.Wait()
+}
+
 func TestTraceBreakpointExists(t *testing.T) {
 	dlvbin, tmpdir := getDlvBin(t)
 	defer os.RemoveAll(tmpdir)
diff --git a/pkg/proc/gdbserial/rr_test.go b/pkg/proc/gdbserial/rr_test.go
index e949b67a..eed5ec54 100644
--- a/pkg/proc/gdbserial/rr_test.go
+++ b/pkg/proc/gdbserial/rr_test.go
@@ -49,7 +49,7 @@ func assertNoError(err error, t testing.TB, s string) {
 	}
 }
 
-func setFunctionBreakpoint(p proc.Process, t *testing.T, fname string) *proc.Breakpoint {
+func setFunctionBreakpoint(p *proc.Target, t *testing.T, fname string) *proc.Breakpoint {
 	_, f, l, _ := runtime.Caller(1)
 	f = filepath.Base(f)
 
@@ -117,7 +117,7 @@ func TestRestartDuringStop(t *testing.T) {
 	})
 }
 
-func setFileBreakpoint(p proc.Process, t *testing.T, fixture protest.Fixture, lineno int) *proc.Breakpoint {
+func setFileBreakpoint(p *proc.Target, t *testing.T, fixture protest.Fixture, lineno int) *proc.Breakpoint {
 	_, f, l, _ := runtime.Caller(1)
 	f = filepath.Base(f)
 
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index d020f16e..ff86808b 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -345,7 +345,8 @@ func runDebugSession(t *testing.T, client *daptest.Client, launchRequest func())
 
 func TestLaunchDebugRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
-		// We reuse the harness that builds, but ignore the actual binary.
+		// We reuse the harness that builds, but ignore the built binary,
+		// only relying on the source to be built in response to LaunchRequest.
 		runDebugSession(t, client, func() {
 			// Use the default output directory.
 			client.LaunchRequestWithArgs(map[string]interface{}{
@@ -357,7 +358,8 @@ func TestLaunchDebugRequest(t *testing.T) {
 func TestLaunchTestRequest(t *testing.T) {
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSession(t, client, func() {
-			// We reuse the harness that builds, but ignore the actual binary.
+			// We reuse the harness that builds, but ignore the built binary,
+			// only relying on the source to be built in response to LaunchRequest.
 			fixtures := protest.FindFixturesDir()
 			testdir, _ := filepath.Abs(filepath.Join(fixtures, "buildtest"))
 			client.LaunchRequestWithArgs(map[string]interface{}{
@@ -366,6 +368,10 @@ func TestLaunchTestRequest(t *testing.T) {
 	})
 }
 
+// Tests that 'args' from LaunchRequest are parsed and passed to the target
+// program. The target program exits without an error on success, and
+// panics on error, causing an unexpected StoppedEvent instead of
+// Terminated Event.
 func TestLaunchRequestWithArgs(t *testing.T) {
 	runTest(t, "testargs", func(client *daptest.Client, fixture protest.Fixture) {
 		runDebugSession(t, client, func() {
@@ -376,6 +382,22 @@ func TestLaunchRequestWithArgs(t *testing.T) {
 	})
 }
 
+// Tests that 'buildFlags' from LaunchRequest are parsed and passed to the
+// compiler. The target program exits without an error on success, and
+// panics on error, causing an unexpected StoppedEvent instead of
+// TerminatedEvent.
+func TestLaunchRequestWithBuildFlags(t *testing.T) {
+	runTest(t, "buildflagtest", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSession(t, client, func() {
+			// We reuse the harness that builds, but ignore the built binary,
+			// only relying on the source to be built in response to LaunchRequest.
+			client.LaunchRequestWithArgs(map[string]interface{}{
+				"mode": "debug", "program": fixture.Source,
+				"buildFlags": "-ldflags '-X main.Hello=World'"})
+		})
+	})
+}
+
 func TestUnupportedCommandResponses(t *testing.T) {
 	var got *dap.ErrorResponse
 	runTest(t, "increment", func(client *daptest.Client, fixture protest.Fixture) {
@@ -586,6 +608,10 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: value '1' in 'args' attribute in debug configuration is not a string.")
 
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": 123})
+		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
+			"Failed to launch: 'buildFlags' attribute '123' in debug configuration is not a string.")
+
 		// Skip detailed message checks for potentially different OS-specific errors.
 		client.LaunchRequest("exec", fixture.Path+"_does_not_exist", stopOnEntry)
 		expectFailedToLaunch(client.ExpectErrorResponse(t))
@@ -596,6 +622,9 @@ func TestBadLaunchRequests(t *testing.T) {
 		client.LaunchRequest("exec", fixture.Source, stopOnEntry)
 		expectFailedToLaunch(client.ExpectErrorResponse(t)) // Not an executable
 
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "buildFlags": "123"})
+		expectFailedToLaunch(client.ExpectErrorResponse(t)) // Build error
+
 		// We failed to launch the program. Make sure shutdown still works.
 		client.DisconnectRequest()
 		dresp := client.ExpectDisconnectResponse(t)