proc/gdbserver: ignore spurious stop packet from debugserver (#3021)

When we send an interrupt request to debugserver we, sometimes, get one
extra spurious stop packet back.
This stop packet gets interpreted as a response to a qThreadStopInfo request
we make causing the protocol to become desynchronized for a while, until
eventually some kind of error appears.

Here's an example of this problem, distilled from issue #3013:

     1 <- $vCont;c#a8
     2 <- interrupt
     3 -> $T05thread:12efb47;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
     4 <- $qThreadStopInfo12efb8e#28
     5 -> $T05thread:12efb47;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
     6 <- $qThreadStopInfo12efb8f#29
     7 -> $T00thread:12efb8e;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
     8 <- $qThreadStopInfo12efb90#f4
     9 -> $T00thread:12efb8f;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
    10 <- $qThreadStopInfo12efb91#f5
    11 -> $T00thread:12efb90;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
    12 <- $qThreadStopInfo12efb47#f6
    13 -> $T00thread:12efb91;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
    14 <- $qThreadStopInfo12efb8d#27
    15 -> $T05thread:12efb47;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...
    16 <- $p0;thread:12efb8e;#f5
    17 -> $T00thread:12efb8d;threads:12efb47,12efb8d,12efb8e,12efb8f,12efb90,12efb91;thread-pcs:10abe83,7ff81b20e2be,7ff81b20e3ea,...

response (3) is interpreted as the response to the vCont request at (1). We
then make a qThreadStopInfo request (4) and receive a stop packet in
response (5). Packet (5) is interpreted as the response to (4) but it
actually isn't, note how the thread ID is different, packet (5) is actually
a spurious stop packet sent by debug server. From response (5) onward the
protocol is desynchronized, none of the response we process are actually the
response to the preceding request.
This eventually causes a failure at packet (17) which debugserver sent as
the response to request (14) but we interpret as the response to (16).

Fixes #3013
This commit is contained in:
Alessandro Arzilli 2022-06-08 19:56:50 +02:00 committed by GitHub
parent ac8720eb6e
commit 2827145f1e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 5 deletions

@ -849,6 +849,9 @@ continueLoop:
// For stubs that support qThreadStopInfo updateThreadList will
// find out the reason why each thread stopped.
// 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)
trapthread = p.findThreadByStrID(threadID)

@ -256,7 +256,8 @@ func setRegFound(regFound map[string]bool, name string) {
// readTargetXml reads target.xml file from stub using qXfer:features:read,
// then parses it requesting any additional files.
// The schema of target.xml is described by:
// https://github.com/bminor/binutils-gdb/blob/61baf725eca99af2569262d10aca03dcde2698f6/gdb/features/gdb-target.dtd
//
// https://github.com/bminor/binutils-gdb/blob/61baf725eca99af2569262d10aca03dcde2698f6/gdb/features/gdb-target.dtd
func (conn *gdbConn) readTargetXml(regFound map[string]bool) (err error) {
conn.regsInfo, err = conn.readAnnex("target.xml")
if err != nil {
@ -715,9 +716,10 @@ type stopPacket struct {
// Mach exception codes used to decode metype/medata keys in stop packets (necessary to support watchpoints with debugserver).
// See:
// https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/exception_types.h.auto.html
// 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
//
// https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/exception_types.h.auto.html
// 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
@ -1095,6 +1097,36 @@ func (conn *gdbConn) threadStopInfo(threadID string) (sp stopPacket, err error)
if err != nil {
return stopPacket{}, err
}
if sp.threadID != threadID {
// When we send a ^C (manual stop request) and the process is close to
// stopping anyway, sometimes, debugserver will send back two stop
// packets. We need to ignore this spurious stop packet. Because the first
// thing we do after the stop is updateThreadList, which calls this
// function, this is relatively painless. We simply need to check that the
// stop packet we receive is for the thread we requested, if it isn't we
// can assume it is the spurious extra stop packet and simply ignore it.
// An example of a problematic interaction is in the commit message for
// this change.
// See https://github.com/go-delve/delve/issues/3013.
conn.conn.SetReadDeadline(time.Now().Add(10 * time.Millisecond))
resp, err = conn.recv(conn.outbuf.Bytes(), "thread stop info", false)
conn.conn.SetReadDeadline(time.Time{})
if err != nil {
if neterr, isneterr := err.(net.Error); isneterr && neterr.Timeout() {
return stopPacket{}, fmt.Errorf("qThreadStopInfo mismatch, requested %s got %s", sp.threadID, threadID)
}
return stopPacket{}, err
}
_, sp, err = conn.parseStopPacket(resp, "", nil)
if err != nil {
return stopPacket{}, err
}
if sp.threadID != threadID {
return stopPacket{}, fmt.Errorf("qThreadStopInfo mismatch, requested %s got %s", sp.threadID, threadID)
}
}
return sp, nil
}
@ -1246,7 +1278,8 @@ func (conn *gdbConn) memoryRegionInfo(addr uint64) (*memoryRegionInfo, error) {
// exec executes a message to the stub and reads a response.
// The details of the wire protocol are described here:
// https://sourceware.org/gdb/onlinedocs/gdb/Overview.html#Overview
//
// https://sourceware.org/gdb/onlinedocs/gdb/Overview.html#Overview
func (conn *gdbConn) exec(cmd []byte, context string) ([]byte, error) {
if err := conn.send(cmd); err != nil {
return nil, err