From bebfa11e2b7f508d346759f6abc1d64c463c493a Mon Sep 17 00:00:00 2001 From: Suzu Date: Sat, 5 Sep 2026 03:57:18 +0800 Subject: [PATCH 1/3] Drain buffered GDB packets before polling The RSP transport can read several requests at once, but the session polls the socket before consuming each request. Remaining requests stall when the client waits for their replies without sending more data. Consume buffered input before polling. Add a regression that sends eight requests together and requires every reply without another socket write. --- src/debug/gdbstub.c | 32 +++++++++++++++++--------------- tests/test-gdbstub.sh | 37 ++++++++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/debug/gdbstub.c b/src/debug/gdbstub.c index be3a10f3..81130961 100644 --- a/src/debug/gdbstub.c +++ b/src/debug/gdbstub.c @@ -1143,23 +1143,25 @@ static void gdb_client_session(void) gdb.rsp_ctx = &rsp_ctx; while (gdb.client_fd >= 0) { - /* Wait for either a packet from GDB or a stop event from a vCPU. Use - * poll() so the GDB stub can wake up when a thread stops. + /* A socket read can buffer several packets. Consume them before waiting + * for more input that the client may never send. */ - struct pollfd pfd = { - .fd = gdb.client_fd, - .events = POLLIN, - }; - - int pr = poll(&pfd, 1, -1); - if (pr <= 0) { - if (pr < 0 && errno == EINTR) - continue; - break; - } + if (rsp_ctx.read_pos >= rsp_ctx.read_len) { + struct pollfd pfd = { + .fd = gdb.client_fd, + .events = POLLIN, + }; + + int pr = poll(&pfd, 1, -1); + if (pr <= 0) { + if (pr < 0 && errno == EINTR) + continue; + break; + } - if (pfd.revents & (POLLERR | POLLHUP)) - break; + if (pfd.revents & (POLLERR | POLLHUP)) + break; + } int pkt_len = gdb_rsp_recv(&rsp_ctx, gdb.client_fd, pkt_buf, GDB_PKT_BUF_SIZE); diff --git a/tests/test-gdbstub.sh b/tests/test-gdbstub.sh index 1a02aa75..c7ec5d68 100755 --- a/tests/test-gdbstub.sh +++ b/tests/test-gdbstub.sh @@ -149,8 +149,9 @@ run_lldb() run_raw_rsp_script() { local timeout_sec="${RAW_RSP_TIMEOUT:-10}" + RAW_RSP_RC=0 RAW_RSP_OUT=$( - timeout "$timeout_sec" python3 - "$GDB_PORT" 2>&1 << 'PY' + timeout "$timeout_sec" python3 - "$GDB_PORT" "${1:-single}" 2>&1 << 'PY' import socket import sys @@ -158,10 +159,13 @@ port = int(sys.argv[1]) sock = socket.create_connection(("127.0.0.1", port), timeout=5) sock.settimeout(5) -def send_packet(payload: str) -> None: +def frame_packet(payload: str) -> bytes: data = payload.encode("ascii") cksum = sum(data) & 0xFF - sock.sendall(b"$" + data + b"#" + f"{cksum:02x}".encode("ascii")) + return b"$" + data + b"#" + f"{cksum:02x}".encode("ascii") + +def send_packet(payload: str) -> None: + sock.sendall(frame_packet(payload)) def recv_exact(n: int) -> bytes: buf = b"" @@ -187,6 +191,8 @@ def recv_packet(): break body.extend(ch) cksum = recv_exact(2).decode("ascii") + if int(cksum, 16) != sum(body) & 0xFF: + raise RuntimeError("invalid reply checksum") return ack, body.decode("ascii"), cksum send_packet("qSupported") @@ -207,9 +213,18 @@ print(f"stop_ack={ack3 or 'none'}") print(f"stop_body={body3}") print(f"stop_cksum={cksum3}") +if sys.argv[2] == "batch": + # No further socket writes may wake the stub while replies are pending. + sock.sendall(frame_packet("qAttached") * 8) + for _ in range(8): + ack, body, _ = recv_packet() + if ack or body != "1": + raise RuntimeError(f"unexpected batched reply: {ack!r} {body!r}") + print("batched_replies=8") + sock.close() PY - ) || true + ) || RAW_RSP_RC=$? } report() @@ -507,7 +522,8 @@ stop_elfuse start_elfuse "$GUEST" run_raw_rsp_script ok=0 -if echo "$RAW_RSP_OUT" | grep -q "qSupported_ack=+" \ +if [ "$RAW_RSP_RC" -eq 0 ] \ + && echo "$RAW_RSP_OUT" | grep -q "qSupported_ack=+" \ && echo "$RAW_RSP_OUT" | grep -q "qSupported_body=.*QStartNoAckMode+" \ && echo "$RAW_RSP_OUT" | grep -q "noack_ack=+" \ && echo "$RAW_RSP_OUT" | grep -q "noack_body=OK" \ @@ -518,6 +534,17 @@ fi report "QStartNoAckMode: final ack then packet-only replies" $ok stop_elfuse +# Test 18: Requests buffered by the transport must not wait for socket input. +start_elfuse "$GUEST" +run_raw_rsp_script batch +ok=0 +if [ "$RAW_RSP_RC" -eq 0 ] \ + && echo "$RAW_RSP_OUT" | grep -q "^batched_replies=8$"; then + ok=1 +fi +report "buffered packets: reply without further socket input" $ok +stop_elfuse + # Summary echo "" if [ "$fails" -eq 0 ]; then From 7a49c10be4daa540488d4078bd3b641b5b69fe3b Mon Sep 17 00:00:00 2001 From: Suzu Date: Sat, 5 Sep 2026 08:05:09 +0800 Subject: [PATCH 2/3] Centralize the GDB buffer predicate The session loop and transport decoder both decide when buffered input is exhausted. Share the transport predicate so changes to the buffer representation have one definition to update. --- src/debug/gdbstub-rsp.c | 9 +++++++-- src/debug/gdbstub-rsp.h | 2 ++ src/debug/gdbstub.c | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/debug/gdbstub-rsp.c b/src/debug/gdbstub-rsp.c index 1d87fc00..de21f19c 100644 --- a/src/debug/gdbstub-rsp.c +++ b/src/debug/gdbstub-rsp.c @@ -267,6 +267,11 @@ void gdb_rsp_reset(gdb_rsp_ctx_t *ctx) ctx->no_ack_mode = false; } +bool gdb_rsp_pending(const gdb_rsp_ctx_t *ctx) +{ + return ctx->read_pos < ctx->read_len; +} + void gdb_rsp_set_noack(gdb_rsp_ctx_t *ctx, bool enabled) { ctx->no_ack_mode = enabled; @@ -299,7 +304,7 @@ int gdb_rsp_recv(gdb_rsp_ctx_t *ctx, int fd, char *buf, size_t bufsz) bool overflow = false; while (1) { - if (ctx->read_pos >= ctx->read_len) { + if (!gdb_rsp_pending(ctx)) { ssize_t n = read(fd, ctx->read_buf, GDB_RSP_READ_BUF_SIZE); if (n <= 0) { if (n < 0 && errno == EINTR) @@ -310,7 +315,7 @@ int gdb_rsp_recv(gdb_rsp_ctx_t *ctx, int fd, char *buf, size_t bufsz) ctx->read_len = (size_t) n; } - while (ctx->read_pos < ctx->read_len) { + while (gdb_rsp_pending(ctx)) { uint8_t c = ctx->read_buf[ctx->read_pos++]; if (state == 0 && (c == '+' || c == '-')) diff --git a/src/debug/gdbstub-rsp.h b/src/debug/gdbstub-rsp.h index 8d96ea8b..2488672d 100644 --- a/src/debug/gdbstub-rsp.h +++ b/src/debug/gdbstub-rsp.h @@ -26,5 +26,7 @@ uint64_t gdb_parse_hex(const char **pp); int gdb_rsp_send(int fd, const char *data, size_t len); void gdb_rsp_reset(gdb_rsp_ctx_t *ctx); +/* Unconsumed bytes may contain acknowledgments or an incomplete packet. */ +bool gdb_rsp_pending(const gdb_rsp_ctx_t *ctx); void gdb_rsp_set_noack(gdb_rsp_ctx_t *ctx, bool enabled); int gdb_rsp_recv(gdb_rsp_ctx_t *ctx, int fd, char *buf, size_t bufsz); diff --git a/src/debug/gdbstub.c b/src/debug/gdbstub.c index 81130961..8287b2b1 100644 --- a/src/debug/gdbstub.c +++ b/src/debug/gdbstub.c @@ -1146,7 +1146,7 @@ static void gdb_client_session(void) /* A socket read can buffer several packets. Consume them before waiting * for more input that the client may never send. */ - if (rsp_ctx.read_pos >= rsp_ctx.read_len) { + if (!gdb_rsp_pending(&rsp_ctx)) { struct pollfd pfd = { .fd = gdb.client_fd, .events = POLLIN, From 6ad8748664ef2aa3de244d7c7ad30318e03e62b2 Mon Sep 17 00:00:00 2001 From: Suzu Date: Sat, 5 Sep 2026 08:05:16 +0800 Subject: [PATCH 3/3] Make buffered GDB regression checks deterministic Force two requests into one transport read and require both replies before any further client input. Exercise the real session with a nonblocking poll wrapper so an unconditional poll fails on the missing reply instead of hanging the test. Run the host case from the GDB and shared check lanes. Compare reply checksum text against the sender's two-digit lowercase format, and document the TCP batch test's coalescing assumption. --- Makefile | 6 ++ mk/config.mk | 3 +- mk/tests.mk | 12 ++- tests/test-gdbstub-host.c | 164 ++++++++++++++++++++++++++++++++++++++ tests/test-gdbstub.sh | 5 +- 5 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 tests/test-gdbstub-host.c diff --git a/Makefile b/Makefile index b5dfb79b..1b5b23aa 100644 --- a/Makefile +++ b/Makefile @@ -213,6 +213,12 @@ $(BUILD_DIR)/test-teardown-live-vcpu-host: \ @echo " LD $@" $(Q)$(CC) $(CFLAGS) -o $@ $^ $(HVF_LDFLAGS) +## Build the buffered GDB session host regression +$(BUILD_DIR)/test-gdbstub-host: $(BUILD_DIR)/test-gdbstub-host.o \ + $(BUILD_DIR)/debug/gdbstub-reg.o | $(BUILD_DIR) + @echo " LD $@" + $(Q)$(CC) $(CFLAGS) -o $@ $^ $(HVF_LDFLAGS) + ## Build the proved/gva.h contract-check host test (native macOS binary) # Header-only: proved/gva.h is static inline, so the test links nothing # from the project. It skips unless the build defines ELFUSE_CONTRACT_ASSERT, diff --git a/mk/config.mk b/mk/config.mk index 69b1f32a..b0118d5b 100644 --- a/mk/config.mk +++ b/mk/config.mk @@ -32,7 +32,8 @@ NATIVE_TESTS := tests/test-multi-vcpu.c tests/test-rwx.c \ tests/test-stdio-nonblock-host.c \ tests/test-guest-env-host.c \ tests/test-usb-desc-host.c \ - tests/test-elf-headers-host.c + tests/test-elf-headers-host.c \ + tests/test-gdbstub-host.c SPECIAL_TEST_SRCS := tests/test-lowbase-mem.c SPECIAL_TEST_BINS := $(BUILD_DIR)/test-lowbase-mem-200000 $(BUILD_DIR)/test-lowbase-mem-300000 diff --git a/mk/tests.mk b/mk/tests.mk index 2238576d..029d7561 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -21,7 +21,7 @@ ELFUSE_HOST_NOFILE_MIN ?= $(shell bash "$(CURDIR)/tests/test-config.sh" --host-n test-sysroot-tmp-remove test-sysroot-host-fallback test-sysroot-case-exact \ test-sysroot-create-paths test-fork-ipc-protocol-host \ test-vcpu-run-hooks-host test-identity-override-host \ - test-dynamic-array-host test-string-builder-host \ + test-dynamic-array-host test-string-builder-host test-gdbstub-host \ test-config \ test-mremap-tail-emfile \ test-proctitle-host test-proctitle-low-stack \ @@ -235,7 +235,7 @@ CHECK_HOST_UNIT_BINS := $(addprefix $(BUILD_DIR)/, \ test-casefold-walk-host test-absock-names-host \ test-dynamic-array-host test-string-builder-host \ test-wakeup-pipe-host test-guest-env-host \ - test-usb-desc-host test-elf-headers-host) + test-usb-desc-host test-elf-headers-host test-gdbstub-host) # Lanes shared by check and check-sanitizer, in execution order: the host # unit binaries, then the name-contract lanes cheap enough for a sanitizer @@ -257,6 +257,7 @@ $(call run-host-unit,test-stdio-nonblock-host,launcher stdio flags across a gues $(call run-host-unit,test-guest-env-host,guest environment merge cross product) $(call run-host-unit,test-usb-desc-host,USB descriptor blob walk unit test) $(call run-host-unit,test-elf-headers-host,ELF header validation unit test) +$(call run-host-unit,test-gdbstub-host,buffered GDB session regression) $(call run-lane,test-usb-sysfs,synthetic USB tree contract) $(call run-lane,test-usb-sysfs-sysroot,synthetic USB /sys sharing a populated sysroot) $(call run-lane,test-usb-sysfs-matrix,every /sys and /dev/bus entry point against every path class) @@ -1147,8 +1148,13 @@ test-launch-flags: $(ELFUSE_BIN) $(TEST_HELLO_DEP) $(TEST_ENV_DEPS) test-usage-synopsis: $(ELFUSE_BIN) @bash tests/test-usage-synopsis.sh $(ELFUSE_BIN) +## Run the buffered GDB session host regression +test-gdbstub-host: $(BUILD_DIR)/test-gdbstub-host + $(BUILD_DIR)/test-gdbstub-host + ## Run GDB stub integration tests (LLDB <-> elfuse gdbstub) -test-gdbstub: $(ELFUSE_BIN) $(TEST_DIR)/test-hello +test-gdbstub: $(ELFUSE_BIN) $(TEST_DIR)/test-hello $(BUILD_DIR)/test-gdbstub-host + $(call run-host-unit,test-gdbstub-host,buffered GDB session regression) @bash tests/test-gdbstub.sh -e $(ELFUSE_BIN) -v ## Run Rosetta CLI gating regressions without requiring Rosetta runtime support diff --git a/tests/test-gdbstub-host.c b/tests/test-gdbstub-host.c new file mode 100644 index 00000000..d9294c4f --- /dev/null +++ b/tests/test-gdbstub-host.c @@ -0,0 +1,164 @@ +/* + * Copyright 2026 elfuse contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +#include "host-test-util.h" +#include "utils.h" + +static const char requests[] = "$qAttached#8f$qAttached#8f"; +static const char replies[] = "+$1#31+$1#31"; +static int server_fd; +static int read_calls; +static int poll_calls; + +static void require(bool ok, const char *detail) +{ + host_check(ok, "buffered session", detail); + if (!ok) + exit(EXIT_FAILURE); +} + +static ssize_t batch_read(int fd, void *buf, size_t count) +{ + require(fd == server_fd && ++read_calls == 1, + "only one transport read is needed"); + require(count >= sizeof(requests) - 1, "the read buffer fits both packets"); + + /* Return both packets together even when the socket returns short reads. */ + size_t used = 0; + while (used < sizeof(requests) - 1) { + ssize_t n = read(fd, (char *) buf + used, sizeof(requests) - 1 - used); + if (n < 0 && errno == EINTR) + continue; + require(n > 0, "the queued request bytes are readable"); + used += (size_t) n; + } + int queued = -1; + require(ioctl(fd, FIONREAD, &queued) == 0 && queued == 0, + "the socket is empty after the transport read"); + return (ssize_t) used; +} + +static int session_poll(struct pollfd *fds, nfds_t count, int timeout) +{ + require(count == 1 && fds[0].fd == server_fd && fds[0].events == POLLIN && + timeout == -1, + "the session polls its client socket"); + poll_calls++; + + /* Keep real readiness results, but return immediately at an empty socket. + */ + int ready = poll(fds, count, 0); + require( + poll_calls == 1 ? ready == 1 && fds[0].revents == POLLIN : ready == 0, + "only the initial poll finds socket input"); + return ready; +} + +/* Compile the real session and transport with only their I/O calls wrapped. */ +#define read batch_read +#include "../src/debug/gdbstub-rsp.c" +#undef read +#define poll session_poll +#include "../src/debug/gdbstub.c" +#undef poll + +/* No guest or vCPU operation belongs to a qAttached exchange. */ +static _Noreturn void unexpected_call(const char *name) +{ + host_fail("unexpected dependency", name); + exit(EXIT_FAILURE); +} + +_Thread_local thread_entry_t *current_thread; + +thread_entry_t *thread_find(int64_t tid) +{ + unexpected_call(__func__); +} + +bool thread_tid_alive(int64_t tid) +{ + unexpected_call(__func__); +} + +void thread_for_each(void (*fn)(thread_entry_t *t, void *ctx), void *ctx) +{ + unexpected_call(__func__); +} + +void thread_interrupt_all(void) +{ + unexpected_call(__func__); +} + +void *guest_ptr_bound(const guest_t *g, + uint64_t gva, + uint64_t *avail, + int required_perms, + uint64_t len_limit) +{ + unexpected_call(__func__); +} + +int guest_read(const guest_t *g, uint64_t gva, void *dst, size_t len) +{ + unexpected_call(__func__); +} + +int guest_write(guest_t *g, uint64_t gva, const void *src, size_t len) +{ + unexpected_call(__func__); +} + +void log_impl(int level, const char *file, int line, const char *fmt, ...) +{ + unexpected_call(__func__); +} + +int main(void) +{ + int sockets[2]; + require(socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == 0, "socketpair"); + server_fd = sockets[0]; + require(fcntl(server_fd, F_SETFL, O_NONBLOCK) == 0 && + fcntl(sockets[1], F_SETFL, O_NONBLOCK) == 0, + "unexpected socket reads must not block"); + require(write_all(sockets[1], requests, sizeof(requests) - 1) == 0, + "queue both requests before entering the session"); + + gdb.client_fd = server_fd; + gdb_client_session(); + + require(read_calls == 1, "both requests use one transport read"); + require(poll_calls == 2, "poll only before input and after draining it"); + require(gdb.rsp_ctx == NULL, "the session clears its transport pointer"); + + int queued = -1; + require(ioctl(server_fd, FIONREAD, &queued) == 0 && queued == 0, + "no further client input wakes the session"); + require(ioctl(sockets[1], FIONREAD, &queued) == 0 && + queued == sizeof(replies) - 1, + "both replies arrived before further client input"); + char received[sizeof(replies) - 1]; + size_t used = 0; + while (used < sizeof(received)) { + ssize_t n = read(sockets[1], received + used, sizeof(received) - used); + if (n < 0 && errno == EINTR) + continue; + require(n > 0, "read the queued replies"); + used += (size_t) n; + } + require(memcmp(received, replies, sizeof(received)) == 0, + "both requests produce their ACK and checksummed reply"); + + close(sockets[0]); + close(sockets[1]); + gdb.client_fd = -1; + return host_summary("test-gdbstub-host"); +} diff --git a/tests/test-gdbstub.sh b/tests/test-gdbstub.sh index c7ec5d68..9d644b4f 100755 --- a/tests/test-gdbstub.sh +++ b/tests/test-gdbstub.sh @@ -191,7 +191,7 @@ def recv_packet(): break body.extend(ch) cksum = recv_exact(2).decode("ascii") - if int(cksum, 16) != sum(body) & 0xFF: + if cksum != f"{sum(body) & 0xFF:02x}": raise RuntimeError("invalid reply checksum") return ack, body.decode("ascii"), cksum @@ -214,7 +214,8 @@ print(f"stop_body={body3}") print(f"stop_cksum={cksum3}") if sys.argv[2] == "batch": - # No further socket writes may wake the stub while replies are pending. + # This loopback write relies on coalescing; short reads can hide the stall. + # Send nothing else until all eight replies arrive. sock.sendall(frame_packet("qAttached") * 8) for _ in range(8): ack, body, _ = recv_packet()