Rémi Bernon : winedbg: Cleanup extract_packets for faster acking.
Module: wine Branch: master Commit: ec76cb754e62c37e65c233f4e4ed7bd0d56f564d URL: https://source.winehq.org/git/wine.git/?a=commit;h=ec76cb754e62c37e65c233f4e... Author: Rémi Bernon <rbernon(a)codeweavers.com> Date: Tue Mar 31 15:12:28 2020 +0200 winedbg: Cleanup extract_packets for faster acking. Sometimes multiple packets are received and we were assuming it was some repeated requests due to slow ack. We can ack packets first. It was also dropping some perfectly valid packets and we should process them all. For instance, lldb frontend sometimes send multiple packets at a the same time and expects them to be handled. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- programs/winedbg/gdbproxy.c | 151 +++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 79 deletions(-) diff --git a/programs/winedbg/gdbproxy.c b/programs/winedbg/gdbproxy.c index fde6c556c2..211d39edd5 100644 --- a/programs/winedbg/gdbproxy.c +++ b/programs/winedbg/gdbproxy.c @@ -1746,95 +1746,88 @@ static struct packet_entry packet_entries[] = static BOOL extract_packets(struct gdb_context* gdbctx) { - char* end; - int plen; - unsigned char in_cksum, loc_cksum; - char* ptr; - enum packet_return ret = packet_error; - int num_packet = 0; + char *ptr, *sum = gdbctx->in_buf, *end = gdbctx->in_buf + gdbctx->in_len; + enum packet_return ret = packet_error; + unsigned int cksum; + int i, len; + + /* ptr points to the beginning ('$') of the current packet + * sum points to the beginning ('#') of the current packet checksum ("#xx") + * len is the length of the current packet data (sum - ptr - 1) + * end points to the end of the received data buffer + */ - while ((ret & packet_last_f) == 0) + while ((ptr = memchr(sum, '$', end - sum)) && + (sum = memchr(ptr, '#', end - ptr)) && + (end - sum >= 3) && sscanf(sum, "#%02x", &cksum) == 1) { - TRACE("Packet: %s\n", debugstr_an(gdbctx->in_buf, gdbctx->in_len)); - ptr = memchr(gdbctx->in_buf, '$', gdbctx->in_len); - if (ptr == NULL) return FALSE; - if (ptr != gdbctx->in_buf) + len = sum - ptr - 1; + sum += 3; + + if (cksum == checksum(ptr + 1, len)) { - int glen = ptr - gdbctx->in_buf; /* garbage len */ - WARN("Removing garbage: %s\n", debugstr_an(gdbctx->in_buf, glen)); - gdbctx->in_len -= glen; - memmove(gdbctx->in_buf, ptr, gdbctx->in_len); + TRACE("Acking: %s\n", debugstr_an(ptr, sum - ptr)); + write(gdbctx->sock, "+", 1); } - end = memchr(gdbctx->in_buf + 1, '#', gdbctx->in_len); - if (end == NULL) return FALSE; - /* no checksum yet */ - if (end + 3 > gdbctx->in_buf + gdbctx->in_len) return FALSE; - plen = end - gdbctx->in_buf - 1; - hex_from(&in_cksum, end + 1, 1); - loc_cksum = checksum(gdbctx->in_buf + 1, plen); - if (loc_cksum == in_cksum) + else { - if (num_packet == 0) { - int i; - - ret = packet_error; - - write(gdbctx->sock, "+", 1); - assert(plen); - - /* FIXME: should use bsearch if packet_entries was sorted */ - for (i = 0; i < ARRAY_SIZE(packet_entries); i++) - { - if (packet_entries[i].key == gdbctx->in_buf[1]) break; - } - if (i == ARRAY_SIZE(packet_entries)) - WARN("Unhandled packet %s\n", debugstr_an(&gdbctx->in_buf[1], plen)); - else - { - gdbctx->in_packet = gdbctx->in_buf + 2; - gdbctx->in_packet_len = plen - 1; - gdbctx->in_packet[gdbctx->in_packet_len] = '\0'; - ret = (packet_entries[i].handler)(gdbctx); - } - switch (ret & ~packet_last_f) - { - case packet_error: packet_reply(gdbctx, ""); break; - case packet_ok: packet_reply(gdbctx, "OK"); break; - case packet_done: break; - } - TRACE("Reply: %s\n", debugstr_an(gdbctx->out_buf, gdbctx->out_len)); - i = write(gdbctx->sock, gdbctx->out_buf, gdbctx->out_len); - assert(i == gdbctx->out_len); - /* if this fails, we'll have to use POLLOUT... - */ - gdbctx->out_len = 0; - num_packet++; - } - else + ERR("Nacking: %s (checksum: %d != %d)\n", debugstr_an(ptr, sum - ptr), + cksum, checksum(ptr + 1, len)); + write(gdbctx->sock, "-", 1); + } + } + + while ((ret & packet_last_f) == 0 && + (ptr = memchr(gdbctx->in_buf, '$', gdbctx->in_len)) && + (sum = memchr(ptr, '#', end - ptr)) && + (end - sum >= 3) && sscanf(sum, "#%02x", &cksum) == 1) + { + if (ptr != gdbctx->in_buf) + WARN("Ignoring: %s\n", debugstr_an(gdbctx->in_buf, ptr - gdbctx->in_buf)); + + len = sum - ptr - 1; + sum += 3; + + if (cksum == checksum(ptr + 1, len)) + { + TRACE("Handling: %s\n", debugstr_an(ptr, sum - ptr)); + + ret = packet_error; + gdbctx->in_packet = ptr + 2; + gdbctx->in_packet_len = len - 1; + gdbctx->in_packet[gdbctx->in_packet_len] = '\0'; + + for (i = 0; i < ARRAY_SIZE(packet_entries); i++) + if (packet_entries[i].key == ptr[1]) + break; + + if (i == ARRAY_SIZE(packet_entries)) + WARN("Unhandled: %s\n", debugstr_an(ptr + 1, len)); + else if (((ret = (packet_entries[i].handler)(gdbctx)) & ~packet_last_f) == packet_error) + WARN("Failed: %s\n", debugstr_an(ptr + 1, len)); + + switch (ret & ~packet_last_f) { - /* FIXME: If we have more than one packet in our input buffer, - * it's very likely that we took too long to answer to a given packet - * and gdb is sending us the same packet again. - * So we simply drop the second packet. This will lower the risk of error, - * but there are still some race conditions here. - * A better fix (yet not perfect) would be to have two threads: - * - one managing the packets for gdb - * - the second one managing the commands... - * This would allow us to send the reply with the '+' character (Ack of - * the command) way sooner than we do now. - */ - ERR("Dropping packet; I was too slow to respond\n"); + case packet_error: packet_reply(gdbctx, ""); break; + case packet_ok: packet_reply(gdbctx, "OK"); break; + case packet_done: break; } + + TRACE("Reply: %s\n", debugstr_an(gdbctx->out_buf, gdbctx->out_len)); + i = write(gdbctx->sock, gdbctx->out_buf, gdbctx->out_len); + assert(i == gdbctx->out_len); + gdbctx->out_len = 0; } else - { - write(gdbctx->sock, "+", 1); - ERR("Dropping packet; invalid checksum %d <> %d\n", in_cksum, loc_cksum); - } - gdbctx->in_len -= plen + 4; - memmove(gdbctx->in_buf, end + 3, gdbctx->in_len); + WARN("Ignoring: %s (checksum: %d != %d)\n", debugstr_an(ptr, sum - ptr), + cksum, checksum(ptr + 1, len)); + + gdbctx->in_len = end - sum; + memmove(gdbctx->in_buf, sum, end - sum); + end = gdbctx->in_buf + gdbctx->in_len; } - return TRUE; + + return (ret & packet_last_f); } static int fetch_data(struct gdb_context* gdbctx)
participants (1)
-
Alexandre Julliard