From f6e8fb37a40312a363baec4cdde5a1ee66debf59 Mon Sep 17 00:00:00 2001 From: Alessandro Arzilli Date: Sat, 22 Oct 2016 06:51:34 +0200 Subject: [PATCH] proc: changed windows backend to deal with simultaneous breakpoints (#598) * proc: changed windows backend to deal with simultaneous breakpoints * bugfix: forgot to add windowsPrologue3 to the prologues list in e4c7df1 * Tolerate errors returned by Stacktrace in TestStacktraceGoroutine. * bugfix: proc: propagate debug events we don't cause back to the target process Fixes: #594 * proc: fixed TestStepConcurrentPtr Implementation of nextInProgress was wrong. --- _fixtures/issue594.go | 22 ++++++ proc/disasm_amd64.go | 2 +- proc/proc_test.go | 33 ++++++--- proc/proc_windows.go | 154 ++++++++++++++++++++++++++++++---------- proc/syscall_windows.go | 19 +++++ proc/threads_windows.go | 50 +++++++------ 6 files changed, 214 insertions(+), 66 deletions(-) create mode 100644 _fixtures/issue594.go diff --git a/_fixtures/issue594.go b/_fixtures/issue594.go new file mode 100644 index 00000000..964bf3ee --- /dev/null +++ b/_fixtures/issue594.go @@ -0,0 +1,22 @@ +package main + +import ( + "fmt" + "runtime" +) + +func dontsegfault() { + var p *int + func() int { + defer func() { + recover() + }() + return *p + }() +} + +func main() { + dontsegfault() + runtime.Breakpoint() + fmt.Println("should stop here") +} diff --git a/proc/disasm_amd64.go b/proc/disasm_amd64.go index b2278a34..df00a30a 100644 --- a/proc/disasm_amd64.go +++ b/proc/disasm_amd64.go @@ -123,7 +123,7 @@ var windowsPrologue3 = instrseq{x86asm.MOV, x86asm.MOV, x86asm.MOV, x86asm.CMP, var unixPrologue = instrseq{x86asm.MOV, x86asm.LEA, x86asm.CMP, x86asm.JBE} var unixPrologue2 = instrseq{x86asm.MOV, x86asm.CMP, x86asm.JBE} var unixPrologue3 = instrseq{x86asm.MOV, x86asm.MOV, x86asm.CMP, x86asm.JE} -var prologues = []instrseq{windowsPrologue, windowsPrologue2, unixPrologue, unixPrologue2, unixPrologue3} +var prologues = []instrseq{windowsPrologue, windowsPrologue2, windowsPrologue3, unixPrologue, unixPrologue2, unixPrologue3} // FirstPCAfterPrologue returns the address of the first instruction after the prologue for function fn // If sameline is set FirstPCAfterPrologue will always return an address associated with the same line as fn.Entry diff --git a/proc/proc_test.go b/proc/proc_test.go index 2a5ad1c0..56355f0d 100644 --- a/proc/proc_test.go +++ b/proc/proc_test.go @@ -801,7 +801,10 @@ func TestStacktraceGoroutine(t *testing.T) { for i, g := range gs { locations, err := g.Stacktrace(40) - assertNoError(err, t, "GoroutineStacktrace()") + if err != nil { + // On windows we do not have frame information for goroutines doing system calls. + continue + } if stackMatch(mainStack, locations, false) { mainCount++ @@ -2084,8 +2087,8 @@ func TestStepConcurrentDirect(t *testing.T) { } func nextInProgress(p *Process) bool { - for _, th := range p.Threads { - if th.CurrentBreakpoint != nil && th.CurrentBreakpoint.Internal() { + for _, bp := range p.Breakpoints { + if bp.Internal() { return true } } @@ -2135,13 +2138,13 @@ func TestStepConcurrentPtr(t *testing.T) { assertNoError(p.Continue(), t, "Continue()") } - f, ln = currentLineNumber(p, t) - if ln != 13 { - t.Fatalf("Step did not step into function call (13): %s:%d (gid: %d)", f, ln, p.SelectedGoroutine.ID) + if p.SelectedGoroutine.ID != gid { + t.Fatalf("Step switched goroutines (wanted: %d got: %d)", gid, p.SelectedGoroutine.ID) } - if p.SelectedGoroutine.ID != gid { - t.Fatalf("Step switched goroutines (%d %d)", gid, p.SelectedGoroutine.ID) + f, ln = currentLineNumber(p, t) + if ln != 13 { + t.Fatalf("Step did not step into function call (13): %s:%d", f, ln) } count++ @@ -2197,3 +2200,17 @@ func TestStepOnCallPtrInstr(t *testing.T) { } }) } + +func TestIssue594(t *testing.T) { + // Exceptions that aren't caused by breakpoints should be propagated + // back to the target. + // In particular the target should be able to cause a nil pointer + // dereference panic and recover from it. + withTestProcess("issue594", t, func(p *Process, fixture protest.Fixture) { + assertNoError(p.Continue(), t, "Continue()") + f, ln := currentLineNumber(p, t) + if ln != 21 { + t.Fatalf("Program stopped at %s:%d, expected :21", f, ln) + } + }) +} diff --git a/proc/proc_windows.go b/proc/proc_windows.go index abd302be..10a2393a 100644 --- a/proc/proc_windows.go +++ b/proc/proc_windows.go @@ -91,19 +91,24 @@ func Launch(cmd []string) (*Process, error) { si.StdOutput = sys.Handle(fd[1]) si.StdErr = sys.Handle(fd[2]) pi := new(sys.ProcessInformation) - err = sys.CreateProcess(argv0, cmdLine, nil, nil, true, _DEBUG_ONLY_THIS_PROCESS, nil, nil, si, pi) + + dbp := New(0) + dbp.execPtraceFunc(func() { + err = sys.CreateProcess(argv0, cmdLine, nil, nil, true, _DEBUG_ONLY_THIS_PROCESS, nil, nil, si, pi) + }) if err != nil { return nil, err } sys.CloseHandle(sys.Handle(pi.Process)) sys.CloseHandle(sys.Handle(pi.Thread)) - return newDebugProcess(int(pi.ProcessId), argv0Go) + dbp.Pid = int(pi.ProcessId) + + return newDebugProcess(dbp, argv0Go) } // newDebugProcess prepares process pid for debugging. -func newDebugProcess(pid int, exepath string) (*Process, error) { - dbp := New(pid) +func newDebugProcess(dbp *Process, exepath string) (*Process, error) { // It should not actually be possible for the // call to waitForDebugEvent to fail, since Windows // will always fire a CREATE_PROCESS_DEBUG_EVENT event @@ -112,7 +117,7 @@ func newDebugProcess(pid int, exepath string) (*Process, error) { var err error var tid, exitCode int dbp.execPtraceFunc(func() { - tid, exitCode, err = dbp.waitForDebugEvent() + tid, exitCode, err = dbp.waitForDebugEvent(waitBlocking) }) if err != nil { return nil, err @@ -121,6 +126,22 @@ func newDebugProcess(pid int, exepath string) (*Process, error) { dbp.postExit() return nil, ProcessExitedError{Pid: dbp.Pid, Status: exitCode} } + // Suspend all threads so that the call to _ContinueDebugEvent will + // not resume the target. + for _, thread := range dbp.Threads { + _, err := _SuspendThread(thread.os.hThread) + if err != nil { + return nil, err + } + } + + dbp.execPtraceFunc(func() { + err = _ContinueDebugEvent(uint32(dbp.Pid), uint32(dbp.os.breakThread), _DBG_CONTINUE) + }) + if err != nil { + return nil, err + } + return initializeDebugProcess(dbp, exepath, false) } @@ -169,7 +190,7 @@ func Attach(pid int) (*Process, error) { if err != nil { return nil, err } - return newDebugProcess(pid, exepath) + return newDebugProcess(New(pid), exepath) } // Kill kills the process. @@ -198,7 +219,7 @@ func (dbp *Process) updateThreadList() error { return nil } -func (dbp *Process) addThread(hThread syscall.Handle, threadID int, attach bool) (*Thread, error) { +func (dbp *Process) addThread(hThread syscall.Handle, threadID int, attach, suspendNewThreads bool) (*Thread, error) { if thread, ok := dbp.Threads[threadID]; ok { return thread, nil } @@ -212,6 +233,12 @@ func (dbp *Process) addThread(hThread syscall.Handle, threadID int, attach bool) if dbp.CurrentThread == nil { dbp.SwitchThread(thread.ID) } + if suspendNewThreads { + _, err := _SuspendThread(thread.os.hThread) + if err != nil { + return nil, err + } + } return thread, nil } @@ -402,12 +429,24 @@ func dwarfFromPE(f *pe.File) (*dwarf.Data, error) { return dwarf.New(abbrev, nil, nil, info, line, nil, nil, str) } -func (dbp *Process) waitForDebugEvent() (threadID, exitCode int, err error) { +type waitForDebugEventFlags int + +const ( + waitBlocking waitForDebugEventFlags = 1 << iota + waitSuspendNewThreads +) + +func (dbp *Process) waitForDebugEvent(flags waitForDebugEventFlags) (threadID, exitCode int, err error) { var debugEvent _DEBUG_EVENT shouldExit := false for { + continueStatus := uint32(_DBG_CONTINUE) + var milliseconds uint32 = 0 + if flags&waitBlocking != 0 { + milliseconds = syscall.INFINITE + } // Wait for a debug event... - err := _WaitForDebugEvent(&debugEvent, syscall.INFINITE) + err := _WaitForDebugEvent(&debugEvent, milliseconds) if err != nil { return 0, 0, err } @@ -425,14 +464,14 @@ func (dbp *Process) waitForDebugEvent() (threadID, exitCode int, err error) { } } dbp.os.hProcess = debugInfo.Process - _, err = dbp.addThread(debugInfo.Thread, int(debugEvent.ThreadId), false) + _, err = dbp.addThread(debugInfo.Thread, int(debugEvent.ThreadId), false, flags&waitSuspendNewThreads != 0) if err != nil { return 0, 0, err } break case _CREATE_THREAD_DEBUG_EVENT: debugInfo := (*_CREATE_THREAD_DEBUG_INFO)(unionPtr) - _, err = dbp.addThread(debugInfo.Thread, int(debugEvent.ThreadId), false) + _, err = dbp.addThread(debugInfo.Thread, int(debugEvent.ThreadId), false, flags&waitSuspendNewThreads != 0) if err != nil { return 0, 0, err } @@ -458,9 +497,14 @@ func (dbp *Process) waitForDebugEvent() (threadID, exitCode int, err error) { case _RIP_EVENT: break case _EXCEPTION_DEBUG_EVENT: - tid := int(debugEvent.ThreadId) - dbp.os.breakThread = tid - return tid, 0, nil + exception := (*_EXCEPTION_DEBUG_INFO)(unionPtr) + if code := exception.ExceptionRecord.ExceptionCode; code == _EXCEPTION_BREAKPOINT || code == _EXCEPTION_SINGLE_STEP { + tid := int(debugEvent.ThreadId) + dbp.os.breakThread = tid + return tid, 0, nil + } else { + continueStatus = _DBG_EXCEPTION_NOT_HANDLED + } case _EXIT_PROCESS_DEBUG_EVENT: debugInfo := (*_EXIT_PROCESS_DEBUG_INFO)(unionPtr) exitCode = int(debugInfo.ExitCode) @@ -470,7 +514,7 @@ func (dbp *Process) waitForDebugEvent() (threadID, exitCode int, err error) { } // .. and then continue unless we received an event that indicated we should break into debugger. - err = _ContinueDebugEvent(debugEvent.ProcessId, debugEvent.ThreadId, _DBG_CONTINUE) + err = _ContinueDebugEvent(debugEvent.ProcessId, debugEvent.ThreadId, continueStatus) if err != nil { return 0, 0, err } @@ -485,7 +529,7 @@ func (dbp *Process) trapWait(pid int) (*Thread, error) { var err error var tid, exitCode int dbp.execPtraceFunc(func() { - tid, exitCode, err = dbp.waitForDebugEvent() + tid, exitCode, err = dbp.waitForDebugEvent(waitBlocking) }) if err != nil { return nil, err @@ -507,17 +551,51 @@ func (dbp *Process) wait(pid, options int) (int, *sys.WaitStatus, error) { } func (dbp *Process) setCurrentBreakpoints(trapthread *Thread) error { - // TODO: In theory, we should also be setting the breakpoints on other - // threads that happen to have hit this BP. But doing so leads to periodic - // failures in the TestBreakpointsCounts test with hit counts being too high, - // which can be traced back to occurrences of multiple threads hitting a BP - // at the same time. + // While the debug event that stopped the target was being propagated + // other target threads could generate other debug events. + // After this function we need to know about all the threads + // stopped on a breakpoint. To do that we first suspend all target + // threads and then repeatedly call _ContinueDebugEvent followed by + // waitForDebugEvent in non-blocking mode. + // We need to explicitly call SuspendThread because otherwise the + // call to _ContinueDebugEvent will resume execution of some of the + // target threads. - // My guess is that Windows will correctly trigger multiple DEBUG_EVENT's - // in this case, one for each thread, so we should only handle the BP hit - // on the thread that the debugger was evented on. + err := trapthread.SetCurrentBreakpoint() + if err != nil { + return err + } - return trapthread.SetCurrentBreakpoint() + for _, thread := range dbp.Threads { + thread.running = false + _, err := _SuspendThread(thread.os.hThread) + if err != nil { + return err + } + } + + for { + var err error + var tid int + dbp.execPtraceFunc(func() { + err = _ContinueDebugEvent(uint32(dbp.Pid), uint32(dbp.os.breakThread), _DBG_CONTINUE) + if err == nil { + tid, _, _ = dbp.waitForDebugEvent(waitSuspendNewThreads) + } + }) + if err != nil { + return err + } + if tid == 0 { + break + } + err = dbp.Threads[tid].SetCurrentBreakpoint() + if err != nil { + return err + } + } + + return nil } func (dbp *Process) exitGuard(err error) error { @@ -525,21 +603,23 @@ func (dbp *Process) exitGuard(err error) error { } func (dbp *Process) resume() error { - // Only resume the thread that broke into the debugger - thread := dbp.Threads[dbp.os.breakThread] - // This relies on the same assumptions as dbp.setCurrentBreakpoints - if thread.CurrentBreakpoint != nil { - if err := thread.StepInstruction(); err != nil { + for _, thread := range dbp.Threads { + if thread.CurrentBreakpoint != nil { + if err := thread.StepInstruction(); err != nil { + return err + } + thread.CurrentBreakpoint = nil + } + } + + for _, thread := range dbp.Threads { + thread.running = true + _, err := _ResumeThread(thread.os.hThread) + if err != nil { return err } - thread.CurrentBreakpoint = nil - } - // In case we are now on a different thread, make sure we resume - // the thread that is broken. - thread = dbp.Threads[dbp.os.breakThread] - if err := thread.resume(); err != nil { - return err } + return nil } diff --git a/proc/syscall_windows.go b/proc/syscall_windows.go index 3154ad69..770a7890 100644 --- a/proc/syscall_windows.go +++ b/proc/syscall_windows.go @@ -54,6 +54,20 @@ type _LOAD_DLL_DEBUG_INFO struct { Unicode uint16 } +type _EXCEPTION_DEBUG_INFO struct { + ExceptionRecord _EXCEPTION_RECORD + FirstChance uint32 +} + +type _EXCEPTION_RECORD struct { + ExceptionCode uint32 + ExceptionFlags uint32 + ExceptionRecord *_EXCEPTION_RECORD + ExceptionAddress uintptr + NumberParameters uint32 + ExceptionInformation [_EXCEPTION_MAXIMUM_PARAMETERS]uintptr +} + const ( _ThreadBasicInformation = 0 @@ -72,6 +86,11 @@ const ( // DEBUG_ONLY_THIS_PROCESS tracks https://msdn.microsoft.com/en-us/library/windows/desktop/ms684863(v=vs.85).aspx _DEBUG_ONLY_THIS_PROCESS = 0x00000002 + + _EXCEPTION_BREAKPOINT = 0x80000003 + _EXCEPTION_SINGLE_STEP = 0x80000004 + + _EXCEPTION_MAXIMUM_PARAMETERS = 15 ) func _NT_SUCCESS(x _NTSTATUS) bool { diff --git a/proc/threads_windows.go b/proc/threads_windows.go index 77514329..e0452e79 100644 --- a/proc/threads_windows.go +++ b/proc/threads_windows.go @@ -42,34 +42,44 @@ func (t *Thread) singleStep() error { return err } - // Suspend all threads except this one - for _, thread := range t.dbp.Threads { - if thread.ID == t.ID { - continue - } - _, _ = _SuspendThread(thread.os.hThread) + _, err = _ResumeThread(t.os.hThread) + if err != nil { + return err + } + + for { + var tid, exitCode int + t.dbp.execPtraceFunc(func() { + tid, exitCode, err = t.dbp.waitForDebugEvent(waitBlocking | waitSuspendNewThreads) + }) + if err != nil { + return err + } + if tid == 0 { + t.dbp.postExit() + return ProcessExitedError{Pid: t.dbp.Pid, Status: exitCode} + } + + if t.dbp.os.breakThread == t.ID { + break + } + + t.dbp.execPtraceFunc(func() { + err = _ContinueDebugEvent(uint32(t.dbp.Pid), uint32(t.dbp.os.breakThread), _DBG_CONTINUE) + }) + } + + _, err = _SuspendThread(t.os.hThread) + if err != nil { + return err } - // Continue and wait for the step to complete - err = nil t.dbp.execPtraceFunc(func() { err = _ContinueDebugEvent(uint32(t.dbp.Pid), uint32(t.ID), _DBG_CONTINUE) }) if err != nil { return err } - _, err = t.dbp.trapWait(0) - if err != nil { - return err - } - - // Resume all threads except this one - for _, thread := range t.dbp.Threads { - if thread.ID == t.ID { - continue - } - _, _ = _ResumeThread(thread.os.hThread) - } // Unset the processor TRAP flag err = _GetThreadContext(t.os.hThread, context)