[PATCH v2 7/8] winedbg: cache GDB qXfer command result for chunked fetching and define handler table.
Jinoh Kang
jinoh.kang.kr at gmail.com
Mon Nov 1 00:41:19 CDT 2021
GDB does not retrieve the result of a qXfer command at once; instead, it
issues a series of requests to obtain the result one "chunk" at a time,
and concatenates those chunks internally. Each request contains offset
and length variables that specify which portion of the result shall be
retrieved.
Today, Winedbg handles this by generating the entire result data each
time a request is received and slicing out the requested range for the
response. This is not only inefficient due to repeated computation,
but also prone to race condition since the result may change between
successive chunk requests due to the dynamic nature of some commands
such as "libraries" and "threads."
Fix this by cacheing the result into a buffer at the first request, and
use the buffer to serve successive chunk requests. The cache is
invalidated when the remote requests a different object, or the debugger
reaches the end of the result cache buffer.
Also replace the series of sscanf in packet_query with a dedicated qXfer
request parser function and a lookup into a predefined handler table.
Signed-off-by: Jinoh Kang <jinoh.kang.kr at gmail.com>
---
programs/winedbg/gdbproxy.c | 235 ++++++++++++++++++++++++++----------
1 file changed, 174 insertions(+), 61 deletions(-)
diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c
index caf64983c49..19ad76910af 100644
--- a/programs/winedbg/gdbproxy.c
+++ b/programs/winedbg/gdbproxy.c
@@ -61,6 +61,8 @@ struct vl_buffer
size_t alloc;
};
+#define MAX_QX_ANNEX_LEN 32
+
struct gdb_context
{
/* gdb information */
@@ -89,6 +91,8 @@ struct gdb_context
/* Unix environment */
ULONG_PTR wine_segs[3]; /* load addresses of the ELF wine exec segments (text, bss and data) */
BOOL no_ack_mode;
+ int qxfer_object_idx;
+ char qxfer_object_annex[MAX_QX_ANNEX_LEN];
struct vl_buffer qxfer_buffer;
};
@@ -807,7 +811,7 @@ static int addr_width(struct gdb_context* gdbctx)
}
enum packet_return {packet_error = 0x00, packet_ok = 0x01, packet_done = 0x02,
- packet_last_f = 0x80};
+ packet_send_buffer = 0x03, packet_last_f = 0x80};
static void* buffer_realloc(void* buf, size_t size)
{
@@ -1776,11 +1780,16 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO
return TRUE;
}
-static void packet_query_libraries(struct gdb_context* gdbctx)
+static enum packet_return packet_query_libraries(struct gdb_context* gdbctx)
{
struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer;
BOOL opt;
+ if (!gdbctx->process) return packet_error;
+
+ if (gdbctx->qxfer_object_annex[0])
+ return packet_reply_error(gdbctx, 0);
+
/* this will resynchronize builtin dbghelp's internal ELF module list */
SymLoadModule(gdbctx->process->handle, 0, 0, 0, 0, 0);
@@ -1789,14 +1798,21 @@ static void packet_query_libraries(struct gdb_context* gdbctx)
SymEnumerateModules64(gdbctx->process->handle, packet_query_libraries_cb, gdbctx);
SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, opt);
vl_append_str(vlbuf, "</library-list>");
+
+ return packet_send_buffer;
}
-static void packet_query_threads(struct gdb_context* gdbctx)
+static enum packet_return packet_query_threads(struct gdb_context* gdbctx)
{
struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer;
struct dbg_process* process = gdbctx->process;
struct dbg_thread* thread;
+ if (!process) return packet_error;
+
+ if (gdbctx->qxfer_object_annex[0])
+ return packet_reply_error(gdbctx, 0);
+
vl_append_str(vlbuf, "<threads>");
LIST_FOR_EACH_ENTRY(thread, &process->threads, struct dbg_thread, entry)
{
@@ -1808,11 +1824,12 @@ static void packet_query_threads(struct gdb_context* gdbctx)
vl_append_str(vlbuf, "\"/>");
}
vl_append_str(vlbuf, "</threads>");
+
+ return packet_send_buffer;
}
-static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_cpu* cpu)
+static void packet_query_target_xml(struct gdb_context* gdbctx, struct vl_buffer *vlbuf, struct backend_cpu* cpu)
{
- struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer;
const char* feature_prefix = NULL;
const char* feature = NULL;
char buffer[256];
@@ -1926,10 +1943,89 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c
vl_append_str(vlbuf, "</target>");
}
+static enum packet_return packet_query_features(struct gdb_context* gdbctx)
+{
+ struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer;
+ struct dbg_process* process = gdbctx->process;
+
+ if (!process) return packet_error;
+
+ if (strncmp(gdbctx->qxfer_object_annex, "target.xml", MAX_QX_ANNEX_LEN) == 0)
+ {
+ struct backend_cpu *cpu = process->be_cpu;
+ if (!cpu) return packet_error;
+
+ packet_query_target_xml(gdbctx, vlbuf, cpu);
+
+ return packet_send_buffer;
+ }
+
+ return packet_reply_error(gdbctx, 0);
+}
+
+struct qxfer
+{
+ const char* name;
+ enum packet_return (*handler)(struct gdb_context* gdbctx);
+} qxfer_handlers[] =
+{
+ {"libraries", packet_query_libraries},
+ {"threads" , packet_query_threads },
+ {"features" , packet_query_features },
+};
+
+static BOOL parse_xfer_read(const char *ptr, char *name_buf, size_t name_buf_size, char *annex_buf, size_t annex_buf_size, unsigned int *offp, unsigned int *lenp)
+{
+ const char *name, *annex;
+ char *endptr;
+ size_t name_size, annex_size;
+ unsigned long off, len;
+
+ if (strncmp(ptr, "Xfer:", 5) != 0)
+ return FALSE;
+ ptr += 5;
+
+ name = ptr;
+ ptr = strchr(ptr, ':');
+ if (!ptr || (name_size = ptr - name) >= name_buf_size)
+ return FALSE;
+ ptr++;
+
+ if (strncmp(ptr, "read:", 5) != 0)
+ return FALSE;
+ ptr += 5;
+
+ annex = ptr;
+ ptr = strchr(ptr, ':');
+ if (!ptr || (annex_size = ptr - annex) >= annex_buf_size)
+ return FALSE;
+ ptr++;
+
+ off = strtoul(ptr, &endptr, 16);
+ if (endptr == ptr || *endptr != ',' || off > (unsigned int)-1)
+ return FALSE;
+ ptr = endptr + 1;
+
+ len = strtoul(ptr, &endptr, 16);
+ if (endptr == ptr || *endptr != '\0' || len > (unsigned int)-1)
+ return FALSE;
+
+ memcpy(name_buf, name, name_size);
+ memset(name_buf + name_size, '\0', name_buf_size - name_size);
+
+ memcpy(annex_buf, annex, annex_size);
+ memset(annex_buf + annex_size, '\0', annex_buf_size - annex_size);
+
+ *offp = off;
+ *lenp = len;
+
+ return TRUE;
+}
+
static enum packet_return packet_query(struct gdb_context* gdbctx)
{
+ char object_name[32], annex[MAX_QX_ANNEX_LEN];
unsigned int off, len;
- struct backend_cpu *cpu;
switch (gdbctx->in_packet[0])
{
@@ -2016,11 +2112,16 @@ static enum packet_return packet_query(struct gdb_context* gdbctx)
return packet_ok;
if (strncmp(gdbctx->in_packet, "Supported", 9) == 0)
{
+ size_t i;
+
packet_reply_open(gdbctx);
packet_reply_add(gdbctx, "QStartNoAckMode+;");
- packet_reply_add(gdbctx, "qXfer:libraries:read+;");
- packet_reply_add(gdbctx, "qXfer:threads:read+;");
- packet_reply_add(gdbctx, "qXfer:features:read+;");
+ for (i = 0; i < ARRAY_SIZE(qxfer_handlers); i++)
+ {
+ packet_reply_add(gdbctx, "qXfer:");
+ packet_reply_add(gdbctx, qxfer_handlers[i].name);
+ packet_reply_add(gdbctx, ":read+;");
+ }
packet_reply_close(gdbctx);
return packet_done;
}
@@ -2051,59 +2152,69 @@ static enum packet_return packet_query(struct gdb_context* gdbctx)
}
break;
case 'X':
- if (sscanf(gdbctx->in_packet, "Xfer:libraries:read::%x,%x", &off, &len) == 2)
+ if (parse_xfer_read(gdbctx->in_packet,
+ object_name, sizeof(object_name),
+ annex, sizeof(annex),
+ &off, &len))
{
- BOOL more;
-
- if (!gdbctx->process) return packet_error;
-
- vl_empty(&gdbctx->qxfer_buffer);
- packet_query_libraries(gdbctx);
- packet_reply_xfer(
- gdbctx,
- gdbctx->qxfer_buffer.base,
- gdbctx->qxfer_buffer.len,
- off, len, &more
- );
- vl_empty(&gdbctx->qxfer_buffer);
- return packet_done;
- }
-
- if (sscanf(gdbctx->in_packet, "Xfer:threads:read::%x,%x", &off, &len) == 2)
- {
- BOOL more;
-
- if (!gdbctx->process) return packet_error;
-
- vl_empty(&gdbctx->qxfer_buffer);
- packet_query_threads(gdbctx);
- packet_reply_xfer(
- gdbctx,
- gdbctx->qxfer_buffer.base,
- gdbctx->qxfer_buffer.len,
- off, len, &more
- );
- vl_empty(&gdbctx->qxfer_buffer);
- return packet_done;
- }
-
- if (sscanf(gdbctx->in_packet, "Xfer:features:read:target.xml:%x,%x", &off, &len) == 2)
- {
- BOOL more;
-
- if (!gdbctx->process) return packet_error;
- if (!(cpu = gdbctx->process->be_cpu)) return packet_error;
-
- vl_empty(&gdbctx->qxfer_buffer);
- packet_query_target_xml(gdbctx, cpu);
- packet_reply_xfer(
- gdbctx,
- gdbctx->qxfer_buffer.base,
- gdbctx->qxfer_buffer.len,
- off, len, &more
- );
- vl_empty(&gdbctx->qxfer_buffer);
- return packet_done;
+ enum packet_return result;
+ int i;
+
+ for (i = 0; ; i++)
+ {
+ if (i >= ARRAY_SIZE(qxfer_handlers))
+ {
+ ERR("unhandled qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len);
+ return packet_error;
+ }
+ if (strcmp(qxfer_handlers[i].name, object_name) == 0)
+ break;
+ }
+
+ TRACE("qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len);
+
+ if (off > 0 &&
+ gdbctx->qxfer_buffer.len > 0 &&
+ gdbctx->qxfer_object_idx == i &&
+ strncmp(gdbctx->qxfer_object_annex, annex, MAX_QX_ANNEX_LEN) == 0)
+ {
+ result = packet_send_buffer;
+ TRACE("qXfer read result = %d (cached)\n", result);
+ }
+ else
+ {
+ vl_empty(&gdbctx->qxfer_buffer);
+
+ gdbctx->qxfer_object_idx = i;
+ memcpy(gdbctx->qxfer_object_annex, annex, MAX_QX_ANNEX_LEN);
+
+ result = (*qxfer_handlers[i].handler)(gdbctx);
+ TRACE("qXfer read result = %d\n", result);
+ }
+
+ if ((result & ~packet_last_f) == packet_send_buffer)
+ {
+ BOOL more;
+
+ packet_reply_xfer(
+ gdbctx,
+ gdbctx->qxfer_buffer.base,
+ gdbctx->qxfer_buffer.len,
+ off, len, &more
+ );
+ if (!more)
+ vl_empty(&gdbctx->qxfer_buffer);
+
+ result = (result & packet_last_f) | packet_done;
+ }
+ else
+ {
+ gdbctx->qxfer_object_idx = -1;
+ memset(gdbctx->qxfer_object_annex, 0, MAX_QX_ANNEX_LEN);
+ vl_empty(&gdbctx->qxfer_buffer);
+ }
+
+ return result;
}
break;
}
@@ -2419,6 +2530,8 @@ static BOOL gdb_init_context(struct gdb_context* gdbctx, unsigned flags, unsigne
for (i = 0; i < ARRAY_SIZE(gdbctx->wine_segs); i++)
gdbctx->wine_segs[i] = 0;
+ gdbctx->qxfer_object_idx = -1;
+ memset(gdbctx->qxfer_object_annex, 0, sizeof(gdbctx->qxfer_object_annex));
memset(&gdbctx->qxfer_buffer, 0, sizeof(gdbctx->qxfer_buffer));
/* wait for first trap */
--
2.31.1
More information about the wine-devel
mailing list