From 1a68f8d3519f7113fcf9f86e174391b973bb7ec1 Mon Sep 17 00:00:00 2001 From: aarzilli Date: Tue, 14 Feb 2017 15:17:00 +0100 Subject: [PATCH] proc/windows: handle delayed events Sometimes windows will send us events about breakpoints we have already removed from the code despite the fact that we go to great lengths to avoid this already. Change waitForDebugEvent to check that when we receive a breakpoint event the corresponding memory actually contains an INT 3 instruction, if it doesn't ignore the event and restart the thread. --- cmd/dlv/dlv_test.go | 5 ++++- pkg/proc/proc_test.go | 43 ++++++++++++++++++++++++++++++++++++---- pkg/proc/proc_windows.go | 35 +++++++++++++++++++++++++++++--- 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/cmd/dlv/dlv_test.go b/cmd/dlv/dlv_test.go index 472cd741..2b424e57 100644 --- a/cmd/dlv/dlv_test.go +++ b/cmd/dlv/dlv_test.go @@ -46,9 +46,12 @@ func TestBuild(t *testing.T) { assertNoError(err, t, "stdout pipe") cmd.Start() defer func() { - cmd.Process.Signal(os.Interrupt) if runtime.GOOS != "windows" { + cmd.Process.Signal(os.Interrupt) cmd.Wait() + } else { + // sending os.Interrupt on windows is not supported + cmd.Process.Kill() } }() diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index 6847b239..2ced0eb0 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -818,6 +818,7 @@ func TestStacktraceGoroutine(t *testing.T) { locations, err := g.Stacktrace(40) if err != nil { // On windows we do not have frame information for goroutines doing system calls. + t.Logf("Could not retrieve goroutine stack for goid=%d: %v", g.ID, err) continue } @@ -2096,6 +2097,14 @@ func TestStepConcurrentDirect(t *testing.T) { _, err = p.ClearBreakpoint(bp.Addr) assertNoError(err, t, "ClearBreakpoint()") + for _, b := range p.Breakpoints() { + if b.Name == "unrecovered-panic" { + _, err := p.ClearBreakpoint(b.Addr) + assertNoError(err, t, "ClearBreakpoint(unrecovered-panic)") + break + } + } + gid := p.selectedGoroutine.ID seq := []int{37, 38, 13, 15, 16, 38} @@ -2103,16 +2112,31 @@ func TestStepConcurrentDirect(t *testing.T) { i := 0 count := 0 for { + anyerr := false + if p.selectedGoroutine.ID != gid { + t.Errorf("Step switched to different goroutine %d %d\n", gid, p.selectedGoroutine.ID) + anyerr = true + } f, ln := currentLineNumber(p, t) if ln != seq[i] { if i == 1 && ln == 40 { // loop exited break } - t.Fatalf("Program did not continue at expected location (%d) %s:%d", seq[i], f, ln) + frames, err := p.currentThread.Stacktrace(20) + if err != nil { + t.Errorf("Could not get stacktrace of goroutine %d\n", p.selectedGoroutine.ID) + } else { + t.Logf("Goroutine %d (thread: %d):", p.selectedGoroutine.ID, p.currentThread.ID) + for _, frame := range frames { + t.Logf("\t%s:%d (%#x)", frame.Call.File, frame.Call.Line, frame.Current.PC) + } + } + t.Errorf("Program did not continue at expected location (%d) %s:%d [i %d count %d]", seq[i], f, ln, i, count) + anyerr = true } - if p.selectedGoroutine.ID != gid { - t.Fatalf("Step switched to different goroutine %d %d\n", gid, p.selectedGoroutine.ID) + if anyerr { + t.FailNow() } i = (i + 1) % len(seq) if i == 0 { @@ -2143,6 +2167,14 @@ func TestStepConcurrentPtr(t *testing.T) { _, err = p.SetBreakpoint(pc, UserBreakpoint, nil) assertNoError(err, t, "SetBreakpoint()") + for _, b := range p.Breakpoints() { + if b.Name == "unrecovered-panic" { + _, err := p.ClearBreakpoint(b.Addr) + assertNoError(err, t, "ClearBreakpoint(unrecovered-panic)") + break + } + } + kvals := map[int]int64{} count := 0 for { @@ -2155,7 +2187,10 @@ func TestStepConcurrentPtr(t *testing.T) { f, ln := currentLineNumber(p, t) if ln != 24 { - t.Fatalf("Program did not continue at expected location (24): %s:%d", f, ln) + for _, th := range p.threads { + t.Logf("thread %d stopped on breakpoint %v", th.ID, th.CurrentBreakpoint) + } + t.Fatalf("Program did not continue at expected location (24): %s:%d %#x [%v] (gid %d count %d)", f, ln, currentPC(p, t), p.currentThread.CurrentBreakpoint, p.selectedGoroutine.ID, count) } gid := p.selectedGoroutine.ID diff --git a/pkg/proc/proc_windows.go b/pkg/proc/proc_windows.go index a67e09c3..d3b7773d 100644 --- a/pkg/proc/proc_windows.go +++ b/pkg/proc/proc_windows.go @@ -509,11 +509,40 @@ func (dbp *Process) waitForDebugEvent(flags waitForDebugEventFlags) (threadID, e break case _EXCEPTION_DEBUG_EVENT: exception := (*_EXCEPTION_DEBUG_INFO)(unionPtr) - if code := exception.ExceptionRecord.ExceptionCode; code == _EXCEPTION_BREAKPOINT || code == _EXCEPTION_SINGLE_STEP { - tid := int(debugEvent.ThreadId) + tid := int(debugEvent.ThreadId) + + switch code := exception.ExceptionRecord.ExceptionCode; code { + case _EXCEPTION_BREAKPOINT: + + // check if the exception address really is a breakpoint instruction, if + // it isn't we already removed that breakpoint and we can't deal with + // this exception anymore. + atbp := true + if thread, found := dbp.threads[tid]; found { + if data, err := thread.readMemory(exception.ExceptionRecord.ExceptionAddress, dbp.arch.BreakpointSize()); err == nil { + instr := dbp.arch.BreakpointInstruction() + for i := range instr { + if data[i] != instr[i] { + atbp = false + break + } + } + } + if !atbp { + thread.SetPC(uint64(exception.ExceptionRecord.ExceptionAddress)) + } + } + + if atbp { + dbp.os.breakThread = tid + return tid, 0, nil + } else { + continueStatus = _DBG_CONTINUE + } + case _EXCEPTION_SINGLE_STEP: dbp.os.breakThread = tid return tid, 0, nil - } else { + default: continueStatus = _DBG_EXCEPTION_NOT_HANDLED } case _EXIT_PROCESS_DEBUG_EVENT: