November 2021 - Wine-Devel - list.winehq.org

[PATCH v2 7/8] winedbg: cache GDB qXfer command result for chunked fetching and define handler table.

by Jinoh Kang

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(a)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

Nov. 1, 2021

[PATCH v2 6/8] winedbg: buffer and escape output of GDB qXfer commands.

by Jinoh Kang

Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 284 +++++++++++++++++++++++++++--------- 1 file changed, 211 insertions(+), 73 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index bdb73659ade..caf64983c49 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -54,6 +54,13 @@ struct gdb_xpoint unsigned int value; }; +struct vl_buffer +{ + void *base; + size_t len; + size_t alloc; +}; + struct gdb_context { /* gdb information */ @@ -82,6 +89,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 vl_buffer qxfer_buffer; }; /* assume standard signal and errno values */ @@ -224,6 +232,102 @@ static void hex_to(char* dst, const void* src, size_t len) } } +static void* buffer_realloc(void* buf, size_t size); + +static void vl_resize(struct vl_buffer* vlbuf, size_t alloc) +{ + vlbuf->alloc = alloc; + vlbuf->base = buffer_realloc(vlbuf->base, vlbuf->alloc); +} + +static void vl_empty(struct vl_buffer *vlbuf) +{ + vlbuf->len = 0; + vl_resize(vlbuf, 0); +} + +static void vl_grow(struct vl_buffer* vlbuf, size_t size) +{ + if (vlbuf->alloc < vlbuf->len + size) + vl_resize(vlbuf, ((vlbuf->len + size) / 32 + 1) * 32); +} + +static void vl_append(struct vl_buffer* vlbuf, const void *data, size_t size) +{ + vl_grow(vlbuf, size); + memcpy((void *)((unsigned char *)vlbuf->base + vlbuf->len), data, size); + vlbuf->len += size; +} + +static inline void vl_append_str(struct vl_buffer* vlbuf, const char* str) +{ + vl_append(vlbuf, (const void *)str, strlen(str)); +} + +static inline void vl_append_uinthex(struct vl_buffer* vlbuf, ULONG_PTR val, int len) +{ + char buf[sizeof(ULONG_PTR) * 2], *ptr; + + assert(len <= sizeof(ULONG_PTR)); + + for (ptr = buf + len * 2; ptr != buf; val >>= 4) + *--ptr = hex_to0(val & 0x0F); + + vl_append(vlbuf, 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) +{ + /* Note: strcspn() uses lookup tables as well, but as of Wine 6.19 + * 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. + */ + + const size_t length = ARRAY_SIZE(xml_special_chars_lookup_table); + return xml_special_chars_lookup_table[val % length] == val; +} + +static void vl_append_xmlstr(struct vl_buffer* vlbuf, const char *str) +{ + const char *ptr = str, *sptr; + + for (;;) + { + for (sptr = ptr; !is_nul_or_xml_special_char((unsigned char)*sptr); sptr++) + ; + + vl_append(vlbuf, ptr, sptr - ptr); + ptr = sptr; + + switch (*ptr++) + { + case '"': vl_append_str(vlbuf, """); break; + case '&': vl_append_str(vlbuf, "&"); break; + case '\'': vl_append_str(vlbuf, "'"); break; + case '<': vl_append_str(vlbuf, "<"); break; + case '>': vl_append_str(vlbuf, ">"); break; + default: return; + } + } +} + static unsigned char checksum(const char* ptr, int len) { unsigned cksum = 0; @@ -833,34 +937,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_len) - { - gdbctx->out_len -= off; - memmove(gdbctx->out_buf + begin, gdbctx->out_buf + begin + off, gdbctx->out_len); - } - else - { - gdbctx->out_buf[gdbctx->out_curr_packet] = 'l'; - gdbctx->out_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'; - - packet_reply_close(gdbctx); -} static enum packet_return packet_reply(struct gdb_context* gdbctx, const char* packet) { @@ -891,6 +967,38 @@ 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, + const void *data, + size_t datalen, + unsigned int off, + unsigned int len, + BOOL *more_p +) +{ + BOOL nonempty, more; + size_t trunc_len; + + packet_reply_open(gdbctx); + + nonempty = (size_t)off < datalen; + more = nonempty && (size_t)off + len < datalen; + if (more) + packet_reply_add(gdbctx, "m"); + else + packet_reply_add(gdbctx, "l"); + + if (nonempty) + { + trunc_len = min((size_t)len, datalen - off); + packet_reply_add_raw(gdbctx, (const unsigned char *)data + off, trunc_len); + } + + packet_reply_close(gdbctx); + + *more_p = more; +} + /* =============================================== * * P A C K E T H A N D L E R S * * =============================================== * @@ -1587,6 +1695,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 vl_buffer* vlbuf = &gdbctx->qxfer_buffer; MEMORY_BASIC_INFORMATION mbi; IMAGE_SECTION_HEADER *sec; IMAGE_DOS_HEADER *dos = NULL; @@ -1599,11 +1708,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=\""); + vl_append_str(vlbuf, "<library name=\""); if (strcmp(mod.LoadedImageName, "[vdso].so") == 0) - packet_reply_add(gdbctx, "linux-vdso.so.1"); + vl_append_xmlstr(vlbuf, "linux-vdso.so.1"); else if (mod.LoadedImageName[0] == '/') - packet_reply_add(gdbctx, mod.LoadedImageName); + vl_append_xmlstr(vlbuf, mod.LoadedImageName); else { UNICODE_STRING nt_name; @@ -1618,15 +1727,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); + vl_append_xmlstr(vlbuf, unix_path); } else - packet_reply_add(gdbctx, mod.LoadedImageName); + vl_append_xmlstr(vlbuf, mod.LoadedImageName); HeapFree(GetProcessHeap(), 0, unix_path); RtlFreeUnicodeString(&nt_name); } - packet_reply_add(gdbctx, "\">"); + vl_append_str(vlbuf, "\">"); size = sizeof(buffer); if (VirtualQueryEx(gdbctx->process->handle, (void *)(UINT_PTR)mod.BaseOfImage, &mbi, sizeof(mbi)) >= sizeof(mbi) && @@ -1657,72 +1766,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, "\"/>"); + vl_append_str(vlbuf, "<segment address=\"0x"); + vl_append_uinthex(vlbuf, mod.BaseOfImage + sec[i].VirtualAddress, sizeof(ULONG_PTR)); + vl_append_str(vlbuf, "\"/>"); } - packet_reply_add(gdbctx, "</library>"); + vl_append_str(vlbuf, "</library>"); return TRUE; } static void packet_query_libraries(struct gdb_context* gdbctx) { + struct vl_buffer *vlbuf = &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>"); + vl_append_str(vlbuf, "<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>"); + vl_append_str(vlbuf, "</library-list>"); } static void packet_query_threads(struct gdb_context* gdbctx) { + struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer; struct dbg_process* process = gdbctx->process; struct dbg_thread* thread; - packet_reply_add(gdbctx, "<threads>"); + vl_append_str(vlbuf, "<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, "\"/>"); + vl_append_str(vlbuf, "<thread "); + vl_append_str(vlbuf, "id=\""); + vl_append_uinthex(vlbuf, thread->tid, 4); + vl_append_str(vlbuf, "\" name=\""); + vl_append_str(vlbuf, thread->name); + vl_append_str(vlbuf, "\"/>"); } - packet_reply_add(gdbctx, "</threads>"); + vl_append_str(vlbuf, "</threads>"); } static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_cpu* cpu) { + struct vl_buffer *vlbuf = &gdbctx->qxfer_buffer; const char* feature_prefix = NULL; const char* feature = NULL; char buffer[256]; int i; - packet_reply_add(gdbctx, "<target>"); + vl_append_str(vlbuf, "<target>"); switch (cpu->machine) { case IMAGE_FILE_MACHINE_AMD64: - packet_reply_add(gdbctx, "<architecture>i386:x86-64</architecture>"); + vl_append_str(vlbuf, "<architecture>i386:x86-64</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_I386: - packet_reply_add(gdbctx, "<architecture>i386</architecture>"); + vl_append_str(vlbuf, "<architecture>i386</architecture>"); feature_prefix = "org.gnu.gdb.i386."; break; case IMAGE_FILE_MACHINE_ARMNT: - packet_reply_add(gdbctx, "<architecture>arm</architecture>"); + vl_append_str(vlbuf, "<architecture>arm</architecture>"); feature_prefix = "org.gnu.gdb.arm."; break; case IMAGE_FILE_MACHINE_ARM64: - packet_reply_add(gdbctx, "<architecture>aarch64</architecture>"); + vl_append_str(vlbuf, "<architecture>aarch64</architecture>"); feature_prefix = "org.gnu.gdb.aarch64."; break; } @@ -1731,17 +1843,17 @@ 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) vl_append_str(vlbuf, "</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, "\">"); + vl_append_str(vlbuf, "<feature name=\""); + if (feature_prefix) vl_append_str(vlbuf, feature_prefix); + vl_append_str(vlbuf, feature); + vl_append_str(vlbuf, "\">"); if (strcmp(feature_prefix, "org.gnu.gdb.i386.") == 0 && strcmp(feature, "core") == 0) - packet_reply_add(gdbctx, "<flags id=\"i386_eflags\" size=\"4\">" + vl_append_str(vlbuf, "<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\"/>" @@ -1763,7 +1875,7 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c 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\"/>" + vl_append_str(vlbuf, "<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\"/>" @@ -1798,20 +1910,20 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c 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); + vl_append_str(vlbuf, 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, "\""); + vl_append_str(vlbuf, " type=\""); + vl_append_str(vlbuf, cpu->gdb_register_map[i].type); + vl_append_str(vlbuf, "\""); } - packet_reply_add(gdbctx, "/>"); + vl_append_str(vlbuf, "/>"); } - if (feature) packet_reply_add(gdbctx, "</feature>"); - packet_reply_add(gdbctx, "</target>"); + if (feature) vl_append_str(vlbuf, "</feature>"); + vl_append_str(vlbuf, "</target>"); } static enum packet_return packet_query(struct gdb_context* gdbctx) @@ -1941,32 +2053,56 @@ static enum packet_return packet_query(struct gdb_context* gdbctx) case 'X': if (sscanf(gdbctx->in_packet, "Xfer:libraries:read::%x,%x", &off, &len) == 2) { + BOOL more; + if (!gdbctx->process) return packet_error; - packet_reply_open_xfer(gdbctx); + vl_empty(&gdbctx->qxfer_buffer); packet_query_libraries(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + 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; - packet_reply_open_xfer(gdbctx); + vl_empty(&gdbctx->qxfer_buffer); packet_query_threads(gdbctx); - packet_reply_close_xfer(gdbctx, off, len); + 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; - packet_reply_open_xfer(gdbctx); + vl_empty(&gdbctx->qxfer_buffer); packet_query_target_xml(gdbctx, cpu); - packet_reply_close_xfer(gdbctx, off, len); + packet_reply_xfer( + gdbctx, + gdbctx->qxfer_buffer.base, + gdbctx->qxfer_buffer.len, + off, len, &more + ); + vl_empty(&gdbctx->qxfer_buffer); return packet_done; } break; @@ -2283,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; + memset(&gdbctx->qxfer_buffer, 0, sizeof(gdbctx->qxfer_buffer)); + /* wait for first trap */ while (WaitForDebugEvent(&gdbctx->de, INFINITE)) { -- 2.31.1

Nov. 1, 2021

[PATCH v2 5/8] winedbg: escape special characters in GDB packet reply.

by Jinoh Kang

There are four special characters in GDB's remote serial protocol: - '$' (0x24): start of packet - '}' (0x7D): escape - '*' (0x2A): run-length encoding repeat count delimiter - '#' (0x23): end of packet; start of checksum In particular, the '#' and '}' characters are problematic since they are often used in library filenames. A few examples: - %SystemRoot%\assembly\NativeImages_v[.NET ver]\[module+hash]#\*\*.dll - {CLSID or UUID}\*\.dll To make GDB happy with those filenames, we scan for those characters and escape them properly. While we are at it, also remove the assert in the packet_reply function that checks for '$' and '#' in the packet payload. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 68 ++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index e6b1d998f7d..bdb73659ade 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -752,18 +752,71 @@ static void packet_reply_val(struct gdb_context* gdbctx, ULONG_PTR val, int len) } } +static const unsigned char gdb_special_chars_lookup_table[4] = { + /* The characters should be sorted by its value modulo table length. */ + + 0x24, /* $: 001001|00 */ + 0x7D, /* }: 011111|01 */ + 0x2A, /* *: 001010|10 */ + 0x23 /* #: 001000|11 */ +}; + +static inline BOOL is_gdb_special_char(unsigned char val) +{ + /* A note on the GDB special character scanning code: + * + * We cannot use strcspn() since we plan to transmit binary data in + * packet reply, which can contain NULL (0x00) bytes. We also don't want + * to slow down memory dump transfers. Therefore, we use a tiny lookup + * table that contains all the four special characters to speed up scanning. + * + * Note: strcspn() uses lookup tables as well, but as of Wine 6.19 + * 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. + */ + + const size_t length = ARRAY_SIZE(gdb_special_chars_lookup_table); + return gdb_special_chars_lookup_table[val % length] == val; +} + +static void packet_reply_add_raw(struct gdb_context* gdbctx, const void* data, size_t len) +{ + const unsigned char *ptr = data, *sptr, *end = ptr + len; + size_t seg_len; + int is_end; + + while (ptr != end) + { + for (sptr = ptr; sptr != end && !is_gdb_special_char(*sptr); sptr++) + ; + + seg_len = sptr - ptr; + is_end = sptr == end; + + packet_reply_grow(gdbctx, is_end ? seg_len : seg_len + 2); + memcpy(&gdbctx->out_buf[gdbctx->out_len], ptr, seg_len); + gdbctx->out_len += seg_len; + ptr += seg_len; + + if (is_end) + break; + + gdbctx->out_buf[gdbctx->out_len++] = 0x7D; + gdbctx->out_buf[gdbctx->out_len++] = 0x20 ^ *ptr++; + } +} + static inline void packet_reply_add(struct gdb_context* gdbctx, const char* str) { - int len = strlen(str); - packet_reply_grow(gdbctx, len); - memcpy(&gdbctx->out_buf[gdbctx->out_len], str, len); - gdbctx->out_len += len; + packet_reply_add_raw(gdbctx, str, strlen(str)); } static void packet_reply_open(struct gdb_context* gdbctx) { assert(gdbctx->out_curr_packet == -1); - packet_reply_add(gdbctx, "$"); + packet_reply_grow(gdbctx, 1); + gdbctx->out_buf[gdbctx->out_len++] = '$'; gdbctx->out_curr_packet = gdbctx->out_len; } @@ -773,7 +826,8 @@ static void packet_reply_close(struct gdb_context* gdbctx) int plen; plen = gdbctx->out_len - gdbctx->out_curr_packet; - packet_reply_add(gdbctx, "#"); + packet_reply_grow(gdbctx, 1); + gdbctx->out_buf[gdbctx->out_len++] = '#'; cksum = checksum(&gdbctx->out_buf[gdbctx->out_curr_packet], plen); packet_reply_hex_to(gdbctx, &cksum, 1); gdbctx->out_curr_packet = -1; @@ -812,8 +866,6 @@ static enum packet_return packet_reply(struct gdb_context* gdbctx, const char* p { packet_reply_open(gdbctx); - assert(strchr(packet, '$') == NULL && strchr(packet, '#') == NULL); - packet_reply_add(gdbctx, packet); packet_reply_close(gdbctx); -- 2.31.1

Nov. 1, 2021

[PATCH v2 4/8] winedbg: Use unsigned int for offset/length in GDB qXfer handler.

by Jinoh Kang

packet_query uses sscanf format "%x" to parse out offset and length values. Since %x corresponds to unsigned int in the C standard, adjust the variable types appropriately. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index db0b8d77ba9..e6b1d998f7d 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -785,7 +785,7 @@ static void packet_reply_open_xfer(struct gdb_context* gdbctx) packet_reply_add(gdbctx, "m"); } -static void packet_reply_close_xfer(struct gdb_context* gdbctx, int off, int len) +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; @@ -802,7 +802,7 @@ static void packet_reply_close_xfer(struct gdb_context* gdbctx, int off, int len } plen = gdbctx->out_len - begin; - if (len >= 0 && plen > len) gdbctx->out_len -= (plen - len); + if (plen > len) gdbctx->out_len -= (plen - len); else gdbctx->out_buf[gdbctx->out_curr_packet] = 'l'; packet_reply_close(gdbctx); @@ -1764,7 +1764,7 @@ static void packet_query_target_xml(struct gdb_context* gdbctx, struct backend_c static enum packet_return packet_query(struct gdb_context* gdbctx) { - int off, len; + unsigned int off, len; struct backend_cpu *cpu; switch (gdbctx->in_packet[0]) -- 2.31.1

Nov. 1, 2021

[PATCH v2 3/8] winedbg: generalise packet_realloc(), rename to buffer_realloc().

by Jinoh Kang

Make it return void * and accept size_t as the size. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 3f5584633e4..db0b8d77ba9 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -705,7 +705,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}; -static char* packet_realloc(char* buf, int size) +static void* buffer_realloc(void* buf, size_t size) { if (!size) { @@ -723,7 +723,7 @@ 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 = packet_realloc(gdbctx->out_buf, gdbctx->out_buf_alloc); + gdbctx->out_buf = buffer_realloc(gdbctx->out_buf, gdbctx->out_buf_alloc); } } @@ -2090,7 +2090,7 @@ static int fetch_data(struct gdb_context* gdbctx) { #define STEP 128 if (gdbctx->in_len + STEP > gdbctx->in_buf_alloc) - gdbctx->in_buf = packet_realloc(gdbctx->in_buf, gdbctx->in_buf_alloc += STEP); + gdbctx->in_buf = buffer_realloc(gdbctx->in_buf, gdbctx->in_buf_alloc += STEP); #undef STEP len = recv(gdbctx->sock, gdbctx->in_buf + gdbctx->in_len, gdbctx->in_buf_alloc - gdbctx->in_len - 1, 0); if (len <= 0) break; -- 2.31.1

Nov. 1, 2021

[PATCH v2 2/8] winedbg: free buffer and return NULL if size==0 in packet_realloc().

by Jinoh Kang

packet_realloc() does not return on error, so we can use NULL for normal circumstances. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index deb6b0973f5..3f5584633e4 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -707,6 +707,12 @@ enum packet_return {packet_error = 0x00, packet_ok = 0x01, packet_done = 0x02, static char* packet_realloc(char* buf, int size) { + if (!size) + { + if (buf) + HeapFree(GetProcessHeap(), 0, buf); + return NULL; + } if (!buf) return HeapAlloc(GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, size); return HeapReAlloc(GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, buf, size); -- 2.31.1

Nov. 1, 2021

[PATCH v2 1/8] winedbg: crash when allocation fails in packet_realloc().

by Jinoh Kang

Today, every caller of packet_realloc simply assumes that the allocation always succeeds. *Not* crashing outright on allocation failure makes diagnosing problems more difficult. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- programs/winedbg/gdbproxy.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index 522e4fdb506..deb6b0973f5 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -708,9 +708,8 @@ enum packet_return {packet_error = 0x00, packet_ok = 0x01, packet_done = 0x02, static char* packet_realloc(char* buf, int size) { if (!buf) - return HeapAlloc(GetProcessHeap(), 0, size); - return HeapReAlloc(GetProcessHeap(), 0, buf, size); - + return HeapAlloc(GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, size); + return HeapReAlloc(GetProcessHeap(), HEAP_GENERATE_EXCEPTIONS, buf, size); } static void packet_reply_grow(struct gdb_context* gdbctx, size_t size) -- 2.31.1

Nov. 1, 2021

[PATCH v2 0/8] winedbg: improve winedbg gdb experience (esp. with esoteric module names)

by Jinoh Kang

Changelog: v1 -> v2: address review comments so far Jinoh Kang (8): winedbg: crash when allocation fails in packet_realloc(). winedbg: free buffer and return NULL if size==0 in packet_realloc(). winedbg: generalise packet_realloc(), rename to buffer_realloc(). winedbg: Use unsigned int for offset/length in GDB qXfer handler. winedbg: escape special characters in GDB packet reply. winedbg: buffer and escape output of GDB qXfer commands. winedbg: cache GDB qXfer command result for chunked fetching and define handler table. winedbg: Implement GDB qXfer object exec-file. programs/winedbg/gdbproxy.c | 565 ++++++++++++++++++++++++++++-------- 1 file changed, 452 insertions(+), 113 deletions(-) -- 2.31.1

Nov. 1, 2021

[PATCH v5 7/7] gdi32: calculate effective number of scan lines and truncate bitmap bits for EMR_SETDIBITSTODEVICE.

by Jinoh Kang

Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/gdi32/emfdc.c | 20 ++++++++++++++++++++ dlls/gdi32/tests/metafile.c | 7 ------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 6495129e97e..ee81f2a505a 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -1808,6 +1808,7 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt EMRSETDIBITSTODEVICE *emr; BOOL ret; UINT bmi_size, img_size, payload_size, emr_size; + UINT src_height, stride; BITMAPINFOHEADER bih; BITMAPINFO *bi; @@ -1815,6 +1816,25 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt if (!emf_parse_user_bitmapinfo( &bih, &info->bmiHeader, usage, TRUE, &bmi_size, &img_size )) return 0; + if (bih.biHeight >= 0) + { + src_height = (UINT)bih.biHeight; + if (src_height > y_src + height) src_height = y_src + height; + + if (src_height < startscan) lines = 0; + else if (lines > src_height - startscan) lines = src_height - startscan; + + if (!lines) return 0; + + if (bih.biCompression == BI_RGB || bih.biCompression == BI_BITFIELDS) + { + /* truncate image and check for overflows */ + stride = get_dib_stride( bih.biWidth, bih.biBitCount ); + img_size = lines * stride; + if (img_size / stride != lines) return 0; + } + } + /* check for overflows */ payload_size = bmi_size + img_size; if (payload_size < bmi_size) return 0; diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 25c851e7384..2df38674c2f 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -7764,13 +7764,6 @@ static void test_emf_SetDIBitsToDevice(void) { winetest_push_context("Test %d", test_idx); - if (tests[test_idx].height != bitmap_height && !strcmp(winetest_platform, "wine")) - { - skip("Wine does not adjust cLines appropriately\n"); - winetest_pop_context(); - continue; - } - memset(&bmi, 0, sizeof(bmi)); if (tests[test_idx].infomode < 2) { -- 2.31.1

Nov. 1, 2021

[PATCH v5 6/7] gdi32: allow passing BITMAPINFO with bmiHeader.biSizeImage == 0 for EMF DC.

by Jinoh Kang

User-facing APIs should accept biSizeImage == 0, and fix it up accordingly. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51834 Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/gdi32/emfdc.c | 203 +++++++++++++++++++++++++++++++++--- dlls/gdi32/tests/metafile.c | 14 --- 2 files changed, 187 insertions(+), 30 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 203350d937d..6495129e97e 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -185,6 +185,146 @@ err: return 0; } +/******************************************************************************************* + * Verify that the DIB parameters are valid. + */ +static BOOL is_valid_dib_format( const BITMAPINFOHEADER *info, BOOL allow_compression ) +{ + if (info->biWidth <= 0) return FALSE; + if (info->biHeight == 0) return FALSE; + + if (allow_compression && (info->biCompression == BI_RLE4 || info->biCompression == BI_RLE8)) + { + if (info->biHeight < 0) return FALSE; + if (!info->biSizeImage) return FALSE; + return info->biBitCount == (info->biCompression == BI_RLE4 ? 4 : 8); + } + + if (!info->biPlanes) return FALSE; + + /* check for size overflow */ + if (!info->biBitCount) return FALSE; + if (UINT_MAX / info->biBitCount < info->biWidth) return FALSE; + if (UINT_MAX / get_dib_stride( info->biWidth, info->biBitCount ) < abs( info->biHeight )) return FALSE; + + switch (info->biBitCount) + { + case 1: + case 4: + case 8: + case 24: + return (info->biCompression == BI_RGB); + case 16: + case 32: + return (info->biCompression == BI_BITFIELDS || info->biCompression == BI_RGB); + default: + return FALSE; + } +} + +static BOOL emf_parse_user_bitmapinfo( BITMAPINFOHEADER *dst, const BITMAPINFOHEADER *info, + UINT coloruse, BOOL allow_compression, + UINT *bmi_size, UINT *img_size ) +{ + UINT colour_table_size; + + if (coloruse > DIB_PAL_COLORS + 1) return FALSE; /* FIXME: handle DIB_PAL_COLORS+1 format */ + if (!info) return FALSE; + + memset(dst, 0, sizeof(*dst)); + + if (info->biSize == sizeof(BITMAPCOREHEADER)) + { + const BITMAPCOREHEADER *core = (const BITMAPCOREHEADER *)info; + dst->biWidth = core->bcWidth; + dst->biHeight = core->bcHeight; + dst->biPlanes = core->bcPlanes; + dst->biBitCount = core->bcBitCount; + dst->biCompression = BI_RGB; + dst->biXPelsPerMeter = 0; + dst->biYPelsPerMeter = 0; + dst->biClrUsed = 0; + dst->biClrImportant = 0; + } + else if (info->biSize >= sizeof(BITMAPINFOHEADER)) /* assume BITMAPINFOHEADER */ + { + *dst = *info; + } + else + { + WARN( "(%u): unknown/wrong size for header\n", info->biSize ); + return FALSE; + } + + dst->biSize = sizeof(*dst); + + if (!is_valid_dib_format( dst, allow_compression )) return FALSE; + + colour_table_size = 0; + if (dst->biCompression == BI_BITFIELDS) + { + colour_table_size = 3 * sizeof(DWORD); + } + else if (dst->biBitCount <= 8) + { + UINT elm_size = coloruse == DIB_PAL_COLORS ? sizeof(WORD) : sizeof(DWORD); + UINT colours = dst->biClrUsed; + + if (colours > UINT_MAX / elm_size) + return FALSE; + + colour_table_size = colours * elm_size; + } + + *bmi_size = sizeof(BITMAPINFOHEADER) + colour_table_size; + if (*bmi_size < sizeof(BITMAPINFOHEADER)) + return FALSE; + + if (dst->biCompression == BI_RGB || dst->biCompression == BI_BITFIELDS) + *img_size = get_dib_stride( dst->biWidth, dst->biBitCount ) * abs( dst->biHeight ); + else + *img_size = dst->biSizeImage; + + return TRUE; +} + +static void emf_copy_colours_from_user_bitmapinfo( BITMAPINFO *dst, const BITMAPINFO *info, UINT coloruse ) +{ + if (dst->bmiHeader.biCompression == BI_BITFIELDS) + { + /* bitfields are always at bmiColors even in larger structures */ + memcpy( dst->bmiColors, info->bmiColors, 3 * sizeof(DWORD) ); + } + else if (dst->bmiHeader.biBitCount <= 8) + { + void *src_colors = (char *)info + info->bmiHeader.biSize; + unsigned int colors = dst->bmiHeader.biClrUsed; + + if (!colors) colors = 1 << dst->bmiHeader.biBitCount; + + if (coloruse == DIB_PAL_COLORS) + { + memcpy( dst->bmiColors, src_colors, colors * sizeof(WORD) ); + } + else if (info->bmiHeader.biSize != sizeof(BITMAPCOREHEADER)) + { + memcpy( dst->bmiColors, src_colors, colors * sizeof(RGBQUAD) ); + } + else + { + unsigned int i; + RGBTRIPLE *triple = (RGBTRIPLE *)src_colors; + for (i = 0; i < colors; i++) + { + dst->bmiColors[i].rgbRed = triple[i].rgbtRed; + dst->bmiColors[i].rgbGreen = triple[i].rgbtGreen; + dst->bmiColors[i].rgbBlue = triple[i].rgbtBlue; + dst->bmiColors[i].rgbReserved = 0; + } + } + } +} + static UINT emfdc_add_handle( struct emf *emf, HGDIOBJ obj ) { UINT index; @@ -1602,19 +1742,31 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, { EMRSTRETCHDIBITS *emr; BOOL ret; - UINT bmi_size, emr_size; + UINT bmi_size, img_size, payload_size, emr_size; + BITMAPINFOHEADER bih; + BITMAPINFO *bi; - /* calculate the size of the colour table */ - bmi_size = get_dib_info_size( info, usage ); + /* calculate the size of the colour table and the image */ + if (!emf_parse_user_bitmapinfo( &bih, &info->bmiHeader, usage, TRUE, + &bmi_size, &img_size )) return 0; - emr_size = sizeof (EMRSTRETCHDIBITS) + bmi_size + info->bmiHeader.biSizeImage; + /* check for overflows */ + payload_size = bmi_size + img_size; + if (payload_size < bmi_size) return 0; + + emr_size = sizeof (EMRSTRETCHDIBITS) + payload_size; + if (emr_size < sizeof (EMRSTRETCHDIBITS)) return 0; + + /* allocate record */ if (!(emr = HeapAlloc(GetProcessHeap(), 0, emr_size ))) return 0; /* write a bitmap info header (with colours) to the record */ - memcpy( &emr[1], info, bmi_size); + bi = (BITMAPINFO *)&emr[1]; + bi->bmiHeader = bih; + emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); /* write bitmap bits to the record */ - memcpy ( (BYTE *)&emr[1] + bmi_size, bits, info->bmiHeader.biSizeImage ); + memcpy ( (BYTE *)&emr[1] + bmi_size, bits, img_size ); /* fill in the EMR header at the front of our piece of memory */ emr->emr.iType = EMR_STRETCHDIBITS; @@ -1632,7 +1784,7 @@ BOOL EMFDC_StretchDIBits( DC_ATTR *dc_attr, INT x_dst, INT y_dst, INT width_dst, emr->offBmiSrc = sizeof (EMRSTRETCHDIBITS); emr->cbBmiSrc = bmi_size; emr->offBitsSrc = emr->offBmiSrc + bmi_size; - emr->cbBitsSrc = info->bmiHeader.biSizeImage; + emr->cbBitsSrc = img_size; emr->cxSrc = width_src; emr->cySrc = height_src; @@ -1654,14 +1806,35 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt const void *bits, const BITMAPINFO *info, UINT usage ) { EMRSETDIBITSTODEVICE *emr; - DWORD bmiSize = get_dib_info_size( info, usage ); - DWORD size = sizeof(EMRSETDIBITSTODEVICE) + bmiSize + info->bmiHeader.biSizeImage; BOOL ret; + UINT bmi_size, img_size, payload_size, emr_size; + BITMAPINFOHEADER bih; + BITMAPINFO *bi; - if (!(emr = HeapAlloc( GetProcessHeap(), 0, size ))) return FALSE; + /* calculate the size of the colour table and the image */ + if (!emf_parse_user_bitmapinfo( &bih, &info->bmiHeader, usage, TRUE, + &bmi_size, &img_size )) return 0; + + /* check for overflows */ + payload_size = bmi_size + img_size; + if (payload_size < bmi_size) return 0; + + emr_size = sizeof (EMRSETDIBITSTODEVICE) + payload_size; + if (emr_size < sizeof (EMRSETDIBITSTODEVICE)) return 0; + + /* allocate record */ + if (!(emr = HeapAlloc( GetProcessHeap(), 0, emr_size ))) return FALSE; + + /* write a bitmap info header (with colours) to the record */ + bi = (BITMAPINFO *)&emr[1]; + bi->bmiHeader = bih; + emf_copy_colours_from_user_bitmapinfo( bi, info, usage ); + + /* write bitmap bits to the record */ + memcpy ( (BYTE *)&emr[1] + bmi_size, bits, img_size ); emr->emr.iType = EMR_SETDIBITSTODEVICE; - emr->emr.nSize = size; + emr->emr.nSize = emr_size; emr->rclBounds.left = x_dst; emr->rclBounds.top = y_dst; emr->rclBounds.right = x_dst + width - 1; @@ -1673,14 +1846,12 @@ BOOL EMFDC_SetDIBitsToDevice( DC_ATTR *dc_attr, INT x_dst, INT y_dst, DWORD widt emr->cxSrc = width; emr->cySrc = height; emr->offBmiSrc = sizeof(EMRSETDIBITSTODEVICE); - emr->cbBmiSrc = bmiSize; - emr->offBitsSrc = sizeof(EMRSETDIBITSTODEVICE) + bmiSize; - emr->cbBitsSrc = info->bmiHeader.biSizeImage; + emr->cbBmiSrc = bmi_size; + emr->offBitsSrc = sizeof(EMRSETDIBITSTODEVICE) + bmi_size; + emr->cbBitsSrc = img_size; emr->iUsageSrc = usage; emr->iStartScan = startscan; emr->cScans = lines; - memcpy( (BYTE*)emr + emr->offBmiSrc, info, bmiSize ); - memcpy( (BYTE*)emr + emr->offBitsSrc, bits, info->bmiHeader.biSizeImage ); if ((ret = emfdc_record( dc_attr->emf, (EMR*)emr ))) emfdc_update_bounds( dc_attr->emf, &emr->rclBounds ); diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 0987df95f2d..25c851e7384 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -6382,13 +6382,6 @@ static void test_emf_StretchDIBits(void) { winetest_push_context("Test %d", test_idx); - if (tests[test_idx].infomode != 1 && !strcmp(winetest_platform, "wine")) - { - skip("biSizeImage == 0 not supported under Wine\n"); - winetest_pop_context(); - continue; - } - memset(&bmi, 0, sizeof(bmi)); if (tests[test_idx].infomode < 2) { @@ -7771,13 +7764,6 @@ static void test_emf_SetDIBitsToDevice(void) { winetest_push_context("Test %d", test_idx); - if (tests[test_idx].infomode != 1 && !strcmp(winetest_platform, "wine")) - { - skip("biSizeImage == 0 not supported under Wine\n"); - winetest_pop_context(); - continue; - } - if (tests[test_idx].height != bitmap_height && !strcmp(winetest_platform, "wine")) { skip("Wine does not adjust cLines appropriately\n"); -- 2.31.1

Nov. 1, 2021