From e4fc5e32c25875b9b810ce8d8e4d58400ff0de0c Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Fri, 12 Jun 2015 23:47:30 -0500 Subject: [PATCH] Refactor: Use thread-locked goroutine for ptrace ops Previously either the terminal client or the debugger service would either lock main goroutine to a thread or provide a locked goroutine to run _all_ DebuggedProcess functions in. This is unnecessary because only ptrace functions need to be run from the same thread that originated the PT_ATTACH request. Here we use a specific thread-locked goroutine to service any ptrace request. That goroutine is also responsible for the initial spawning / attaching of the process, since it must be responsible for the PT_ATTACH request. --- proc/breakpoints.go | 4 +- proc/breakpoints_darwin_amd64.go | 4 +- proc/breakpoints_linux_amd64.go | 19 +- proc/proc.go | 83 +++++- proc/proc_darwin.go | 17 +- proc/proc_linux.go | 45 +-- proc/proc_test.go | 9 +- proc/registers_linux_amd64.go | 12 +- proc/test/support.go | 2 +- proc/threads_darwin.go | 4 +- proc/threads_linux.go | 32 +- service/api/conversions.go | 77 +++++ service/debugger/debugger.go | 490 +++++++++---------------------- service/rest/integration_test.go | 22 +- service/rest/server.go | 28 +- 15 files changed, 390 insertions(+), 458 deletions(-) create mode 100644 service/api/conversions.go diff --git a/proc/breakpoints.go b/proc/breakpoints.go index 5c834024..7e7739a6 100644 --- a/proc/breakpoints.go +++ b/proc/breakpoints.go @@ -30,7 +30,7 @@ func (bp *Breakpoint) String() string { // hardware or software breakpoint. func (bp *Breakpoint) Clear(thread *Thread) (*Breakpoint, error) { if bp.hardware { - if err := clearHardwareBreakpoint(bp.reg, thread.Id); err != nil { + if err := thread.dbp.clearHardwareBreakpoint(bp.reg, thread.Id); err != nil { return nil, err } return bp, nil @@ -121,7 +121,7 @@ func (dbp *DebuggedProcess) setBreakpoint(tid int, addr uint64, temp bool) (*Bre } if v == nil { for t, _ := range dbp.Threads { - if err := setHardwareBreakpoint(i, t, addr); err != nil { + if err := dbp.setHardwareBreakpoint(i, t, addr); err != nil { return nil, fmt.Errorf("could not set hardware breakpoint on thread %d: %s", t, err) } } diff --git a/proc/breakpoints_darwin_amd64.go b/proc/breakpoints_darwin_amd64.go index 1b10e898..d0fa036d 100644 --- a/proc/breakpoints_darwin_amd64.go +++ b/proc/breakpoints_darwin_amd64.go @@ -3,11 +3,11 @@ package proc import "fmt" // TODO(darwin) -func setHardwareBreakpoint(reg, tid int, addr uint64) error { +func (dbp *DebuggedProcess) setHardwareBreakpoint(reg, tid int, addr uint64) error { return fmt.Errorf("not implemented on darwin") } // TODO(darwin) -func clearHardwareBreakpoint(reg, tid int) error { +func (dbp *DebuggedProcess) clearHardwareBreakpoint(reg, tid int) error { return fmt.Errorf("not implemented on darwin") } diff --git a/proc/breakpoints_linux_amd64.go b/proc/breakpoints_linux_amd64.go index 90c753ae..b493f05c 100644 --- a/proc/breakpoints_linux_amd64.go +++ b/proc/breakpoints_linux_amd64.go @@ -21,7 +21,7 @@ import "fmt" // debug register `reg` with the address of the instruction // that we want to break at. There are only 4 debug registers // DR0-DR3. Debug register 7 is the control register. -func setHardwareBreakpoint(reg, tid int, addr uint64) error { +func (dbp *DebuggedProcess) setHardwareBreakpoint(reg, tid int, addr uint64) error { if reg < 0 || reg > 3 { return fmt.Errorf("invalid debug register value") } @@ -32,10 +32,12 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error { drxmask = uintptr((((1 << C.DR_CONTROL_SIZE) - 1) << uintptr(reg*C.DR_CONTROL_SIZE)) | (((1 << C.DR_ENABLE_SIZE) - 1) << uintptr(reg*C.DR_ENABLE_SIZE))) drxenable = uintptr(0x1) << uintptr(reg*C.DR_ENABLE_SIZE) drxctl = uintptr(C.DR_RW_EXECUTE|C.DR_LEN_1) << uintptr(reg*C.DR_CONTROL_SIZE) + dr7 uintptr ) // Get current state - dr7, err := PtracePeekUser(tid, dr7off) + var err error + dbp.execPtraceFunc(func() { dr7, err = PtracePeekUser(tid, dr7off) }) if err != nil { return err } @@ -43,12 +45,14 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error { // If addr == 0 we are expected to disable the breakpoint if addr == 0 { dr7 &= ^drxmask - return PtracePokeUser(tid, dr7off, dr7) + dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, dr7off, dr7) }) + return err } // Set the debug register `reg` with the address of the // instruction we want to trigger a debug exception. - if err := PtracePokeUser(tid, drxoff, uintptr(addr)); err != nil { + dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, drxoff, uintptr(addr)) }) + if err != nil { return err } @@ -61,12 +65,13 @@ func setHardwareBreakpoint(reg, tid int, addr uint64) error { // instructs the cpu to raise a debug // exception when hitting the address of // an instruction stored in dr0-dr3. - return PtracePokeUser(tid, dr7off, dr7) + dbp.execPtraceFunc(func() { err = PtracePokeUser(tid, dr7off, dr7) }) + return err } // Clears a hardware breakpoint. Essentially sets // the debug reg to 0 and clears the control register // flags for that reg. -func clearHardwareBreakpoint(reg, tid int) error { - return setHardwareBreakpoint(reg, tid, 0) +func (dbp *DebuggedProcess) clearHardwareBreakpoint(reg, tid int) error { + return dbp.setHardwareBreakpoint(reg, tid, 0) } diff --git a/proc/proc.go b/proc/proc.go index f95e268a..53516716 100644 --- a/proc/proc.go +++ b/proc/proc.go @@ -49,6 +49,23 @@ type DebuggedProcess struct { running bool halt bool exited bool + ptraceChan chan func() + ptraceDoneChan chan interface{} +} + +func New(pid int) *DebuggedProcess { + dbp := &DebuggedProcess{ + Pid: pid, + Threads: make(map[int]*Thread), + Breakpoints: make(map[uint64]*Breakpoint), + firstStart: true, + os: new(OSProcessDetails), + ast: source.New(), + ptraceChan: make(chan func()), + ptraceDoneChan: make(chan interface{}), + } + go dbp.handlePtraceFuncs() + return dbp } // A ManualStopError happens when the user triggers a @@ -72,22 +89,33 @@ func (pe ProcessExitedError) Error() string { // Attach to an existing process with the given PID. func Attach(pid int) (*DebuggedProcess, error) { - dbp := &DebuggedProcess{ - Pid: pid, - Threads: make(map[int]*Thread), - Breakpoints: make(map[uint64]*Breakpoint), - os: new(OSProcessDetails), - ast: source.New(), - } - dbp, err := initializeDebugProcess(dbp, "", true) + dbp, err := initializeDebugProcess(New(pid), "", true) if err != nil { return nil, err } return dbp, nil } -func (dbp *DebuggedProcess) Detach() error { - return PtraceDetach(dbp.Pid, int(sys.SIGINT)) +func (dbp *DebuggedProcess) Detach(kill bool) (err error) { + // Clean up any breakpoints we've set. + for _, bp := range dbp.arch.HardwareBreakpoints() { + if bp != nil { + dbp.Clear(bp.Addr) + } + } + for _, bp := range dbp.Breakpoints { + if bp != nil { + dbp.Clear(bp.Addr) + } + } + dbp.execPtraceFunc(func() { + var sig int + if kill { + sig = int(sys.SIGINT) + } + err = PtraceDetach(dbp.Pid, sig) + }) + return } // Returns whether or not Delve thinks the debugged @@ -497,6 +525,21 @@ func (dbp *DebuggedProcess) PCToLine(pc uint64) (string, int, *gosym.Func) { return dbp.goSymTable.PCToLine(pc) } +// Finds the breakpoint for the given ID. +func (dbp *DebuggedProcess) FindBreakpointByID(id int) (*Breakpoint, bool) { + for _, bp := range dbp.arch.HardwareBreakpoints() { + if bp != nil && bp.ID == id { + return bp, true + } + } + for _, bp := range dbp.Breakpoints { + if bp.ID == id { + return bp, true + } + } + return nil, false +} + // Finds the breakpoint for the given pc. func (dbp *DebuggedProcess) FindBreakpoint(pc uint64) (*Breakpoint, bool) { for _, bp := range dbp.arch.HardwareBreakpoints() { @@ -513,7 +556,8 @@ func (dbp *DebuggedProcess) FindBreakpoint(pc uint64) (*Breakpoint, bool) { // Returns a new DebuggedProcess struct. func initializeDebugProcess(dbp *DebuggedProcess, path string, attach bool) (*DebuggedProcess, error) { if attach { - err := sys.PtraceAttach(dbp.Pid) + var err error + dbp.execPtraceFunc(func() { err = sys.PtraceAttach(dbp.Pid) }) if err != nil { return nil, err } @@ -623,3 +667,20 @@ func (dbp *DebuggedProcess) run(fn func() error) error { } return nil } + +func (dbp *DebuggedProcess) handlePtraceFuncs() { + // We must ensure here that we are running on the same thread during + // the execution of dbg. This is due to the fact that ptrace(2) expects + // all commands after PTRACE_ATTACH to come from the same thread. + runtime.LockOSThread() + + for fn := range dbp.ptraceChan { + fn() + dbp.ptraceDoneChan <- nil + } +} + +func (dbp *DebuggedProcess) execPtraceFunc(fn func()) { + dbp.ptraceChan <- fn + <-dbp.ptraceDoneChan +} diff --git a/proc/proc_darwin.go b/proc/proc_darwin.go index c30a4106..10df3184 100644 --- a/proc/proc_darwin.go +++ b/proc/proc_darwin.go @@ -15,7 +15,6 @@ import ( "github.com/derekparker/delve/dwarf/frame" "github.com/derekparker/delve/dwarf/line" - "github.com/derekparker/delve/source" sys "golang.org/x/sys/unix" ) @@ -44,16 +43,12 @@ func Launch(cmd []string) (*DebuggedProcess, error) { var argv **C.char argv = &argvSlice[0] - dbp := &DebuggedProcess{ - Threads: make(map[int]*Thread), - Breakpoints: make(map[uint64]*Breakpoint), - firstStart: true, - os: new(OSProcessDetails), - ast: source.New(), - } - - ret := C.fork_exec(argv0, argv, &dbp.os.task, &dbp.os.portSet, &dbp.os.exceptionPort, &dbp.os.notificationPort) - pid := int(ret) + dbp := New(0) + var pid int + dbp.execPtraceFunc(func() { + ret := C.fork_exec(argv0, argv, &dbp.os.task, &dbp.os.portSet, &dbp.os.exceptionPort, &dbp.os.notificationPort) + pid = int(ret) + }) if pid <= 0 { return nil, fmt.Errorf("could not fork/exec") } diff --git a/proc/proc_linux.go b/proc/proc_linux.go index d2471d19..184d57ca 100644 --- a/proc/proc_linux.go +++ b/proc/proc_linux.go @@ -15,7 +15,6 @@ import ( "github.com/derekparker/delve/dwarf/frame" "github.com/derekparker/delve/dwarf/line" - "github.com/derekparker/delve/source" ) const ( @@ -31,27 +30,27 @@ type OSProcessDetails interface{} // `cmd` is the program to run, and then rest are the arguments // to be supplied to that process. func Launch(cmd []string) (*DebuggedProcess, error) { - proc := exec.Command(cmd[0]) - proc.Args = cmd - proc.Stdout = os.Stdout - proc.Stderr = os.Stderr - proc.SysProcAttr = &syscall.SysProcAttr{Ptrace: true} - - if err := proc.Start(); err != nil { + var ( + proc *exec.Cmd + err error + ) + dbp := New(0) + dbp.execPtraceFunc(func() { + proc = exec.Command(cmd[0]) + proc.Args = cmd + proc.Stdout = os.Stdout + proc.Stderr = os.Stderr + proc.SysProcAttr = &syscall.SysProcAttr{Ptrace: true} + err = proc.Start() + }) + if err != nil { return nil, err } - _, _, err := wait(proc.Process.Pid, 0) + dbp.Pid = proc.Process.Pid + _, _, err = wait(proc.Process.Pid, 0) if err != nil { return nil, fmt.Errorf("waiting for target execve failed: %s", err) } - dbp := &DebuggedProcess{ - Pid: proc.Process.Pid, - Threads: make(map[int]*Thread), - Breakpoints: make(map[uint64]*Breakpoint), - os: new(OSProcessDetails), - ast: source.New(), - } - return initializeDebugProcess(dbp, proc.Path, false) } @@ -66,8 +65,9 @@ func (dbp *DebuggedProcess) addThread(tid int, attach bool) (*Thread, error) { return thread, nil } + var err error if attach { - err := sys.PtraceAttach(tid) + dbp.execPtraceFunc(func() { err = sys.PtraceAttach(tid) }) if err != nil && err != sys.EPERM { // Do not return err if err == EPERM, // we may already be tracing this thread due to @@ -86,14 +86,14 @@ func (dbp *DebuggedProcess) addThread(tid int, attach bool) (*Thread, error) { } } - err := syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) + dbp.execPtraceFunc(func() { err = syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) }) if err == syscall.ESRCH { _, _, err = wait(tid, 0) if err != nil { return nil, fmt.Errorf("error while waiting after adding thread: %d %s", tid, err) } - err := syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) + dbp.execPtraceFunc(func() { err = syscall.PtraceSetOptions(tid, syscall.PTRACE_O_TRACECLONE) }) if err != nil { return nil, fmt.Errorf("could not set options for new traced thread %d %s", tid, err) } @@ -244,7 +244,8 @@ func (dbp *DebuggedProcess) trapWait(pid int) (*Thread, error) { 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. - cloned, err := sys.PtraceGetEventMsg(wpid) + var cloned uint + dbp.execPtraceFunc(func() { cloned, err = sys.PtraceGetEventMsg(wpid) }) if err != nil { return nil, fmt.Errorf("could not get event message: %s", err) } @@ -256,7 +257,7 @@ func (dbp *DebuggedProcess) trapWait(pid int) (*Thread, error) { if bp == nil { continue } - if err = setHardwareBreakpoint(reg, th.Id, bp.Addr); err != nil { + if err = dbp.setHardwareBreakpoint(reg, th.Id, bp.Addr); err != nil { return nil, err } } diff --git a/proc/proc_test.go b/proc/proc_test.go index c9f62dfa..d45bf194 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -10,20 +10,22 @@ import ( protest "github.com/derekparker/delve/proc/test" ) +func init() { + runtime.GOMAXPROCS(2) +} + func TestMain(m *testing.M) { protest.RunTestsWithFixtures(m) } func withTestProcess(name string, t *testing.T, fn func(p *DebuggedProcess, fixture protest.Fixture)) { - runtime.LockOSThread() - fixture := protest.BuildFixture(name) p, err := Launch([]string{fixture.Path}) if err != nil { t.Fatal("Launch():", err) } - defer p.Detach() + defer p.Detach(true) fn(p, fixture) } @@ -88,6 +90,7 @@ func TestExit(t *testing.T) { } func TestHalt(t *testing.T) { + runtime.GOMAXPROCS(2) withTestProcess("testprog", t, func(p *DebuggedProcess, fixture protest.Fixture) { go func() { for { diff --git a/proc/registers_linux_amd64.go b/proc/registers_linux_amd64.go index 054e69d1..d217f3cb 100644 --- a/proc/registers_linux_amd64.go +++ b/proc/registers_linux_amd64.go @@ -18,14 +18,18 @@ func (r *Regs) CX() uint64 { return r.regs.Rcx } -func (r *Regs) SetPC(thread *Thread, pc uint64) error { +func (r *Regs) SetPC(thread *Thread, pc uint64) (err error) { r.regs.SetPC(pc) - return sys.PtraceSetRegs(thread.Id, r.regs) + thread.dbp.execPtraceFunc(func() { err = sys.PtraceSetRegs(thread.Id, r.regs) }) + return } func registers(thread *Thread) (Registers, error) { - var regs sys.PtraceRegs - err := sys.PtraceGetRegs(thread.Id, ®s) + var ( + regs sys.PtraceRegs + err error + ) + thread.dbp.execPtraceFunc(func() { err = sys.PtraceGetRegs(thread.Id, ®s) }) if err != nil { return nil, err } diff --git a/proc/test/support.go b/proc/test/support.go index b179bc09..e3c184e6 100644 --- a/proc/test/support.go +++ b/proc/test/support.go @@ -28,7 +28,7 @@ func BuildFixture(name string) Fixture { return f } parent := ".." - fixturesDir := filepath.Join(parent, "_fixtures") + fixturesDir := "_fixtures" for depth := 0; depth < 10; depth++ { if _, err := os.Stat(fixturesDir); err == nil { break diff --git a/proc/threads_darwin.go b/proc/threads_darwin.go index f31fcf8c..b26dab5f 100644 --- a/proc/threads_darwin.go +++ b/proc/threads_darwin.go @@ -36,7 +36,9 @@ func (t *Thread) singleStep() error { func (t *Thread) resume() error { // TODO(dp) set flag for ptrace stops - if PtraceCont(t.dbp.Pid, 0) == nil { + var err error + t.dbp.execPtraceFunc(func() { err = PtraceCont(t.dbp.Pid, 0) }) + if err == nil { return nil } kret := C.resume_thread(t.os.thread_act) diff --git a/proc/threads_linux.go b/proc/threads_linux.go index d22c8361..28572b07 100644 --- a/proc/threads_linux.go +++ b/proc/threads_linux.go @@ -27,12 +27,13 @@ func (t *Thread) Halt() error { return nil } -func (t *Thread) resume() error { - return PtraceCont(t.Id, 0) +func (t *Thread) resume() (err error) { + t.dbp.execPtraceFunc(func() { err = PtraceCont(t.Id, 0) }) + return } -func (t *Thread) singleStep() error { - err := sys.PtraceSingleStep(t.Id) +func (t *Thread) singleStep() (err error) { + t.dbp.execPtraceFunc(func() { err = sys.PtraceSingleStep(t.Id) }) if err != nil { return err } @@ -51,26 +52,31 @@ func (t *Thread) blocked() bool { } func (thread *Thread) saveRegisters() (Registers, error) { - if err := sys.PtraceGetRegs(thread.Id, &thread.os.registers); err != nil { + var err error + thread.dbp.execPtraceFunc(func() { err = sys.PtraceGetRegs(thread.Id, &thread.os.registers) }) + if err != nil { return nil, fmt.Errorf("could not save register contents") } return &Regs{&thread.os.registers}, nil } -func (thread *Thread) restoreRegisters() error { - return sys.PtraceSetRegs(thread.Id, &thread.os.registers) +func (thread *Thread) restoreRegisters() (err error) { + thread.dbp.execPtraceFunc(func() { err = sys.PtraceSetRegs(thread.Id, &thread.os.registers) }) + return } -func writeMemory(thread *Thread, addr uintptr, data []byte) (int, error) { +func writeMemory(thread *Thread, addr uintptr, data []byte) (written int, err error) { if len(data) == 0 { - return 0, nil + return } - return sys.PtracePokeData(thread.Id, addr, data) + thread.dbp.execPtraceFunc(func() { written, err = sys.PtracePokeData(thread.Id, addr, data) }) + return } -func readMemory(thread *Thread, addr uintptr, data []byte) (int, error) { +func readMemory(thread *Thread, addr uintptr, data []byte) (read int, err error) { if len(data) == 0 { - return 0, nil + return } - return sys.PtracePeekData(thread.Id, addr, data) + thread.dbp.execPtraceFunc(func() { read, err = sys.PtracePeekData(thread.Id, addr, data) }) + return } diff --git a/service/api/conversions.go b/service/api/conversions.go new file mode 100644 index 00000000..e326c09a --- /dev/null +++ b/service/api/conversions.go @@ -0,0 +1,77 @@ +package api + +import "github.com/derekparker/delve/proc" + +// convertBreakpoint converts an internal breakpoint to an API Breakpoint. +func ConvertBreakpoint(bp *proc.Breakpoint) *Breakpoint { + return &Breakpoint{ + ID: bp.ID, + FunctionName: bp.FunctionName, + File: bp.File, + Line: bp.Line, + Addr: bp.Addr, + } +} + +// convertThread converts an internal thread to an API Thread. +func ConvertThread(th *proc.Thread) *Thread { + var ( + function *Function + file string + line int + pc uint64 + ) + + loc, err := th.Location() + if err == nil { + pc = loc.PC + file = loc.File + line = loc.Line + if loc.Fn != nil { + function = &Function{ + Name: loc.Fn.Name, + Type: loc.Fn.Type, + Value: loc.Fn.Value, + GoType: loc.Fn.GoType, + } + } + } + + return &Thread{ + ID: th.Id, + PC: pc, + File: file, + Line: line, + Function: function, + } +} + +// convertVar converts an internal variable to an API Variable. +func ConvertVar(v *proc.Variable) Variable { + return Variable{ + Name: v.Name, + Value: v.Value, + Type: v.Type, + } +} + +// convertGoroutine converts an internal Goroutine to an API Goroutine. +func ConvertGoroutine(g *proc.G) *Goroutine { + var function *Function + if g.Func != nil { + function = &Function{ + Name: g.Func.Name, + Type: g.Func.Type, + Value: g.Func.Value, + GoType: g.Func.GoType, + } + } + + return &Goroutine{ + ID: g.Id, + PC: g.PC, + File: g.File, + Line: g.Line, + Function: function, + } +} diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 62b6ab2a..5ec333f9 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -4,20 +4,16 @@ import ( "fmt" "log" "regexp" - "runtime" "github.com/derekparker/delve/proc" "github.com/derekparker/delve/service/api" ) -// Debugger provides a thread-safe DebuggedProcess service. Instances of -// Debugger can be exposed by other services. +// Debugger provides a conveniant way to convert from internal +// structure representations to API representations. type Debugger struct { - config *Config - process *proc.DebuggedProcess - processOps chan func(*proc.DebuggedProcess) - stop chan stopSignal - running bool + config *Config + process *proc.DebuggedProcess } // Config provides the configuration to start a Debugger. @@ -33,222 +29,107 @@ type Config struct { AttachPid int } -// stopSignal is used to stop the debugger. -type stopSignal struct { - // KillProcess indicates whether to kill the debugee following detachment. - KillProcess bool -} - // New creates a new Debugger. -func New(config *Config) *Debugger { - debugger := &Debugger{ - processOps: make(chan func(*proc.DebuggedProcess)), - config: config, - stop: make(chan stopSignal), - } - return debugger -} - -// withProcess facilitates thread-safe access to the DebuggedProcess. Most -// interaction with DebuggedProcess should occur via calls to withProcess[1], -// and the functions placed on the processOps channel should be consumed and -// executed from the same thread as the DebuggedProcess. -// -// This is convenient because it allows things like HTTP handlers in -// goroutines to work with the DebuggedProcess with synchronous semantics. -// -// [1] There are some exceptional cases where direct access is okay; for -// instance, when performing an operation like halt which merely sends a -// signal to the process rather than performing something like a ptrace -// operation. -func (d *Debugger) withProcess(f func(*proc.DebuggedProcess) error) error { - if !d.running { - return fmt.Errorf("debugger isn't running") +func New(config *Config) (*Debugger, error) { + d := &Debugger{ + config: config, } - result := make(chan error) - d.processOps <- func(proc *proc.DebuggedProcess) { - result <- f(proc) - } - return <-result -} - -// Run starts debugging a process until Detach is called. -func (d *Debugger) Run() error { - // We must ensure here that we are running on the same thread during - // the execution of dbg. This is due to the fact that ptrace(2) expects - // all commands after PTRACE_ATTACH to come from the same thread. - runtime.LockOSThread() - defer runtime.UnlockOSThread() - - d.running = true - defer func() { d.running = false }() - // Create the process by either attaching or launching. if d.config.AttachPid > 0 { log.Printf("attaching to pid %d", d.config.AttachPid) p, err := proc.Attach(d.config.AttachPid) if err != nil { - return fmt.Errorf("couldn't attach to pid %d: %s", d.config.AttachPid, err) + return nil, fmt.Errorf("couldn't attach to pid %d: %s", d.config.AttachPid, err) } d.process = p } else { log.Printf("launching process with args: %v", d.config.ProcessArgs) p, err := proc.Launch(d.config.ProcessArgs) if err != nil { - return fmt.Errorf("couldn't launch process: %s", err) + return nil, fmt.Errorf("couldn't launch process: %s", err) } d.process = p } - - // Handle access to the process from the current thread. - log.Print("debugger started") - for { - select { - case f := <-d.processOps: - // Execute the function - f(d.process) - case s := <-d.stop: - // Handle shutdown - log.Print("debugger is stopping") - - // Clear breakpoints - bps := []*proc.Breakpoint{} - for _, bp := range d.process.Breakpoints { - if bp != nil { - bps = append(bps, bp) - } - } - for _, bp := range d.process.HardwareBreakpoints() { - if bp != nil { - bps = append(bps, bp) - } - } - for _, bp := range bps { - _, err := d.process.Clear(bp.Addr) - if err != nil { - log.Printf("warning: couldn't clear breakpoint @ %#v: %s", bp.Addr, err) - } else { - log.Printf("cleared breakpoint @ %#v", bp.Addr) - } - } - - // Kill the process if requested - if s.KillProcess { - if err := d.process.Detach(); err == nil { - log.Print("killed process") - } else { - log.Printf("couldn't kill process: %s", err) - } - } else { - // Detach - if !d.process.Exited() { - if err := proc.PtraceDetach(d.process.Pid, 0); err == nil { - log.Print("detached from process") - } else { - log.Printf("couldn't detach from process: %s", err) - } - } - } - - return nil - } - } + return d, nil } -// Detach stops the debugger. func (d *Debugger) Detach(kill bool) error { - if !d.running { - return fmt.Errorf("debugger isn't running") - } - - d.stop <- stopSignal{KillProcess: kill} - return nil + return d.process.Detach(kill) } func (d *Debugger) State() (*api.DebuggerState, error) { - var state *api.DebuggerState - - err := d.withProcess(func(p *proc.DebuggedProcess) error { - var thread *api.Thread - th := p.CurrentThread - if th != nil { - thread = convertThread(th) - } + var ( + state *api.DebuggerState + thread *api.Thread + ) + th := d.process.CurrentThread + if th != nil { + thread = api.ConvertThread(th) + } - var breakpoint *api.Breakpoint - bp := p.CurrentBreakpoint() - if bp != nil { - breakpoint = convertBreakpoint(bp) - } + var breakpoint *api.Breakpoint + bp := d.process.CurrentBreakpoint() + if bp != nil { + breakpoint = api.ConvertBreakpoint(bp) + } - state = &api.DebuggerState{ - Breakpoint: breakpoint, - CurrentThread: thread, - Exited: p.Exited(), - } - return nil - }) + state = &api.DebuggerState{ + Breakpoint: breakpoint, + CurrentThread: thread, + Exited: d.process.Exited(), + } - return state, err + return state, nil } func (d *Debugger) CreateBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) { var createdBp *api.Breakpoint - err := d.withProcess(func(p *proc.DebuggedProcess) error { - var loc string - switch { - case len(requestedBp.File) > 0: - loc = fmt.Sprintf("%s:%d", requestedBp.File, requestedBp.Line) - case len(requestedBp.FunctionName) > 0: - loc = requestedBp.FunctionName - default: - return fmt.Errorf("no file or function name specified") - } + var loc string + switch { + case len(requestedBp.File) > 0: + loc = fmt.Sprintf("%s:%d", requestedBp.File, requestedBp.Line) + case len(requestedBp.FunctionName) > 0: + loc = requestedBp.FunctionName + default: + return nil, fmt.Errorf("no file or function name specified") + } - bp, breakError := p.BreakByLocation(loc) - if breakError != nil { - return breakError - } - createdBp = convertBreakpoint(bp) - log.Printf("created breakpoint: %#v", createdBp) - return nil - }) - return createdBp, err + bp, err := d.process.BreakByLocation(loc) + if err != nil { + return nil, err + } + createdBp = api.ConvertBreakpoint(bp) + log.Printf("created breakpoint: %#v", createdBp) + return createdBp, nil } func (d *Debugger) ClearBreakpoint(requestedBp *api.Breakpoint) (*api.Breakpoint, error) { var clearedBp *api.Breakpoint - err := d.withProcess(func(p *proc.DebuggedProcess) error { - bp, err := p.Clear(requestedBp.Addr) - if err != nil { - return fmt.Errorf("Can't clear breakpoint @%x: %s", requestedBp.Addr, err) - } - clearedBp = convertBreakpoint(bp) - log.Printf("cleared breakpoint: %#v", clearedBp) - return nil - }) + bp, err := d.process.Clear(requestedBp.Addr) + if err != nil { + return nil, fmt.Errorf("Can't clear breakpoint @%x: %s", requestedBp.Addr, err) + } + clearedBp = api.ConvertBreakpoint(bp) + log.Printf("cleared breakpoint: %#v", clearedBp) return clearedBp, err } func (d *Debugger) Breakpoints() []*api.Breakpoint { bps := []*api.Breakpoint{} - d.withProcess(func(p *proc.DebuggedProcess) error { - for _, bp := range p.HardwareBreakpoints() { - if bp == nil { - continue - } - bps = append(bps, convertBreakpoint(bp)) + for _, bp := range d.process.HardwareBreakpoints() { + if bp == nil { + continue } + bps = append(bps, api.ConvertBreakpoint(bp)) + } - for _, bp := range p.Breakpoints { - if bp.Temp { - continue - } - bps = append(bps, convertBreakpoint(bp)) + for _, bp := range d.process.Breakpoints { + if bp.Temp { + continue } - return nil - }) + bps = append(bps, api.ConvertBreakpoint(bp)) + } return bps } @@ -263,12 +144,9 @@ func (d *Debugger) FindBreakpoint(id int) *api.Breakpoint { func (d *Debugger) Threads() []*api.Thread { threads := []*api.Thread{} - d.withProcess(func(p *proc.DebuggedProcess) error { - for _, th := range p.Threads { - threads = append(threads, convertThread(th)) - } - return nil - }) + for _, th := range d.process.Threads { + threads = append(threads, api.ConvertThread(th)) + } return threads } @@ -291,26 +169,17 @@ func (d *Debugger) Command(command *api.DebuggerCommand) (*api.DebuggerState, er var err error switch command.Name { case api.Continue: - err = d.withProcess(func(p *proc.DebuggedProcess) error { - log.Print("continuing") - e := p.Continue() - return e - }) + log.Print("continuing") + err = d.process.Continue() case api.Next: - err = d.withProcess(func(p *proc.DebuggedProcess) error { - log.Print("nexting") - return p.Next() - }) + log.Print("nexting") + err = d.process.Next() case api.Step: - err = d.withProcess(func(p *proc.DebuggedProcess) error { - log.Print("stepping") - return p.Step() - }) + log.Print("stepping") + err = d.process.Step() case api.SwitchThread: - err = d.withProcess(func(p *proc.DebuggedProcess) error { - log.Printf("switching to thread %d", command.ThreadID) - return p.SwitchThread(command.ThreadID) - }) + log.Printf("switching to thread %d", command.ThreadID) + err = d.process.SwitchThread(command.ThreadID) case api.Halt: // RequestManualStop does not invoke any ptrace syscalls, so it's safe to // access the process directly. @@ -333,14 +202,11 @@ func (d *Debugger) Sources(filter string) ([]string, error) { } files := []string{} - d.withProcess(func(p *proc.DebuggedProcess) error { - for f := range p.Sources() { - if regex.Match([]byte(f)) { - files = append(files, f) - } + for f := range d.process.Sources() { + if regex.Match([]byte(f)) { + files = append(files, f) } - return nil - }) + } return files, nil } @@ -351,14 +217,11 @@ func (d *Debugger) Functions(filter string) ([]string, error) { } funcs := []string{} - d.withProcess(func(p *proc.DebuggedProcess) error { - for _, f := range p.Funcs() { - if f.Sym != nil && regex.Match([]byte(f.Name)) { - funcs = append(funcs, f.Name) - } + for _, f := range d.process.Funcs() { + if f.Sym != nil && regex.Match([]byte(f.Name)) { + funcs = append(funcs, f.Name) } - return nil - }) + } return funcs, nil } @@ -369,166 +232,75 @@ func (d *Debugger) PackageVariables(threadID int, filter string) ([]api.Variable } vars := []api.Variable{} - err = d.withProcess(func(p *proc.DebuggedProcess) error { - thread, found := p.Threads[threadID] - if !found { - return fmt.Errorf("couldn't find thread %d", threadID) - } - pv, err := thread.PackageVariables() - if err != nil { - return err - } - for _, v := range pv { - if regex.Match([]byte(v.Name)) { - vars = append(vars, convertVar(v)) - } + thread, found := d.process.Threads[threadID] + if !found { + return nil, fmt.Errorf("couldn't find thread %d", threadID) + } + pv, err := thread.PackageVariables() + if err != nil { + return nil, err + } + for _, v := range pv { + if regex.Match([]byte(v.Name)) { + vars = append(vars, api.ConvertVar(v)) } - return nil - }) + } return vars, err } func (d *Debugger) LocalVariables(threadID int) ([]api.Variable, error) { vars := []api.Variable{} - err := d.withProcess(func(p *proc.DebuggedProcess) error { - thread, found := p.Threads[threadID] - if !found { - return fmt.Errorf("couldn't find thread %d", threadID) - } - pv, err := thread.LocalVariables() - if err != nil { - return err - } - for _, v := range pv { - vars = append(vars, convertVar(v)) - } - return nil - }) + thread, found := d.process.Threads[threadID] + if !found { + return nil, fmt.Errorf("couldn't find thread %d", threadID) + } + pv, err := thread.LocalVariables() + if err != nil { + return nil, err + } + for _, v := range pv { + vars = append(vars, api.ConvertVar(v)) + } return vars, err } func (d *Debugger) FunctionArguments(threadID int) ([]api.Variable, error) { vars := []api.Variable{} - err := d.withProcess(func(p *proc.DebuggedProcess) error { - thread, found := p.Threads[threadID] - if !found { - return fmt.Errorf("couldn't find thread %d", threadID) - } - pv, err := thread.FunctionArguments() - if err != nil { - return err - } - for _, v := range pv { - vars = append(vars, convertVar(v)) - } - return nil - }) - return vars, err -} - -func (d *Debugger) EvalVariableInThread(threadID int, symbol string) (*api.Variable, error) { - var variable *api.Variable - err := d.withProcess(func(p *proc.DebuggedProcess) error { - thread, found := p.Threads[threadID] - if !found { - return fmt.Errorf("couldn't find thread %d", threadID) - } - v, err := thread.EvalVariable(symbol) - if err != nil { - return err - } - converted := convertVar(v) - variable = &converted - return nil - }) - return variable, err -} - -func (d *Debugger) Goroutines() ([]*api.Goroutine, error) { - goroutines := []*api.Goroutine{} - err := d.withProcess(func(p *proc.DebuggedProcess) error { - gs, err := p.GoroutinesInfo() - if err != nil { - return err - } - for _, g := range gs { - goroutines = append(goroutines, convertGoroutine(g)) - } - return nil - }) - return goroutines, err -} - -// convertBreakpoint converts an internal breakpoint to an API Breakpoint. -func convertBreakpoint(bp *proc.Breakpoint) *api.Breakpoint { - return &api.Breakpoint{ - ID: bp.ID, - FunctionName: bp.FunctionName, - File: bp.File, - Line: bp.Line, - Addr: bp.Addr, + thread, found := d.process.Threads[threadID] + if !found { + return nil, fmt.Errorf("couldn't find thread %d", threadID) } -} - -// convertThread converts an internal thread to an API Thread. -func convertThread(th *proc.Thread) *api.Thread { - var ( - function *api.Function - file string - line int - pc uint64 - ) - - loc, err := th.Location() - if err == nil { - pc = loc.PC - file = loc.File - line = loc.Line - if loc.Fn != nil { - function = &api.Function{ - Name: loc.Fn.Name, - Type: loc.Fn.Type, - Value: loc.Fn.Value, - GoType: loc.Fn.GoType, - } - } + pv, err := thread.FunctionArguments() + if err != nil { + return nil, err } - - return &api.Thread{ - ID: th.Id, - PC: pc, - File: file, - Line: line, - Function: function, + for _, v := range pv { + vars = append(vars, api.ConvertVar(v)) } + return vars, err } -// convertVar converts an internal variable to an API Variable. -func convertVar(v *proc.Variable) api.Variable { - return api.Variable{ - Name: v.Name, - Value: v.Value, - Type: v.Type, +func (d *Debugger) EvalVariableInThread(threadID int, symbol string) (*api.Variable, error) { + thread, found := d.process.Threads[threadID] + if !found { + return nil, fmt.Errorf("couldn't find thread %d", threadID) + } + v, err := thread.EvalVariable(symbol) + if err != nil { + return nil, err } + converted := api.ConvertVar(v) + return &converted, err } -// convertGoroutine converts an internal Goroutine to an API Goroutine. -func convertGoroutine(g *proc.G) *api.Goroutine { - var function *api.Function - if g.Func != nil { - function = &api.Function{ - Name: g.Func.Name, - Type: g.Func.Type, - Value: g.Func.Value, - GoType: g.Func.GoType, - } +func (d *Debugger) Goroutines() ([]*api.Goroutine, error) { + goroutines := []*api.Goroutine{} + gs, err := d.process.GoroutinesInfo() + if err != nil { + return nil, err } - - return &api.Goroutine{ - ID: g.Id, - PC: g.PC, - File: g.File, - Line: g.Line, - Function: function, + for _, g := range gs { + goroutines = append(goroutines, api.ConvertGoroutine(g)) } + return goroutines, err } diff --git a/service/rest/integration_test.go b/service/rest/integration_test.go index 6333ab43..24ab9a6f 100644 --- a/service/rest/integration_test.go +++ b/service/rest/integration_test.go @@ -2,7 +2,9 @@ package rest import ( "net" + "os" "path/filepath" + "runtime" "testing" protest "github.com/derekparker/delve/proc/test" @@ -10,6 +12,10 @@ import ( "github.com/derekparker/delve/service/api" ) +func init() { + runtime.GOMAXPROCS(2) +} + func TestMain(m *testing.M) { protest.RunTestsWithFixtures(m) } @@ -272,10 +278,16 @@ func TestClientServer_switchThread(t *testing.T) { func TestClientServer_infoLocals(t *testing.T) { withTestClient("testnextprog", t, func(c service.Client) { - fp, err := filepath.Abs("../../_fixtures/testnextprog.go") + fp, err := filepath.Abs("_fixtures/testnextprog.go") if err != nil { t.Fatal(err) } + if _, err := os.Stat(fp); err != nil { + fp, err = filepath.Abs("../../_fixtures/testnextprog.go") + if err != nil { + t.Fatal(err) + } + } _, err = c.CreateBreakpoint(&api.Breakpoint{File: fp, Line: 23}) if err != nil { t.Fatalf("Unexpected error: %v", err) @@ -296,10 +308,16 @@ func TestClientServer_infoLocals(t *testing.T) { func TestClientServer_infoArgs(t *testing.T) { withTestClient("testnextprog", t, func(c service.Client) { - fp, err := filepath.Abs("../../_fixtures/testnextprog.go") + fp, err := filepath.Abs("_fixtures/testnextprog.go") if err != nil { t.Fatal(err) } + if _, err := os.Stat(fp); err != nil { + fp, err = filepath.Abs("../../_fixtures/testnextprog.go") + if err != nil { + t.Fatal(err) + } + } _, err = c.CreateBreakpoint(&api.Breakpoint{File: fp, Line: 47}) if err != nil { t.Fatalf("Unexpected error: %v", err) diff --git a/service/rest/server.go b/service/rest/server.go index f1c11e31..ed61f109 100644 --- a/service/rest/server.go +++ b/service/rest/server.go @@ -21,8 +21,6 @@ type RESTServer struct { listener net.Listener // debugger is a debugger service. debugger *debugger.Debugger - // debuggerStopped is used to detect shutdown of the debugger service. - debuggerStopped chan error } // Config provides the configuration to start a Debugger and expose it with a @@ -49,9 +47,8 @@ func NewServer(config *Config, logEnabled bool) *RESTServer { } return &RESTServer{ - config: config, - listener: config.Listener, - debuggerStopped: make(chan error), + config: config, + listener: config.Listener, } } @@ -59,22 +56,17 @@ func NewServer(config *Config, logEnabled bool) *RESTServer { // itself can be stopped with the `detach` API. Run blocks until the HTTP // server stops. func (s *RESTServer) Run() error { + var err error // Create and start the debugger - s.debugger = debugger.New(&debugger.Config{ + if s.debugger, err = debugger.New(&debugger.Config{ ProcessArgs: s.config.ProcessArgs, AttachPid: s.config.AttachPid, - }) - go func() { - err := s.debugger.Run() - if err != nil { - log.Printf("debugger stopped with error: %s", err) - } - s.debuggerStopped <- err - }() + }); err != nil { + return err + } // Set up the HTTP server container := restful.NewContainer() - ws := new(restful.WebService) ws. Path(""). @@ -108,11 +100,7 @@ func (s *RESTServer) Run() error { // Stop detaches from the debugger and waits for it to stop. func (s *RESTServer) Stop(kill bool) error { - err := s.debugger.Detach(kill) - if err != nil { - return err - } - return <-s.debuggerStopped + return s.debugger.Detach(kill) } // writeError writes a simple error response. -- GitLab