diff --git a/_fixtures/databpcountstest.go b/_fixtures/databpcountstest.go index 6a79a28b8b..a317db5cba 100644 --- a/_fixtures/databpcountstest.go +++ b/_fixtures/databpcountstest.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "math/rand" "sync" "time" @@ -12,7 +11,6 @@ var globalvar1 int func demo(id int, wait *sync.WaitGroup) { for i := 0; i < 100; i++ { sleep := rand.Intn(10) + 1 - fmt.Printf("id: %d step: %d sleeping %d\n", id, i, sleep) globalvar1 = globalvar1 + 1 time.Sleep(time.Duration(sleep) * time.Millisecond) } diff --git a/pkg/proc/bininfo.go b/pkg/proc/bininfo.go index 6be55f8216..0171fea474 100644 --- a/pkg/proc/bininfo.go +++ b/pkg/proc/bininfo.go @@ -1876,7 +1876,6 @@ func (bi *BinaryInfo) parseDebugFramePE(image *Image, exe *pe.File, debugInfoByt // loadBinaryInfoMacho specifically loads information from a Mach-O binary. func loadBinaryInfoMacho(bi *BinaryInfo, image *Image, path string, entryPoint uint64, wg *sync.WaitGroup) error { exe, err := macho.Open(path) - if err != nil { return err } diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 23fdbafd8f..436af81f65 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -182,6 +182,7 @@ type gdbThread struct { sig uint8 // signal received by thread after last stop setbp bool // thread was stopped because of a breakpoint watchAddr uint64 // if > 0 this is the watchpoint address + watchReg int // if < 0 there are no active watchpoints returned common proc.CommonThread } @@ -851,7 +852,13 @@ func (p *gdbProcess) ContinueOnce(cctx *proc.ContinueOnceContext) (proc.Thread, if p.conn.direction == proc.Forward { // step threads stopped at any breakpoint over their breakpoint for _, thread := range p.threads { - if thread.CurrentBreakpoint.Breakpoint != nil { + // Do not single step over a breakpoint if it is a watchpoint. The PC will already have advanced and we + // won't experience the same stutter effect as with a breakpoint. Also, there is a bug in certain versions + // of the MacOS mach kernel where single stepping when we have hardware watchpoints set will cause the + // kernel to send a spurious mach exception. + // See: https://github.com/llvm/llvm-project/blob/b9f2c16b50f68c978e90190f46a7c0db3f39e98c/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp#L814 + // TODO(deparker): We can skip single stepping in the case of thread.BinInfo().Arch.BreakInstrMovesPC(). + if thread.CurrentBreakpoint.Breakpoint != nil && thread.CurrentBreakpoint.WatchType == 0 { if err := thread.stepInstruction(); err != nil { return nil, proc.StopUnknown, err } @@ -896,6 +903,7 @@ continueLoop: // reason the thread returned by resume() stopped. trapthread.sig = sp.sig trapthread.watchAddr = sp.watchAddr + trapthread.watchReg = sp.watchReg } var shouldStop, shouldExitErr bool @@ -1427,12 +1435,14 @@ func (p *gdbProcess) updateThreadList(tu *threadUpdater) error { } return err } - th.setbp = (sp.reason == "breakpoint" || (sp.reason == "" && sp.sig == breakpointSignal) || (sp.watchAddr > 0)) + th.setbp = (sp.reason == "breakpoint" || (sp.reason == "" && sp.sig == breakpointSignal) || (sp.watchAddr > 0) || (sp.watchReg >= 0)) th.sig = sp.sig th.watchAddr = sp.watchAddr + th.watchReg = sp.watchReg } else { th.sig = 0 th.watchAddr = 0 + th.watchReg = -1 } } @@ -1913,6 +1923,8 @@ func (t *gdbThread) reloadGAlloc() error { func (t *gdbThread) clearBreakpointState() { t.setbp = false + t.watchAddr = 0 + t.watchReg = -1 t.CurrentBreakpoint.Clear() } @@ -1921,13 +1933,36 @@ func (t *gdbThread) SetCurrentBreakpoint(adjustPC bool) error { // adjustPC is ignored, it is the stub's responsibility to set the PC // address correctly after hitting a breakpoint. t.CurrentBreakpoint.Clear() + // t.watchAddr on certain mach kernel versions could contain the address of a + // software breakpoint, hardcoded breakpoint (e.g. runtime.Breakpoint) or a + // hardware watchpoint. The mach exception produced by the kernel *should* disambiguate + // but it doesn't. if t.watchAddr > 0 { t.CurrentBreakpoint.Breakpoint = t.p.Breakpoints().M[t.watchAddr] if t.CurrentBreakpoint.Breakpoint == nil { + buf := make([]byte, t.BinInfo().Arch.BreakpointSize()) + _, err := t.p.ReadMemory(buf, t.watchAddr) + isHardcodedBreakpoint := err == nil && (bytes.Equal(t.BinInfo().Arch.BreakpointInstruction(), buf) || bytes.Equal(t.BinInfo().Arch.AltBreakpointInstruction(), buf)) + if isHardcodedBreakpoint { + // This is a hardcoded breakpoint, ignore. + // TODO(deparker): There's an optimization here since we will do this + // again at a higher level to determine if we've stopped at a hardcoded breakpoint. + // We could set some state here so that we don't do extra work later. + t.watchAddr = 0 + return nil + } return fmt.Errorf("could not find watchpoint at address %#x", t.watchAddr) } return nil } + if t.watchReg >= 0 { + for _, bp := range t.p.Breakpoints().M { + if bp.WatchType != 0 && bp.HWBreakIndex == uint8(t.watchReg) { + t.CurrentBreakpoint.Breakpoint = bp + return nil + } + } + } regs, err := t.Registers() if err != nil { return err diff --git a/pkg/proc/gdbserial/gdbserver_conn.go b/pkg/proc/gdbserial/gdbserver_conn.go index 61d61b9ea7..47295d12ea 100644 --- a/pkg/proc/gdbserial/gdbserver_conn.go +++ b/pkg/proc/gdbserial/gdbserver_conn.go @@ -723,6 +723,7 @@ type stopPacket struct { sig uint8 reason string watchAddr uint64 + watchReg int } // Mach exception codes used to decode metype/medata keys in stop packets (necessary to support watchpoints with debugserver). @@ -732,9 +733,10 @@ type stopPacket struct { // https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/i386/exception.h.auto.html // https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/arm/exception.h.auto.html const ( - _EXC_BREAKPOINT = 6 // mach exception type for hardware breakpoints - _EXC_I386_SGL = 1 // mach exception code for single step on x86, for some reason this is also used for watchpoints - _EXC_ARM_DA_DEBUG = 0x102 // mach exception code for debug fault on arm/arm64 + _EXC_BREAKPOINT = 6 // mach exception type for hardware breakpoints + _EXC_I386_SGL = 1 // mach exception code for single step on x86, for some reason this is also used for watchpoints + _EXC_ARM_DA_DEBUG = 0x102 // mach exception code for debug fault on arm/arm64 + _EXC_ARM_BREAKPOINT = 1 // mach exception code for breakpoint on arm/arm64 ) // executes 'vCont' (continue/step) command @@ -750,13 +752,14 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd return false, stopPacket{}, fmt.Errorf("malformed stop packet: %s", string(resp)) } sp.sig = uint8(sig) + sp.watchReg = -1 if logflags.GdbWire() && gdbWireFullStopPacket { conn.log.Debugf("full stop packet: %s", string(resp)) } var metype int - var medata = make([]uint64, 0, 10) + medata := make([]uint64, 0, 10) csp := colonSemicolonParser{buf: resp[3:]} for csp.next() { @@ -796,8 +799,18 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd sp.watchAddr = medata[1] // this should be zero if this is really a single step stop and non-zero for watchpoints } case "arm64": - if metype == _EXC_BREAKPOINT && len(medata) >= 2 && medata[0] == _EXC_ARM_DA_DEBUG { - sp.watchAddr = medata[1] + if metype == _EXC_BREAKPOINT && len(medata) >= 2 && (medata[0] == _EXC_ARM_DA_DEBUG || medata[0] == _EXC_ARM_BREAKPOINT) { + // The arm64 specification allows for up to 16 debug registers. + // The registers are zero indexed, thus a value less than 16 will + // be a hardware breakpoint register index. + // See: https://developer.arm.com/documentation/102120/0101/Debug-exceptions + // TODO(deparker): we can ask debugserver for the number of hardware breakpoints + // directly. + if medata[1] < 16 { + sp.watchReg = int(medata[1]) + } else { + sp.watchAddr = medata[1] + } } } @@ -805,9 +818,7 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd case 'W', 'X': // process exited, next two character are exit code - semicolon := bytes.Index(resp, []byte{';'}) - if semicolon < 0 { semicolon = len(resp) } diff --git a/pkg/proc/proc_test.go b/pkg/proc/proc_test.go index d127d9e6a6..219f7170dd 100644 --- a/pkg/proc/proc_test.go +++ b/pkg/proc/proc_test.go @@ -3,6 +3,7 @@ package proc_test import ( "bytes" "encoding/binary" + "errors" "flag" "fmt" "go/ast" @@ -173,6 +174,22 @@ func assertLineNumber(p *proc.Target, t *testing.T, lineno int, descr string) (s return f, l } +func assertLineNumberIn(p *proc.Target, t *testing.T, linenos []int, descr string) (string, int) { + f, l := currentLineNumber(p, t) + found := false + for _, lineno := range linenos { + if l == lineno { + found = true + break + } + } + if !found { + _, callerFile, callerLine, _ := runtime.Caller(1) + t.Fatalf("%s expected lines :%#v got %s:%d\n\tat %s:%d", descr, linenos, f, l, callerFile, callerLine) + } + return f, l +} + func assertFunctionName(p *proc.Target, t *testing.T, fnname string, descr string) { pc := currentPC(p, t) f, l, fn := p.BinInfo().PCToLine(pc) @@ -985,7 +1002,6 @@ func TestStacktrace2(t *testing.T) { t.Fatalf("Stack error at main.g()\n%v\n", locations) } }) - } func stackMatch(stack []loc, locations []proc.Stackframe, skipRuntime bool) bool { @@ -5421,13 +5437,8 @@ func TestWatchpointsBasic(t *testing.T) { skipOn(t, "see https://github.com/go-delve/delve/issues/2768", "windows") protest.AllowRecording(t) - position1 := 19 - position5 := 41 - - if runtime.GOARCH == "arm64" { - position1 = 18 - position5 = 40 - } + position1 := []int{18, 19} + position5 := []int{40, 41} withTestProcess("databpeasy", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { setFunctionBreakpoint(p, t, "main.main") @@ -5443,7 +5454,7 @@ func TestWatchpointsBasic(t *testing.T) { assertNoError(err, t, "SetDataBreakpoint(write-only)") assertNoError(grp.Continue(), t, "Continue 1") - assertLineNumber(p, t, position1, "Continue 1") // Position 1 + assertLineNumberIn(p, t, position1, "Continue 1") // Position 1 if curbp := p.CurrentThread().Breakpoint().Breakpoint; curbp == nil || (curbp.LogicalID() != bp.LogicalID()) { t.Fatal("breakpoint not set") @@ -5470,7 +5481,7 @@ func TestWatchpointsBasic(t *testing.T) { assertNoError(err, t, "SetDataBreakpoint(write-only, again)") assertNoError(grp.Continue(), t, "Continue 5") - assertLineNumber(p, t, position5, "Continue 5") // Position 5 + assertLineNumberIn(p, t, position5, "Continue 5") // Position 5 }) } @@ -5479,6 +5490,9 @@ func TestWatchpointCounts(t *testing.T) { skipOn(t, "not implemented", "386") skipOn(t, "see https://github.com/go-delve/delve/issues/2768", "windows") skipOn(t, "not implemented", "ppc64le") + if _, isTeamCityTest := os.LookupEnv("TEAMCITY_VERSION"); isTeamCityTest { + skipOn(t, "CI is running a version of macOS that is too old (11.2)", "darwin", "arm64") + } protest.AllowRecording(t) withTestProcess("databpcountstest", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { @@ -5493,7 +5507,7 @@ func TestWatchpointCounts(t *testing.T) { for { if err := grp.Continue(); err != nil { - if _, exited := err.(proc.ErrProcessExited); exited { + if errors.As(err, &proc.ErrProcessExited{}) { break } assertNoError(err, t, "Continue()") @@ -5595,13 +5609,12 @@ func TestWatchpointStack(t *testing.T) { skipOn(t, "not implemented", "386") skipOn(t, "not implemented", "ppc64le") skipOn(t, "see https://github.com/go-delve/delve/issues/2768", "windows") + if _, isTeamCityTest := os.LookupEnv("TEAMCITY_VERSION"); isTeamCityTest { + skipOn(t, "CI is running a version of macOS that is too old (11.2)", "darwin", "arm64") + } protest.AllowRecording(t) - position1 := 17 - - if runtime.GOARCH == "arm64" { - position1 = 16 - } + position1 := []int{16, 17} withTestProcess("databpstack", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) { setFileBreakpoint(p, t, fixture.Source, 11) // Position 0 breakpoint @@ -5650,7 +5663,7 @@ func TestWatchpointStack(t *testing.T) { } assertNoError(grp.Continue(), t, "Continue 1") - assertLineNumber(p, t, position1, "Continue 1") // Position 1 + assertLineNumberIn(p, t, position1, "Continue 1") // Position 1 assertNoError(grp.Continue(), t, "Continue 2") t.Logf("%#v", p.CurrentThread().Breakpoint().Breakpoint) diff --git a/service/debugger/debugger.go b/service/debugger/debugger.go index 4432426b2b..0be8c70a82 100644 --- a/service/debugger/debugger.go +++ b/service/debugger/debugger.go @@ -494,7 +494,6 @@ func (d *Debugger) Restart(rerecord bool, pos string, resetArgs bool, newArgs [] if !resetArgs && (d.config.Stdout.File != nil || d.config.Stderr.File != nil) { return nil, ErrCanNotRestart - } if err := d.detach(true); err != nil { diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go index b6aaac43db..48382c2185 100644 --- a/service/test/integration2_test.go +++ b/service/test/integration2_test.go @@ -1011,7 +1011,6 @@ func TestClientServer_FindLocations(t *testing.T) { if locsNoSubst[0].PC != locsSubst[0].PC { t.Fatalf("FindLocation with substitute rules mismatch %#v %#v", locsNoSubst[0], locsSubst[0]) } - }) withTestClient2("testnextdefer", t, func(c service.Client) { @@ -2623,7 +2622,6 @@ func TestLongStringArg(t *testing.T) { t.Logf("%#v\n", var2) if var2.Value != val2 { t.Fatalf("wrong value for variable: %q", var2.Value) - } return var1.Addr }