From 4f28742da6df1b126add74609270c799adbbc9ca Mon Sep 17 00:00:00 2001 From: Derek Parker Date: Wed, 15 May 2024 00:48:38 -0700 Subject: [PATCH] pkg/proc/gdbserial: optimize gdbwire backend (#3715) This change optimizes the gdbwire backend by reducing the number of round trips we have to make to debugserver. It does this by using the jstopinfo packet to only query threads which we know to have a stop reason, and it also uses the registers returned by the 'T' packet to avoid issuing a bunch of 'p' packets to get the register values. --- pkg/proc/gdbserial/gdbserver.go | 72 +++++++++++++++------- pkg/proc/gdbserial/gdbserver_conn.go | 91 +++++++++++++++++++++------- 2 files changed, 117 insertions(+), 46 deletions(-) diff --git a/pkg/proc/gdbserial/gdbserver.go b/pkg/proc/gdbserial/gdbserver.go index 436af81f..131006e0 100644 --- a/pkg/proc/gdbserial/gdbserver.go +++ b/pkg/proc/gdbserial/gdbserver.go @@ -726,7 +726,7 @@ func (p *gdbProcess) initialize(path, cmdline string, debugInfoDirs []string, st } } - err = p.updateThreadList(&threadUpdater{p: p}) + err = p.updateThreadList(&threadUpdater{p: p}, nil) if err != nil { p.conn.conn.Close() p.bi.Close() @@ -877,6 +877,7 @@ func (p *gdbProcess) ContinueOnce(cctx *proc.ContinueOnceContext) (proc.Thread, var trapthread *gdbThread var tu = threadUpdater{p: p} var atstart bool + var trapThreadRegs map[uint64]uint64 continueLoop: for { tu.Reset() @@ -895,7 +896,7 @@ continueLoop: // NOTE: because debugserver will sometimes send two stop packets after a // continue it is important that this is the very first thing we do after // resume(). See comment in threadStopInfo for an explanation. - p.updateThreadList(&tu) + p.updateThreadList(&tu, sp.jstopInfo) trapthread = p.findThreadByStrID(threadID) if trapthread != nil && !p.threadStopInfo { @@ -913,11 +914,16 @@ continueLoop: return nil, proc.StopExited, proc.ErrProcessExited{Pid: p.conn.pid} } if shouldStop { + trapThreadRegs = sp.regs break continueLoop } } p.clearThreadRegisters() + // TODO(deparker): Can we support this optimization for the RR backend? + if p.conn.isDebugserver { + trapthread.reloadRegisters(trapThreadRegs) + } stopReason := proc.StopUnknown if atstart { @@ -1000,7 +1006,6 @@ func (p *gdbProcess) handleThreadSignals(cctx *proc.ContinueOnceContext, trapthr // Unfortunately debugserver can not convert them into signals for the // process so we must stop here. case debugServerTargetExcBadAccess, debugServerTargetExcBadInstruction, debugServerTargetExcArithmetic, debugServerTargetExcEmulation, debugServerTargetExcSoftware, debugServerTargetExcBreakpoint: - trapthreadCandidate = th shouldStop = true @@ -1127,7 +1132,7 @@ func (p *gdbProcess) Restart(cctx *proc.ContinueOnceContext, pos string) (proc.T return nil, err } - err = p.updateThreadList(&threadUpdater{p: p}) + err = p.updateThreadList(&threadUpdater{p: p}, nil) if err != nil { return nil, err } @@ -1405,7 +1410,7 @@ func (tu *threadUpdater) Finish() { // Some stubs will return the list of running threads in the stop packet, if // this happens the threadUpdater will know that we have already updated the // thread list and the first step of updateThreadList will be skipped. -func (p *gdbProcess) updateThreadList(tu *threadUpdater) error { +func (p *gdbProcess) updateThreadList(tu *threadUpdater, jstopInfo map[int]stopPacket) error { if !tu.done { first := true for { @@ -1426,7 +1431,13 @@ func (p *gdbProcess) updateThreadList(tu *threadUpdater) error { } for _, th := range p.threads { - if p.threadStopInfo { + queryThreadInfo := true + if jstopInfo != nil { + // TODO(derekparker): Use jstopInfo directly, if present, instead of + // issuing another stop info request. + _, queryThreadInfo = jstopInfo[th.ID] + } + if p.threadStopInfo && queryThreadInfo { sp, err := p.conn.threadStopInfo(th.strID) if err != nil { if isProtocolErrorUnsupported(err) { @@ -1440,9 +1451,7 @@ func (p *gdbProcess) updateThreadList(tu *threadUpdater) error { th.watchAddr = sp.watchAddr th.watchReg = sp.watchReg } else { - th.sig = 0 - th.watchAddr = 0 - th.watchReg = -1 + th.clearBreakpointState() } } @@ -1537,7 +1546,7 @@ func (t *gdbThread) ThreadID() int { // Registers returns the CPU registers for this thread. func (t *gdbThread) Registers() (proc.Registers, error) { if t.regs.regs == nil { - if err := t.reloadRegisters(); err != nil { + if err := t.reloadRegisters(nil); err != nil { return nil, err } } @@ -1689,25 +1698,42 @@ func (regs *gdbRegisters) gdbRegisterNew(reginfo *gdbRegisterInfo) gdbRegister { // It will also load the address of the thread's G. // Loading the address of G can be done in one of two ways reloadGAlloc, if // the stub can allocate memory, or reloadGAtPC, if the stub can't. -func (t *gdbThread) reloadRegisters() error { +func (t *gdbThread) reloadRegisters(regs map[uint64]uint64) error { if t.regs.regs == nil { t.regs.init(t.p.conn.regsInfo, t.p.bi.Arch, t.p.regnames) } - if t.p.gcmdok { - if err := t.p.conn.readRegisters(t.strID, t.regs.buf); err != nil { - gdberr, isProt := err.(*GdbProtocolError) - if isProtocolErrorUnsupported(err) || (t.p.conn.isDebugserver && isProt && gdberr.code == "E74") { - t.p.gcmdok = false - } else { - return err + if regs == nil { + if t.p.gcmdok { + if err := t.p.conn.readRegisters(t.strID, t.regs.buf); err != nil { + gdberr, isProt := err.(*GdbProtocolError) + if isProtocolErrorUnsupported(err) || (t.p.conn.isDebugserver && isProt && gdberr.code == "E74") { + t.p.gcmdok = false + } else { + return err + } } } - } - if !t.p.gcmdok { - for _, reginfo := range t.p.conn.regsInfo { - if err := t.p.conn.readRegister(t.strID, reginfo.Regnum, t.regs.regs[reginfo.Name].value); err != nil { - return err + if !t.p.gcmdok { + for _, reginfo := range t.p.conn.regsInfo { + if err := t.p.conn.readRegister(t.strID, reginfo.Regnum, t.regs.regs[reginfo.Name].value); err != nil { + return err + } + } + } + } else { + for _, r := range t.p.conn.regsInfo { + if val, ok := regs[uint64(r.Regnum)]; ok { + switch r.Bitsize / 8 { + case 8: + binary.BigEndian.PutUint64(t.regs.regs[r.Name].value, val) + case 4: + binary.BigEndian.PutUint32(t.regs.regs[r.Name].value, uint32(val)) + } + } else { + if err := t.p.conn.readRegister(t.strID, r.Regnum, t.regs.regs[r.Name].value); err != nil { + return err + } } } } diff --git a/pkg/proc/gdbserial/gdbserver_conn.go b/pkg/proc/gdbserial/gdbserver_conn.go index 47295d12..fc9f6aa4 100644 --- a/pkg/proc/gdbserial/gdbserver_conn.go +++ b/pkg/proc/gdbserial/gdbserver_conn.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "debug/macho" + "encoding/hex" "encoding/json" "encoding/xml" "errors" @@ -724,6 +725,16 @@ type stopPacket struct { reason string watchAddr uint64 watchReg int + jstopInfo map[int]stopPacket + regs map[uint64]uint64 +} + +type jsonStopPacket struct { + Tid int `json:"tid"` + Signal int `json:"signal"` + Metype int `json:"metype"` + Medata []uint64 `json:"medata"` + Reason string `json:"reason"` } // Mach exception codes used to decode metype/medata keys in stop packets (necessary to support watchpoints with debugserver). @@ -753,6 +764,7 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd } sp.sig = uint8(sig) sp.watchReg = -1 + sp.regs = make(map[uint64]uint64) if logflags.GdbWire() && gdbWireFullStopPacket { conn.log.Debugf("full stop packet: %s", string(resp)) @@ -761,10 +773,45 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd var metype int medata := make([]uint64, 0, 10) + parseMachException := func(sp *stopPacket, metype int, medata []uint64) { + // Debugserver does not report watchpoint stops in the standard way preferring + // instead the semi-undocumented metype/medata keys. + // These values also have different meanings depending on the CPU architecture. + switch conn.goarch { + case "amd64": + if metype == _EXC_BREAKPOINT && len(medata) >= 2 && medata[0] == _EXC_I386_SGL { + 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 || 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] + } + } + } + } + csp := colonSemicolonParser{buf: resp[3:]} for csp.next() { key, value := csp.key, csp.value + if reg, err := strconv.ParseUint(string(key), 16, 64); err == nil { + // This is a register. + v, err := strconv.ParseUint(string(value), 16, 64) + if err != nil { + return false, stopPacket{}, fmt.Errorf("malformed stop packet: %s", string(resp)) + } + sp.regs[reg] = v + } + switch string(key) { case "thread": sp.threadID = string(value) @@ -787,33 +834,31 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd // mach exception data (debugserver extension) d, _ := strconv.ParseUint(string(value), 16, 64) medata = append(medata, d) - } - } - - // Debugserver does not report watchpoint stops in the standard way preferring - // instead the semi-undocumented metype/medata keys. - // These values also have different meanings depending on the CPU architecture. - switch conn.goarch { - case "amd64": - if metype == _EXC_BREAKPOINT && len(medata) >= 2 && medata[0] == _EXC_I386_SGL { - 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 || 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] + case "jstopinfo": + jstopinfo, err := hex.DecodeString(string(value)) + if err != nil { + return false, stopPacket{}, fmt.Errorf("malformed stop packet: %s (wrong jstopinfo)", string(resp)) + } + parsedJstopInfo := []jsonStopPacket{} + if err := json.Unmarshal(jstopinfo, &parsedJstopInfo); err != nil { + return false, stopPacket{}, fmt.Errorf("malformed stop packet: %s (wrong jstopinfo)", string(resp)) + } + sp.jstopInfo = make(map[int]stopPacket) + for _, jsp := range parsedJstopInfo { + threadID := fmt.Sprintf("%d", jsp.Tid) + newStopPacket := stopPacket{ + threadID: threadID, + reason: jsp.Reason, + sig: uint8(jsp.Signal), + } + parseMachException(&newStopPacket, jsp.Metype, jsp.Medata) + sp.jstopInfo[jsp.Tid] = newStopPacket } } } + parseMachException(&sp, metype, medata) + return false, sp, nil case 'W', 'X':