[PATCH v7 0/7] winedbg(gdbproxy): don't misbehave on module names w/ special characters; modernise
Changelog: v1 -> v2: address review comments so far v2 -> v3: - Change cover letter subject - Capitalise subject - Split qXfer cache patch into two v3 -> v4: - Fix pointer variable declaration style - Synchronize to prior modified patches. - Refactor the code a bit - Revert to scanf() when parsing Xfer packet lines - Fix indentation - s/buffer_realloc/realloc/g - s/vl_buffer/reply_buffer/g; s/vl_/reply_buffer_/g; s/vlbuf/reply/g - Rewrite reply_buffer_add_uinthex(): convert for loop into while loop - Rewrite reply_buffer_append_xmlstr(): convert for loop into while loop - Split packet_reply_add() into packet_reply_add_data() here - Rewrite packet_reply_add for clarity. - Update Wine version in comment. v4 -> v5: - Fix pointer variable declaration style. - Synchronize to prior modified patches. - Split out XML escaping in a separate patch - Don't realloc/free when emptying - Drop reply_buffer_resize(), merge into reply_buffer_grow() - Remove "more" variables for now - Don't empty the reply buffer twice - Use two sscanf for alternative patterns - Don't always zero-initialise qxfer_object_annex - Use strcpy (I just did what I was told to do) - Use strcmp (I just did what I was told to do) v5 -> v6: - Remove a gratuitous blank line - Edit commit messages v6 -> v7: - Change type of reply_buffer::base from void* to unsigned char* - s/reply_buffer_empty/reply_buffer_clear/g - Remove unused function reply_buffer_append_str - Add function reply_buffer_append_str here - packet_reply_xfer: Change type of `off` and `len` parameters to size_t - packet_reply_xfer: Remove data and datalen parameters Jinoh Kang (7): winedbg: Refactor gdb_context::out_{buf*,len} into reply_buffer. winedbg: Use exponential growth in gdbproxy reply_buffer_grow. winedbg: Buffer output of GDB qXfer commands for proper slicing. winedbg: Escape XML special characters in qXfer reply. winedbg: Define table for GDB qXfer command handlers. winedbg: Cache GDB qXfer command result for chunked fetching. winedbg: Implement GDB qXfer object exec-file. programs/winedbg/gdbproxy.c | 604 ++++++++++++++++++++++++------------ 1 file changed, 406 insertions(+), 198 deletions(-) -- 2.31.1
This is required for a subsequent patch that adds buffering for GDB qXfer reply data. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v5 -> v6: - Edit* commit message v6 -> v7: - Change type of reply_buffer::base from void* to unsigned char* - s/reply_buffer_empty/reply_buffer_clear/g - Remove unused function reply_buffer_append_str programs/winedbg/gdbproxy.c | 134 +++++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 54 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index c1d0bda1a41..07a45cd1b15 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -54,6 +54,13 @@ struct gdb_xpoint unsigned int value; }; +struct reply_buffer +{ + unsigned char* base; + size_t len; + size_t alloc; +}; + struct gdb_context { /* gdb information */ @@ -66,9 +73,7 @@ struct gdb_context char* in_packet; int in_packet_len; /* outgoing buffer */ - char* out_buf; - int out_buf_alloc; - int out_len; + struct reply_buffer out_buf; int out_curr_packet; /* generic GDB thread information */ int exec_tid; /* tid used in step & continue */ @@ -224,12 +229,57 @@ static void hex_to(char* dst, const void* src, size_t len) } } -static unsigned char checksum(const char* ptr, int len) +static void reply_buffer_clear(struct reply_buffer* reply) +{ + reply->len = 0; +} + +static void reply_buffer_grow(struct reply_buffer* reply, size_t size) +{ + if (reply->alloc < reply->len + size) + { + reply->alloc = ((reply->len + size) / 32 + 1) * 32; + reply->base = realloc(reply->base, reply->alloc); + } +} + +static void reply_buffer_append(struct reply_buffer* reply, const void* data, size_t size) +{ + reply_buffer_grow(reply, size); + memcpy(reply->base + reply->len, data, size); + reply->len += size; +} + +static inline void reply_buffer_append_hex(struct reply_buffer* reply, const void* src, size_t len) +{ + reply_buffer_grow(reply, len * 2); + hex_to((char *)reply->base + reply->len, src, len); + reply->len += len * 2; +} + +static inline void reply_buffer_append_uinthex(struct reply_buffer* reply, ULONG_PTR val, int len) +{ + char buf[sizeof(ULONG_PTR) * 2], *ptr; + + assert(len <= sizeof(ULONG_PTR)); + + ptr = buf + len * 2; + while (ptr != buf) + { + *--ptr = hex_to0(val & 0x0F); + val >>= 4; + } + + reply_buffer_append(reply, ptr, len * 2); +} + +static unsigned char checksum(const void* data, int len) { unsigned cksum = 0; + const unsigned char* ptr = data; while (len-- > 0) - cksum += (unsigned char)*ptr++; + cksum += *ptr++; return cksum; } @@ -705,20 +755,9 @@ static int addr_width(struct gdb_context* gdbctx) enum packet_return {packet_error = 0x00, packet_ok = 0x01, packet_done = 0x02, packet_last_f = 0x80}; -static void packet_reply_grow(struct gdb_context* gdbctx, size_t size) -{ - if (gdbctx->out_buf_alloc < gdbctx->out_len + size) - { - gdbctx->out_buf_alloc = ((gdbctx->out_len + size) / 32 + 1) * 32; - gdbctx->out_buf = realloc(gdbctx->out_buf, gdbctx->out_buf_alloc); - } -} - static void packet_reply_hex_to(struct gdb_context* gdbctx, const void* src, int len) { - packet_reply_grow(gdbctx, len * 2); - hex_to(&gdbctx->out_buf[gdbctx->out_len], src, len); - gdbctx->out_len += len * 2; + reply_buffer_append_hex(&gdbctx->out_buf, src, len); } static inline void packet_reply_hex_to_str(struct gdb_context* gdbctx, const char* src) @@ -728,15 +767,7 @@ static inline void packet_reply_hex_to_str(struct gdb_context* gdbctx, const cha static void packet_reply_val(struct gdb_context* gdbctx, ULONG_PTR val, int len) { - int i, shift; - - shift = (len - 1) * 8; - packet_reply_grow(gdbctx, len * 2); - for (i = 0; i < len; i++, shift -= 8) - { - gdbctx->out_buf[gdbctx->out_len++] = hex_to0((val >> (shift + 4)) & 0x0F); - gdbctx->out_buf[gdbctx->out_len++] = hex_to0((val >> shift ) & 0x0F); - } + reply_buffer_append_uinthex(&gdbctx->out_buf, val, len); } static const unsigned char gdb_special_chars_lookup_table[4] = { @@ -764,6 +795,7 @@ static inline BOOL is_gdb_special_char(unsigned char val) static void packet_reply_add(struct gdb_context* gdbctx, const char* str) { const unsigned char *ptr = (unsigned char *)str, *curr; + unsigned char esc_seq[2]; while (*ptr) { @@ -772,23 +804,20 @@ static void packet_reply_add(struct gdb_context* gdbctx, const char* str) while (*ptr && !is_gdb_special_char(*ptr)) ptr++; - packet_reply_grow(gdbctx, ptr - curr); - memcpy(&gdbctx->out_buf[gdbctx->out_len], curr, ptr - curr); - gdbctx->out_len += ptr - curr; + reply_buffer_append(&gdbctx->out_buf, curr, ptr - curr); if (!*ptr) break; - packet_reply_grow(gdbctx, 2); - gdbctx->out_buf[gdbctx->out_len++] = 0x7D; - gdbctx->out_buf[gdbctx->out_len++] = 0x20 ^ *ptr++; + esc_seq[0] = 0x7D; + esc_seq[1] = 0x20 ^ *ptr++; + reply_buffer_append(&gdbctx->out_buf, esc_seq, 2); } } static void packet_reply_open(struct gdb_context* gdbctx) { assert(gdbctx->out_curr_packet == -1); - packet_reply_grow(gdbctx, 1); - gdbctx->out_buf[gdbctx->out_len++] = '$'; - gdbctx->out_curr_packet = gdbctx->out_len; + reply_buffer_append(&gdbctx->out_buf, "$", 1); + gdbctx->out_curr_packet = gdbctx->out_buf.len; } static void packet_reply_close(struct gdb_context* gdbctx) @@ -796,10 +825,9 @@ static void packet_reply_close(struct gdb_context* gdbctx) unsigned char cksum; int plen; - plen = gdbctx->out_len - gdbctx->out_curr_packet; - packet_reply_grow(gdbctx, 1); - gdbctx->out_buf[gdbctx->out_len++] = '#'; - cksum = checksum(&gdbctx->out_buf[gdbctx->out_curr_packet], plen); + plen = gdbctx->out_buf.len - gdbctx->out_curr_packet; + reply_buffer_append(&gdbctx->out_buf, "#", 1); + cksum = checksum(gdbctx->out_buf.base + gdbctx->out_curr_packet, plen); packet_reply_hex_to(gdbctx, &cksum, 1); gdbctx->out_curr_packet = -1; } @@ -815,20 +843,20 @@ static void packet_reply_close_xfer(struct gdb_context* gdbctx, unsigned int off int begin = gdbctx->out_curr_packet + 1; int plen; - if (begin + off < gdbctx->out_len) + if (begin + off < gdbctx->out_buf.len) { - gdbctx->out_len -= off; - memmove(gdbctx->out_buf + begin, gdbctx->out_buf + begin + off, gdbctx->out_len); + gdbctx->out_buf.len -= off; + memmove(gdbctx->out_buf.base + begin, gdbctx->out_buf.base + begin + off, gdbctx->out_buf.len); } else { - gdbctx->out_buf[gdbctx->out_curr_packet] = 'l'; - gdbctx->out_len = gdbctx->out_curr_packet + 1; + gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; + gdbctx->out_buf.len = gdbctx->out_curr_packet + 1; } - plen = gdbctx->out_len - begin; - if (plen > len) gdbctx->out_len -= (plen - len); - else gdbctx->out_buf[gdbctx->out_curr_packet] = 'l'; + plen = gdbctx->out_buf.len - begin; + if (plen > len) gdbctx->out_buf.len -= (plen - len); + else gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; packet_reply_close(gdbctx); } @@ -2098,10 +2126,10 @@ static BOOL extract_packets(struct gdb_context* gdbctx) case packet_done: break; } - TRACE("Reply: %s\n", debugstr_an(gdbctx->out_buf, gdbctx->out_len)); - i = send(gdbctx->sock, gdbctx->out_buf, gdbctx->out_len, 0); - assert(i == gdbctx->out_len); - gdbctx->out_len = 0; + TRACE("Reply: %s\n", debugstr_an((char *)gdbctx->out_buf.base, gdbctx->out_buf.len)); + i = send(gdbctx->sock, (char *)gdbctx->out_buf.base, gdbctx->out_buf.len, 0); + assert(i == gdbctx->out_buf.len); + reply_buffer_clear(&gdbctx->out_buf); } else WARN("Ignoring: %s (checksum: %d != %d)\n", debugstr_an(ptr, sum - ptr), @@ -2255,9 +2283,7 @@ static BOOL gdb_init_context(struct gdb_context* gdbctx, unsigned flags, unsigne gdbctx->in_buf = NULL; gdbctx->in_buf_alloc = 0; gdbctx->in_len = 0; - gdbctx->out_buf = NULL; - gdbctx->out_buf_alloc = 0; - gdbctx->out_len = 0; + memset(&gdbctx->out_buf, 0, sizeof(gdbctx->out_buf)); gdbctx->out_curr_packet = -1; gdbctx->exec_tid = -1; -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 07a45cd1b15..120cc84e008 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -236,9 +236,14 @@ static void reply_buffer_clear(struct reply_buffer* reply) static void reply_buffer_grow(struct reply_buffer* reply, size_t size) { - if (reply->alloc < reply->len + size) + size_t required_alloc = reply->len + size; + + if (reply->alloc < required_alloc) { - reply->alloc = ((reply->len + size) / 32 + 1) * 32; + reply->alloc = reply->alloc * 3 / 2; + if (reply->alloc < required_alloc) + reply->alloc = required_alloc; + reply->base = realloc(reply->base, reply->alloc); } } -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Today, gdbproxy reuses the same buffer for both the qXfer reply and the actual GDB packet reply. This worked well, since each byte in the qXfer reply buffer matched 1:1 to each byte in the actual GDB reply packet. Since we escape special characters now, this property no longer holds and a single byte in qXfer reply will take up to two bytes in the GDB reply packet. This causes offsets to shift, preventing the offset/length response slicing (part of GDB protocol) from working correctly. Fix this by writing the qXfer reply data in a separate buffer, and performing slicing out of it. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v3 -> v4: - Fix pointer variable declaration style - Fix indentation - s/buffer_realloc/realloc/g - s/vl_buffer/reply_buffer/g; s/vl_/reply_buffer_/g; s/vlbuf/reply/g - Rewrite reply_buffer_add_uinthex(): convert for loop into while loop - Rewrite reply_buffer_append_xmlstr(): convert for loop into while loop - Split packet_reply_add() into packet_reply_add_data() here v4 -> v5: - Split out XML escaping in a separate patch - Don't realloc/free when emptying - Drop reply_buffer_resize(), merge into reply_buffer_grow() - Remove "more" variables for now - Don't empty buffer before and after v5 -> v6: - Edit commit message - Remove gratuitous blank line v6 -> v7: - Add function reply_buffer_append_str here - packet_reply_xfer: Change type of `off` and `len` parameters to size_t - packet_reply_xfer: Remove data and datalen parameters programs/winedbg/gdbproxy.c | 264 +++++++++++++++++++----------------- 1 file changed, 137 insertions(+), 127 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 120cc84e008..a1ecdea770f 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -87,6 +87,7 @@ 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; + struct reply_buffer qxfer_buffer; }; /* assume standard signal and errno values */ @@ -255,6 +256,11 @@ static void reply_buffer_append(struct reply_buffer* reply, const void* data, si reply->len += size; } +static inline void reply_buffer_append_str(struct reply_buffer* reply, const char* str) +{ + reply_buffer_append(reply, str, strlen(str)); +} + static inline void reply_buffer_append_hex(struct reply_buffer* reply, const void* src, size_t len) { reply_buffer_grow(reply, len * 2); @@ -797,20 +803,20 @@ static inline BOOL is_gdb_special_char(unsigned char val) return gdb_special_chars_lookup_table[val % length] == val; } -static void packet_reply_add(struct gdb_context* gdbctx, const char* str) +static void packet_reply_add_data(struct gdb_context* gdbctx, const void* data, size_t len) { - const unsigned char *ptr = (unsigned char *)str, *curr; + const unsigned char *ptr = data, *end = ptr + len, *curr; unsigned char esc_seq[2]; - while (*ptr) + while (ptr != end) { curr = ptr; - while (*ptr && !is_gdb_special_char(*ptr)) + while (ptr != end && !is_gdb_special_char(*ptr)) ptr++; reply_buffer_append(&gdbctx->out_buf, curr, ptr - curr); - if (!*ptr) break; + if (ptr == end) break; esc_seq[0] = 0x7D; esc_seq[1] = 0x20 ^ *ptr++; @@ -818,6 +824,11 @@ static void packet_reply_add(struct gdb_context* gdbctx, const char* str) } } +static inline void packet_reply_add(struct gdb_context* gdbctx, const char* str) +{ + packet_reply_add_data(gdbctx, str, strlen(str)); +} + static void packet_reply_open(struct gdb_context* gdbctx) { assert(gdbctx->out_curr_packet == -1); @@ -837,35 +848,6 @@ static void packet_reply_close(struct gdb_context* gdbctx) gdbctx->out_curr_packet = -1; } -static void packet_reply_open_xfer(struct gdb_context* gdbctx) -{ - packet_reply_open(gdbctx); - packet_reply_add(gdbctx, "m"); -} - -static void packet_reply_close_xfer(struct gdb_context* gdbctx, unsigned int off, unsigned int len) -{ - int begin = gdbctx->out_curr_packet + 1; - int plen; - - if (begin + off < gdbctx->out_buf.len) - { - gdbctx->out_buf.len -= off; - memmove(gdbctx->out_buf.base + begin, gdbctx->out_buf.base + begin + off, gdbctx->out_buf.len); - } - else - { - gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; - gdbctx->out_buf.len = gdbctx->out_curr_packet + 1; - } - - plen = gdbctx->out_buf.len - begin; - if (plen > len) gdbctx->out_buf.len -= (plen - len); - else gdbctx->out_buf.base[gdbctx->out_curr_packet] = 'l'; - - packet_reply_close(gdbctx); -} - static enum packet_return packet_reply(struct gdb_context* gdbctx, const char* packet) { packet_reply_open(gdbctx); @@ -895,6 +877,28 @@ static inline void packet_reply_register_hex_to(struct gdb_context* gdbctx, dbg_ packet_reply_hex_to(gdbctx, cpu_register_ptr(gdbctx, ctx, idx), cpu_register_map[idx].length); } +static void packet_reply_xfer(struct gdb_context* gdbctx, size_t off, size_t len) +{ + size_t data_len, trunc_len; + + packet_reply_open(gdbctx); + data_len = gdbctx->qxfer_buffer.len; + + /* check if off + len would overflow */ + if (off < data_len && off + len < data_len) + packet_reply_add(gdbctx, "m"); + else + packet_reply_add(gdbctx, "l"); + + if (off < data_len) + { + trunc_len = min(len, data_len - off); + packet_reply_add_data(gdbctx, gdbctx->qxfer_buffer.base + off, trunc_len); + } + + packet_reply_close(gdbctx); +} + /* =============================================== * * P A C K E T H A N D L E R S * * =============================================== * @@ -1602,6 +1606,7 @@ static enum packet_return packet_query_remote_command(struct gdb_context* gdbctx static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVOID ctx) { struct gdb_context* gdbctx = ctx; + struct reply_buffer* reply = &gdbctx->qxfer_buffer; MEMORY_BASIC_INFORMATION mbi; IMAGE_SECTION_HEADER *sec; IMAGE_DOS_HEADER *dos = NULL; @@ -1614,11 +1619,11 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO mod.SizeOfStruct = sizeof(mod); SymGetModuleInfo64(gdbctx->process->handle, base, &mod); - packet_reply_add(gdbctx, "<library name=\""); + reply_buffer_append_str(reply, "<library name=\""); if (strcmp(mod.LoadedImageName, "[vdso].so") == 0) - packet_reply_add(gdbctx, "linux-vdso.so.1"); + reply_buffer_append_str(reply, "linux-vdso.so.1"); else if (mod.LoadedImageName[0] == '/') - packet_reply_add(gdbctx, mod.LoadedImageName); + reply_buffer_append_str(reply, mod.LoadedImageName); else { UNICODE_STRING nt_name; @@ -1633,15 +1638,15 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO if (IsWow64Process(gdbctx->process->handle, &is_wow64) && is_wow64 && (tmp = strstr(unix_path, "system32"))) memcpy(tmp, "syswow64", 8); - packet_reply_add(gdbctx, unix_path); + reply_buffer_append_str(reply, unix_path); } else - packet_reply_add(gdbctx, mod.LoadedImageName); + reply_buffer_append_str(reply, mod.LoadedImageName); HeapFree(GetProcessHeap(), 0, unix_path); RtlFreeUnicodeString(&nt_name); } - packet_reply_add(gdbctx, "\">"); + reply_buffer_append_str(reply, "\">"); size = sizeof(buffer); if (VirtualQueryEx(gdbctx->process->handle, (void *)(UINT_PTR)mod.BaseOfImage, &mbi, sizeof(mbi)) >= sizeof(mbi) && @@ -1672,72 +1677,75 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO for (i = 0; i < max(nth->FileHeader.NumberOfSections, 1); ++i) { if ((char *)(sec + i) >= buffer + size) break; - packet_reply_add(gdbctx, "<segment address=\"0x"); - packet_reply_val(gdbctx, mod.BaseOfImage + sec[i].VirtualAddress, sizeof(ULONG_PTR)); - packet_reply_add(gdbctx, "\"/>"); + reply_buffer_append_str(reply, "<segment address=\"0x"); + reply_buffer_append_uinthex(reply, mod.BaseOfImage + sec[i].VirtualAddress, sizeof(ULONG_PTR)); + reply_buffer_append_str(reply, "\"/>"); } - packet_reply_add(gdbctx, "</library>"); + reply_buffer_append_str(reply, "</library>"); return TRUE; } static void packet_query_libraries(struct gdb_context* gdbctx) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; BOOL opt; /* this will resynchronize builtin dbghelp's internal ELF module list */ SymLoadModule(gdbctx->process->handle, 0, 0, 0, 0, 0); - packet_reply_add(gdbctx, "<library-list>"); + reply_buffer_append_str(reply, "<library-list>"); opt = SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, TRUE); SymEnumerateModules64(gdbctx->process->handle, packet_query_libraries_cb, gdbctx); SymSetExtendedOption(SYMOPT_EX_WINE_NATIVE_MODULES, opt); - packet_reply_add(gdbctx, "</library-list>"); + reply_buffer_append_str(reply, "</library-list>"); } static void packet_query_threads(struct gdb_context* gdbctx) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; struct dbg_process* process = gdbctx->process; struct dbg_thread* thread; - packet_reply_add(gdbctx, "<threads>"); + reply_buffer_append_str(reply, "<threads>"); LIST_FOR_EACH_ENTRY(thread, &process->threads, struct dbg_thread, entry) { - packet_reply_add(gdbctx, "<thread "); - packet_reply_add(gdbctx, "id=\""); - packet_reply_val(gdbctx, thread->tid, 4); - packet_reply_add(gdbctx, "\" name=\""); - packet_reply_add(gdbctx, thread->name); - packet_reply_add(gdbctx, "\"/>"); + reply_buffer_append_str(reply, "<thread "); + reply_buffer_append_str(reply, "id=\""); + reply_buffer_append_uinthex(reply, thread->tid, 4); + reply_buffer_append_str(reply, "\" name=\""); + reply_buffer_append_str(reply, thread->name); + reply_buffer_append_str(reply, "\"/>"); } - packet_reply_add(gdbctx, "</threads>"); + reply_buffer_append_str(reply, "</threads>"); } static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_cpu* cpu) { + struct reply_buffer* reply = &gdbctx->qxfer_buffer; const char* feature_prefix = NULL; const char* feature = NULL; char buffer[256]; int i; - packet_reply_add(gdbctx, "<target>"); + reply_buffer_append_str(reply, "<target>"); switch (cpu->machine) { case IMAGE_FILE_MACHINE_AMD64: - packet_reply_add(gdbctx, "<architecture>i386:x86-64</architecture>"); + reply_buffer_append_str(reply, "<architecture>i386:x86-64</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_I386: - packet_reply_add(gdbctx, "<architecture>i386</architecture>"); + reply_buffer_append_str(reply, "<architecture>i386</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_ARMNT: - packet_reply_add(gdbctx, "<architecture>arm</architecture>"); + reply_buffer_append_str(reply, "<architecture>arm</architecture>"); feature_prefix = "org.gnu.gdb.arm."; break; case IMAGE_FILE_MACHINE_ARM64: - packet_reply_add(gdbctx, "<architecture>aarch64</architecture>"); + reply_buffer_append_str(reply, "<architecture>aarch64</architecture>"); feature_prefix = "org.gnu.gdb.aarch64."; break; } @@ -1746,87 +1754,87 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c { if (cpu->gdb_register_map[i].feature) { - if (feature) packet_reply_add(gdbctx, "</feature>"); + if (feature) reply_buffer_append_str(reply, "</feature>"); feature = cpu->gdb_register_map[i].feature; - packet_reply_add(gdbctx, "<feature name=\""); - if (feature_prefix) packet_reply_add(gdbctx, feature_prefix); - packet_reply_add(gdbctx, feature); - packet_reply_add(gdbctx, "\">"); + reply_buffer_append_str(reply, "<feature name=\""); + if (feature_prefix) reply_buffer_append_str(reply, feature_prefix); + reply_buffer_append_str(reply, feature); + reply_buffer_append_str(reply, "\">"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && strcmp(feature, "core") == 0) - packet_reply_add(gdbctx, "<flags id=\"i386_eflags\" size=\"4\">" - "<field name=\"CF\" start=\"0\" end=\"0\"/>" - "<field name=\"\" start=\"1\" end=\"1\"/>" - "<field name=\"PF\" start=\"2\" end=\"2\"/>" - "<field name=\"AF\" start=\"4\" end=\"4\"/>" - "<field name=\"ZF\" start=\"6\" end=\"6\"/>" - "<field name=\"SF\" start=\"7\" end=\"7\"/>" - "<field name=\"TF\" start=\"8\" end=\"8\"/>" - "<field name=\"IF\" start=\"9\" end=\"9\"/>" - "<field name=\"DF\" start=\"10\" end=\"10\"/>" - "<field name=\"OF\" start=\"11\" end=\"11\"/>" - "<field name=\"NT\" start=\"14\" end=\"14\"/>" - "<field name=\"RF\" start=\"16\" end=\"16\"/>" - "<field name=\"VM\" start=\"17\" end=\"17\"/>" - "<field name=\"AC\" start=\"18\" end=\"18\"/>" - "<field name=\"VIF\" start=\"19\" end=\"19\"/>" - "<field name=\"VIP\" start=\"20\" end=\"20\"/>" - "<field name=\"ID\" start=\"21\" end=\"21\"/>" - "</flags>"); + reply_buffer_append_str(reply, "<flags id=\"i386_eflags\" size=\"4\">" + "<field name=\"CF\" start=\"0\" end=\"0\"/>" + "<field name=\"\" start=\"1\" end=\"1\"/>" + "<field name=\"PF\" start=\"2\" end=\"2\"/>" + "<field name=\"AF\" start=\"4\" end=\"4\"/>" + "<field name=\"ZF\" start=\"6\" end=\"6\"/>" + "<field name=\"SF\" start=\"7\" end=\"7\"/>" + "<field name=\"TF\" start=\"8\" end=\"8\"/>" + "<field name=\"IF\" start=\"9\" end=\"9\"/>" + "<field name=\"DF\" start=\"10\" end=\"10\"/>" + "<field name=\"OF\" start=\"11\" end=\"11\"/>" + "<field name=\"NT\" start=\"14\" end=\"14\"/>" + "<field name=\"RF\" start=\"16\" end=\"16\"/>" + "<field name=\"VM\" start=\"17\" end=\"17\"/>" + "<field name=\"AC\" start=\"18\" end=\"18\"/>" + "<field name=\"VIF\" start=\"19\" end=\"19\"/>" + "<field name=\"VIP\" start=\"20\" end=\"20\"/>" + "<field name=\"ID\" start=\"21\" end=\"21\"/>" + "</flags>"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && strcmp(feature, "sse") == 0) - packet_reply_add(gdbctx, "<vector id=\"v4f\" type=\"ieee_single\" count=\"4\"/>" - "<vector id=\"v2d\" type=\"ieee_double\" count=\"2\"/>" - "<vector id=\"v16i8\" type=\"int8\" count=\"16\"/>" - "<vector id=\"v8i16\" type=\"int16\" count=\"8\"/>" - "<vector id=\"v4i32\" type=\"int32\" count=\"4\"/>" - "<vector id=\"v2i64\" type=\"int64\" count=\"2\"/>" - "<union id=\"vec128\">" - "<field name=\"v4_float\" type=\"v4f\"/>" - "<field name=\"v2_double\" type=\"v2d\"/>" - "<field name=\"v16_int8\" type=\"v16i8\"/>" - "<field name=\"v8_int16\" type=\"v8i16\"/>" - "<field name=\"v4_int32\" type=\"v4i32\"/>" - "<field name=\"v2_int64\" type=\"v2i64\"/>" - "<field name=\"uint128\" type=\"uint128\"/>" - "</union>" - "<flags id=\"i386_mxcsr\" size=\"4\">" - "<field name=\"IE\" start=\"0\" end=\"0\"/>" - "<field name=\"DE\" start=\"1\" end=\"1\"/>" - "<field name=\"ZE\" start=\"2\" end=\"2\"/>" - "<field name=\"OE\" start=\"3\" end=\"3\"/>" - "<field name=\"UE\" start=\"4\" end=\"4\"/>" - "<field name=\"PE\" start=\"5\" end=\"5\"/>" - "<field name=\"DAZ\" start=\"6\" end=\"6\"/>" - "<field name=\"IM\" start=\"7\" end=\"7\"/>" - "<field name=\"DM\" start=\"8\" end=\"8\"/>" - "<field name=\"ZM\" start=\"9\" end=\"9\"/>" - "<field name=\"OM\" start=\"10\" end=\"10\"/>" - "<field name=\"UM\" start=\"11\" end=\"11\"/>" - "<field name=\"PM\" start=\"12\" end=\"12\"/>" - "<field name=\"FZ\" start=\"15\" end=\"15\"/>" - "</flags>"); + reply_buffer_append_str(reply, "<vector id=\"v4f\" type=\"ieee_single\" count=\"4\"/>" + "<vector id=\"v2d\" type=\"ieee_double\" count=\"2\"/>" + "<vector id=\"v16i8\" type=\"int8\" count=\"16\"/>" + "<vector id=\"v8i16\" type=\"int16\" count=\"8\"/>" + "<vector id=\"v4i32\" type=\"int32\" count=\"4\"/>" + "<vector id=\"v2i64\" type=\"int64\" count=\"2\"/>" + "<union id=\"vec128\">" + "<field name=\"v4_float\" type=\"v4f\"/>" + "<field name=\"v2_double\" type=\"v2d\"/>" + "<field name=\"v16_int8\" type=\"v16i8\"/>" + "<field name=\"v8_int16\" type=\"v8i16\"/>" + "<field name=\"v4_int32\" type=\"v4i32\"/>" + "<field name=\"v2_int64\" type=\"v2i64\"/>" + "<field name=\"uint128\" type=\"uint128\"/>" + "</union>" + "<flags id=\"i386_mxcsr\" size=\"4\">" + "<field name=\"IE\" start=\"0\" end=\"0\"/>" + "<field name=\"DE\" start=\"1\" end=\"1\"/>" + "<field name=\"ZE\" start=\"2\" end=\"2\"/>" + "<field name=\"OE\" start=\"3\" end=\"3\"/>" + "<field name=\"UE\" start=\"4\" end=\"4\"/>" + "<field name=\"PE\" start=\"5\" end=\"5\"/>" + "<field name=\"DAZ\" start=\"6\" end=\"6\"/>" + "<field name=\"IM\" start=\"7\" end=\"7\"/>" + "<field name=\"DM\" start=\"8\" end=\"8\"/>" + "<field name=\"ZM\" start=\"9\" end=\"9\"/>" + "<field name=\"OM\" start=\"10\" end=\"10\"/>" + "<field name=\"UM\" start=\"11\" end=\"11\"/>" + "<field name=\"PM\" start=\"12\" end=\"12\"/>" + "<field name=\"FZ\" start=\"15\" end=\"15\"/>" + "</flags>"); } snprintf(buffer, ARRAY_SIZE(buffer), "<reg name=\"%s\" bitsize=\"%Iu\"", cpu->gdb_register_map[i].name, 8 * cpu->gdb_register_map[i].length); - packet_reply_add(gdbctx, buffer); + reply_buffer_append_str(reply, buffer); if (cpu->gdb_register_map[i].type) { - packet_reply_add(gdbctx, " type=\""); - packet_reply_add(gdbctx, cpu->gdb_register_map[i].type); - packet_reply_add(gdbctx, "\""); + reply_buffer_append_str(reply, " type=\""); + reply_buffer_append_str(reply, cpu->gdb_register_map[i].type); + reply_buffer_append_str(reply, "\""); } - packet_reply_add(gdbctx, "/>"); + reply_buffer_append_str(reply, "/>"); } - if (feature) packet_reply_add(gdbctx, "</feature>"); - packet_reply_add(gdbctx, "</target>"); + if (feature) reply_buffer_append_str(reply, "</feature>"); + reply_buffer_append_str(reply, "</target>"); } static enum packet_return packet_query(struct gdb_context* gdbctx) @@ -1958,9 +1966,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) { if (!gdbctx->process) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_libraries(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } @@ -1968,9 +1976,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) { if (!gdbctx->process) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_threads(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } @@ -1979,9 +1987,9 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) if (!gdbctx->process) return packet_error; if (!(cpu = gdbctx->process->be_cpu)) return packet_error; - packet_reply_open_xfer(gdbctx); packet_query_target_xml(gdbctx, cpu); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len); + reply_buffer_clear(&gdbctx->qxfer_buffer); return packet_done; } break; @@ -2299,6 +2307,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; + memset(&gdbctx->qxfer_buffer, 0, sizeof(gdbctx->qxfer_buffer)); + /* wait for first trap */ while (WaitForDebugEvent(&gdbctx->de, INFINITE)) { -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- Signing it off because I don't think it's really important, but the comment here is a bit misleading, if you read overflow as in "integer overflow":
+ /* check if off + len would overflow */ + if (off < data_len && off + len < data_len)
Some dynamic strings (e.g. loaded image paths) may contain XML special characters which breaks parsing. Fix this by escaping all dynamic strings (i.e. character data and attribute values) that go into the XML replies. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: On XML special character escaping code: strcspn() uses lookup tables as well, but as of Wine 6.22 msvcrt!strcspn allocates 1024 bytes (sizeof(BOOL)*256) of table on the stack and populates it on the fly. It would be slower and less cache-friendly than a preallocated, tiny static lookup table. v5 -> v6: Edit commit message programs/winedbg/gdbproxy.c | 63 ++++++++++++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index a1ecdea770f..3d6aa02824e 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -284,6 +284,55 @@ static inline void reply_buffer_append_uinthex(struct reply_buffer* reply, ULONG reply_buffer_append(reply, ptr, len * 2); } +static const unsigned char xml_special_chars_lookup_table[16] = { + /* The characters should be sorted by its value modulo table length. */ + + 0x00, /* NUL */ + 0, + 0x22, /* ": 0010|0010 */ + 0, 0, 0, + 0x26, /* &: 0010|0110 */ + 0x27, /* ': 0010|0111 */ + 0, 0, 0, 0, + 0x3C, /* <: 0011|1100 */ + 0, + 0x3E, /* >: 0011|1110 */ + 0 +}; + +static inline BOOL is_nul_or_xml_special_char(unsigned char val) +{ + const size_t length = ARRAY_SIZE(xml_special_chars_lookup_table); + return xml_special_chars_lookup_table[val % length] == val; +} + +static void reply_buffer_append_xmlstr(struct reply_buffer* reply, const char* str) +{ + const char *ptr = str, *curr; + + for (;;) + { + curr = ptr; + + while (!is_nul_or_xml_special_char((unsigned char)*ptr)) + ptr++; + + reply_buffer_append(reply, curr, ptr - curr); + + switch (*ptr++) + { + case '"': reply_buffer_append_str(reply, """); break; + case '&': reply_buffer_append_str(reply, "&"); break; + case '\'': reply_buffer_append_str(reply, "'"); break; + case '<': reply_buffer_append_str(reply, "<"); break; + case '>': reply_buffer_append_str(reply, ">"); break; + case '\0': + default: + return; + } + } +} + static unsigned char checksum(const void* data, int len) { unsigned cksum = 0; @@ -1621,9 +1670,9 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO reply_buffer_append_str(reply, "<library name=\""); if (strcmp(mod.LoadedImageName, "[vdso].so") == 0) - reply_buffer_append_str(reply, "linux-vdso.so.1"); + reply_buffer_append_xmlstr(reply, "linux-vdso.so.1"); else if (mod.LoadedImageName[0] == '/') - reply_buffer_append_str(reply, mod.LoadedImageName); + reply_buffer_append_xmlstr(reply, mod.LoadedImageName); else { UNICODE_STRING nt_name; @@ -1638,10 +1687,10 @@ static BOOL CALLBACK packet_query_libraries_cb(PCSTR mod_name, DWORD64 base, PVO if (IsWow64Process(gdbctx->process->handle, &is_wow64) && is_wow64 && (tmp = strstr(unix_path, "system32"))) memcpy(tmp, "syswow64", 8); - reply_buffer_append_str(reply, unix_path); + reply_buffer_append_xmlstr(reply, unix_path); } else - reply_buffer_append_str(reply, mod.LoadedImageName); + reply_buffer_append_xmlstr(reply, mod.LoadedImageName); HeapFree(GetProcessHeap(), 0, unix_path); RtlFreeUnicodeString(&nt_name); @@ -1758,8 +1807,8 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c feature = cpu->gdb_register_map[i].feature; reply_buffer_append_str(reply, "<feature name=\""); - if (feature_prefix) reply_buffer_append_str(reply, feature_prefix); - reply_buffer_append_str(reply, feature); + if (feature_prefix) reply_buffer_append_xmlstr(reply, feature_prefix); + reply_buffer_append_xmlstr(reply, feature); reply_buffer_append_str(reply, "\">"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && @@ -1826,7 +1875,7 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c if (cpu->gdb_register_map[i].type) { reply_buffer_append_str(reply, " type=\""); - reply_buffer_append_str(reply, cpu->gdb_register_map[i].type); + reply_buffer_append_xmlstr(reply, cpu->gdb_register_map[i].type); reply_buffer_append_str(reply, "\""); } -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Define a handler lookup table for qXfer commands and use it. This facilitates implementing more qXfer commands and cacheing reply data. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v3 -> v4: - Refactor the code a bit - Revert to scanf() when parsing Xfer packet lines - Fix pointer variable declaration style v4 -> v5: - Remove "more" variables for now - Don't empty the reply buffer twice - Use two sscanf for alternative patterns - Don't always zero-initialise qxfer_object_annex - Use strcpy (I just did what I was told to do) - Use strcmp (I just did what I was told to do) programs/winedbg/gdbproxy.c | 127 +++++++++++++++++++++++++++--------- 1 file changed, 96 insertions(+), 31 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 3d6aa02824e..0f5d9df35b8 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -61,6 +61,9 @@ struct reply_buffer size_t alloc; }; +#define QX_NAME_SIZE 32 +#define QX_ANNEX_SIZE MAX_PATH + struct gdb_context { /* gdb information */ @@ -87,6 +90,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[QX_ANNEX_SIZE]; struct reply_buffer qxfer_buffer; }; @@ -813,7 +818,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 packet_reply_hex_to(struct gdb_context* gdbctx, const void* src, int len) { @@ -1736,11 +1741,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 reply_buffer* reply = &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); @@ -1749,14 +1759,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); reply_buffer_append_str(reply, "</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 reply_buffer* reply = &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); + reply_buffer_append_str(reply, "<threads>"); LIST_FOR_EACH_ENTRY(thread, &process->threads, struct dbg_thread, entry) { @@ -1768,11 +1785,12 @@ static void packet_query_threads(struct gdb_context* gdbctx) reply_buffer_append_str(reply, "\"/>"); } reply_buffer_append_str(reply, "</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 reply_buffer* reply, struct backend_cpu* cpu) { - struct reply_buffer* reply = &gdbctx->qxfer_buffer; const char* feature_prefix = NULL; const char* feature = NULL; char buffer[256]; @@ -1886,10 +1904,41 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c reply_buffer_append_str(reply, "</target>"); } +static enum packet_return packet_query_features(struct gdb_context* gdbctx) +{ + struct reply_buffer* reply = &gdbctx->qxfer_buffer; + struct dbg_process* process = gdbctx->process; + + if (!process) return packet_error; + + if (strncmp(gdbctx->qxfer_object_annex, "target.xml", QX_ANNEX_SIZE) == 0) + { + struct backend_cpu *cpu = process->be_cpu; + if (!cpu) return packet_error; + + packet_query_target_xml(gdbctx, reply, 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 enum packet_return packet_query(struct gdb_context* gdbctx) { + char object_name[QX_NAME_SIZE], annex[QX_ANNEX_SIZE]; unsigned int off, len; - struct backend_cpu *cpu; switch (gdbctx->in_packet[0]) { @@ -1976,11 +2025,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; } @@ -2011,35 +2065,44 @@ 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) + annex[0] = '\0'; + if (sscanf(gdbctx->in_packet, "Xfer:%31[^:]:read::%x,%x", object_name, &off, &len) == 3 || + sscanf(gdbctx->in_packet, "Xfer:%31[^:]:read:%255[^:]:%x,%x", object_name, annex, &off, &len) == 4) { - if (!gdbctx->process) return packet_error; + enum packet_return result; + int i; - packet_query_libraries(gdbctx); - packet_reply_xfer(gdbctx, off, len); - reply_buffer_clear(&gdbctx->qxfer_buffer); - return packet_done; - } + for (i = 0; i < ARRAY_SIZE(qxfer_handlers); i++) + { + if (strcmp(qxfer_handlers[i].name, object_name) == 0) + break; + } - if (sscanf(gdbctx->in_packet, "Xfer:threads:read::%x,%x", &off, &len) == 2) - { - if (!gdbctx->process) return packet_error; + 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; + } - packet_query_threads(gdbctx); - packet_reply_xfer(gdbctx, off, len); - reply_buffer_clear(&gdbctx->qxfer_buffer); - return packet_done; - } + TRACE("qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len); - if (sscanf(gdbctx->in_packet, "Xfer:features:read:target.xml:%x,%x", &off, &len) == 2) - { - if (!gdbctx->process) return packet_error; - if (!(cpu = gdbctx->process->be_cpu)) return packet_error; + gdbctx->qxfer_object_idx = i; + strcpy(gdbctx->qxfer_object_annex, annex); - packet_query_target_xml(gdbctx, cpu); - packet_reply_xfer(gdbctx, off, len); + result = (*qxfer_handlers[i].handler)(gdbctx); + TRACE("qXfer read result = %d\n", result); + + if ((result & ~packet_last_f) == packet_send_buffer) + { + packet_reply_xfer(gdbctx, off, len); + result = (result & packet_last_f) | packet_done; + } + + gdbctx->qxfer_object_idx = -1; + gdbctx->qxfer_object_annex[0] = '\0'; reply_buffer_clear(&gdbctx->qxfer_buffer); - return packet_done; + + return result; } break; } @@ -2356,6 +2419,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
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
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. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v3 -> v4: - Refactor the code a bit - Revert to scanf() when parsing Xfer packet lines - Fix pointer variable declaration style v3 -> v4: Synchronize to prior modified patches. v4 -> v5: - Synchronize to prior modified patches. - Fix pointer variable declaration style. programs/winedbg/gdbproxy.c | 42 ++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 0f5d9df35b8..c21fed43b48 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -931,15 +931,17 @@ static inline void packet_reply_register_hex_to(struct gdb_context* gdbctx, dbg_ packet_reply_hex_to(gdbctx, cpu_register_ptr(gdbctx, ctx, idx), cpu_register_map[idx].length); } -static void packet_reply_xfer(struct gdb_context* gdbctx, size_t off, size_t len) +static void packet_reply_xfer(struct gdb_context* gdbctx, size_t off, size_t len, BOOL* more_p) { + BOOL more; size_t data_len, trunc_len; packet_reply_open(gdbctx); data_len = gdbctx->qxfer_buffer.len; /* check if off + len would overflow */ - if (off < data_len && off + len < data_len) + more = off < data_len && off + len < data_len; + if (more) packet_reply_add(gdbctx, "m"); else packet_reply_add(gdbctx, "l"); @@ -951,6 +953,8 @@ static void packet_reply_xfer(struct gdb_context* gdbctx, size_t off, size_t len } packet_reply_close(gdbctx); + + *more_p = more; } /* =============================================== * @@ -2071,6 +2075,7 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) { enum packet_return result; int i; + BOOL more; for (i = 0; i < ARRAY_SIZE(qxfer_handlers); i++) { @@ -2086,21 +2091,38 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) TRACE("qXfer %s read %s %u,%u\n", debugstr_a(object_name), debugstr_a(annex), off, len); - gdbctx->qxfer_object_idx = i; - strcpy(gdbctx->qxfer_object_annex, annex); + if (off > 0 && + gdbctx->qxfer_buffer.len > 0 && + gdbctx->qxfer_object_idx == i && + strcmp(gdbctx->qxfer_object_annex, annex) == 0) + { + result = packet_send_buffer; + TRACE("qXfer read result = %d (cached)\n", result); + } + else + { + reply_buffer_clear(&gdbctx->qxfer_buffer); - result = (*qxfer_handlers[i].handler)(gdbctx); - TRACE("qXfer read result = %d\n", result); + gdbctx->qxfer_object_idx = i; + strcpy(gdbctx->qxfer_object_annex, annex); + result = (*qxfer_handlers[i].handler)(gdbctx); + TRACE("qXfer read result = %d\n", result); + } + + more = FALSE; if ((result & ~packet_last_f) == packet_send_buffer) { - packet_reply_xfer(gdbctx, off, len); + packet_reply_xfer(gdbctx, off, len, &more); result = (result & packet_last_f) | packet_done; } - gdbctx->qxfer_object_idx = -1; - gdbctx->qxfer_object_annex[0] = '\0'; - reply_buffer_clear(&gdbctx->qxfer_buffer); + if (!more) + { + gdbctx->qxfer_object_idx = -1; + gdbctx->qxfer_object_annex[0] = '\0'; + reply_buffer_clear(&gdbctx->qxfer_buffer); + } return result; } -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
Today, when gdbproxy is started with --no-start mode, GDB fails to recognise the symbol file unless the `file` command or the `sharedlibrary` command is explicitly issued. Also, RHEL's downstream GDB complains with the following message: Remote gdbserver does not support determining executable automatically. RHEL <=6.8 and <=7.2 versions of gdbserver do not support such automatic executable detection. The following versions of gdbserver support it: - Upstream version of gdbserver (unsupported) 7.10 or later - Red Hat Developer Toolset (DTS) version of gdbserver from DTS 4.0 or later (only on x86_64) - RHEL-7.3 versions of gdbserver (on any architecture) Fix this by implementing the qXfer object "exec-file". Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- Notes: v3 -> v4: Fix pointer variable declaration style v4 -> v5: (no changes) v5 -> v6: Edit commit message programs/winedbg/gdbproxy.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index c21fed43b48..37d7c8ffc66 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -98,7 +98,10 @@ struct gdb_context /* assume standard signal and errno values */ enum host_error { + HOST_EPERM = 1, + HOST_ENOENT = 2, HOST_ESRCH = 3, + HOST_ENOMEM = 12, HOST_EFAULT = 14, HOST_EINVAL = 22, }; @@ -1928,6 +1931,33 @@ static enum packet_return packet_query_features(struct gdb_context* gdbctx) return packet_reply_error(gdbctx, 0); } +static enum packet_return packet_query_exec_file(struct gdb_context* gdbctx) +{ + struct reply_buffer* reply = &gdbctx->qxfer_buffer; + struct dbg_process* process = gdbctx->process; + char *unix_path; + BOOL is_wow64; + char *tmp; + + if (!process) return packet_error; + + if (gdbctx->qxfer_object_annex[0] || !process->imageName) + return packet_reply_error(gdbctx, HOST_EPERM); + + if (!(unix_path = wine_get_unix_file_name(process->imageName))) + return packet_reply_error(gdbctx, GetLastError() == ERROR_NOT_ENOUGH_MEMORY ? HOST_ENOMEM : HOST_ENOENT); + + if (IsWow64Process(process->handle, &is_wow64) && + is_wow64 && (tmp = strstr(unix_path, "system32"))) + memcpy(tmp, "syswow64", 8); + + reply_buffer_append_str(reply, unix_path); + + HeapFree(GetProcessHeap(), 0, unix_path); + + return packet_send_buffer; +} + struct qxfer { const char* name; @@ -1937,6 +1967,7 @@ struct qxfer {"libraries", packet_query_libraries}, {"threads" , packet_query_threads }, {"features" , packet_query_features }, + {"exec-file", packet_query_exec_file}, }; static enum packet_return packet_query(struct gdb_context* gdbctx) -- 2.31.1
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com>
participants (2)
-
Jinoh Kang -
Rémi Bernon