PenaDevops/delve
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pkg/proc: fix watchpoints on macos (#3703)

Browse Source 
It seems newer MacOS kernels are sending mach exceptions for watchpoints
which contain the hardware register number as opposed to the address
which triggered the exception. Also, ARM64 seems to have switched to
sending _EXC_I386_SGL as medata[0] for this exception type.
This commit is contained in:
Derek Parker 2024-04-29 05:06:32 -07:00 committed by GitHub
parent 8c07c984e8
commit bba7547156
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 86 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
_fixtures/databpcountstest.go
View File

@ -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)
}

1
pkg/proc/bininfo.go
View File

@ -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
}

39
pkg/proc/gdbserial/gdbserver.go
View File

@ -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

19
pkg/proc/gdbserial/gdbserver_conn.go
View File

@ -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).
@ -735,6 +736,7 @@ 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_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,18 +799,26 @@ 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 {
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]
}
}
}
return false, sp, nil
case 'W', 'X':
// process exited, next two character are exit code
semicolon := bytes.Index(resp, []byte{';'})
if semicolon < 0 {
semicolon = len(resp)
}

47
pkg/proc/proc_test.go
View File

@ -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)

1
service/debugger/debugger.go
View File

@ -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 {

2
service/test/integration2_test.go
View File

@ -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
}