cfallin requested dicej for a review on PR #12771.
cfallin requested wasmtime-default-reviewers for a review on PR #12771.
cfallin requested wasmtime-core-reviewers for a review on PR #12771.
cfallin opened PR #12771 from cfallin:gdbstub-component to bytecodealliance:main:
This adds a debug component that makes use of the debug-main world defined in #12756 and serves the gdbstub protocol, with Wasm extensions, compatible with LLDB.
This component is built and included inside the Wasmtime binary, and is loaded using the lower-level
-D debugger=...debug-main option; the user doesn't need to specify the.wasmadapter component. Instead, the user simply runswasmtime run -g <PORT> program.wasm ...and Wasmtime will load and prepare to runprogram.wasmas the debuggee, waiting for a gdbstub connection on the given TCP port before continuing.The workflow is:
$ wasmtime run -g 1234 program.wasm [ wasmtime starts and waits for connection ] $ /opt/wasi-sdk/bin/lldb # use LLDB from wasi-sdk release 32 or later (lldb) process connect --plugin wasm connect://localhost:1234 Process 1 stopped * thread #1, stop reason = signal SIGTRAP frame #0: 0x40000000000001cc -> 0x40000000000001cc: unreachable 0x40000000000001cd: end 0x40000000000001ce: local.get 0 0x40000000000001d0: call 13 (lldb) si Process 1 stopped * thread #1, stop reason = instruction step into frame #0: 0x4000000000000184 -> 0x4000000000000184: block 0x4000000000000186: block 0x4000000000000188: global.get 1 0x400000000000018e: i32.const 3664 [ ... ]This makes use of the
gdbstubthird-party crate, into which I've upstreamed support for the Wasm extensions in daniel5151/gdbstub#188, daniel5151/gdbstub#189, daniel5151/gdbstub#190, and daniel5151/gdbstub#192. (I'll add vets as part of this PR.)
cfallin commented on PR #12771:
This is stacked on top of #12756 until that one lands; only the last commit is new.
I haven't added end-to-end tests that spawn/interact with LLDB yet; depending on how that goes I might be able to include that here or might defer to another PR if that's OK.
cfallin requested alexcrichton for a review on PR #12771.
cfallin unassigned dicej from PR #12771 Debugging: add builtin gdbstub component..
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
cfallin commented on PR #12771:
Rebased out #12756; should be good to review now.
github-actions[bot] added the label wizer on PR #12771.
github-actions[bot] commented on PR #12771:
Subscribe to Label Action
cc @fitzgen
<details>
This issue or pull request has been labeled: "wizer"Thus the following users have been cc'd because of the following labels:
- fitzgen: wizer
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton submitted PR review.
alexcrichton created PR review comment:
To gauge, how big is this binary? Are we talking ~30M or something more like ~1M? Given that this is included in the CLI uncompressed it might be reasonable to try to apply simple size optimizations where possible if it's extra large.
alexcrichton created PR review comment:
To me defaulting to localhost feels more natural, but at the same time if doing so we should probably have the option to handle both 0.0.0.0 and localhost. Maybe the
gdbstub_portoption is renamed to justgdbstub, and then internally the wasm does parsing to figure out if it's a port or port-and-address?
alexcrichton created PR review comment:
Instead of pulling in
structopt, can you useclap? They should be pretty similar and easy to transition between, but my impression is thatstructoptis more-or-less deprecated in favor ofclap
alexcrichton created PR review comment:
To avoid
.to_vec()on a big thing, could the field inRunCommandchange to&'static [u8]?
alexcrichton created PR review comment:
Hm ok here's a sticking point I didn't realize: this makes the
wasmtime-clicrate non-publishable because there's a dependency on something that doesn't exist on crates.io. I'm also realizing now that it's not as simple as publishing this crate since it's fundamentally not publishable, it relies on being part of this workspace to build a sibling crate, which isn't present when built as a dep from crates.io.The "true fix" for this is https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#artifact-dependencies, an unstable Cargo feature. Unfortunately we can't rely on that even in a nightly-conditional context I believe, if we tried to use that it would mean that Wasmtime would always require nightly.
Some possible ideas:
- Check in the gdbstub binary and verify it's built in CI. I suspect it's a bit large and will receive many changes, so not my first choice.
- Download gdbstub from github releases for released wasmtime-cli artifacts. We don't currently have HTTP/network dependencies in the CLI outside of WASI impls, so that'll be hard.
- Publish this crate to crates.io, but handle failures in the
build.rsscript. That'd mean that the gdbstub binary would have to be an off-by-default optional feature which we enable for our release builds but would be off-by-default for crates.io.Well, "some possible ideas" aka I think randomly in real time until something semi-reasonable pops out... Maybe that last one?
alexcrichton commented on PR #12771:
Also, to clarify, @cfallin what depth would you like me to review the gdbstub component code itself? I'm happy more-or-less not reviewing it at all in the sense that it's well-sequestered, low-risk, and we'll likely iterate a lot on it in-tree. If you'd prefer though I could give it a closer look in any particular areas of interest.
cfallin submitted PR review.
cfallin created PR review comment:
Oh, hmm, I definitely didn't foresee this one being a problem either!
Question on the last option: what do you mean by "handle failures in
build.rs"; in other words, why would such a failure be any different than some other build failure for a crate on crates.io (some of which are e.g. crates that wrap C code and usecc, or do other build-time shenanigans)?
cfallin commented on PR #12771:
Also, to clarify, @cfallin what depth would you like me to review the gdbstub component code itself? I'm happy more-or-less not reviewing it at all in the sense that it's well-sequestered, low-risk, and we'll likely iterate a lot on it in-tree. If you'd prefer though I could give it a closer look in any particular areas of interest.
I guess my default answer is "to whatever extent allows us to fulfill policy and be comfortable having this code in-repo" :-) I agree that since it's sandboxed, the bar could be lower than for core runtime code. I guess the spirit of our code-review policies is still that someone should give it a once-over -- but up to you how deep you take that!
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Sort of two things:
- I forget what cwd is used for
build.rsso I don't know what effectcargo build -p thingwill have when something is published to crates.io. If that tries to build other random crates in a workspace, for example, I think that'd be bad.- I think this would ideally have a better error message than "package
thingnot found" when built from crates.io, aka something like "you can't enable the gdbstub component when wasmtime is built from crates.io" or similar.Basically, yeah, build-time weirdness is expected, but I'd like to ideally tame it. Another example would be printing a better error if
wasm32-wasip2weren't installed, but rustc does a decent job of this already.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I forget what QEMU does, but this might be a reasonable place to print "Hey I'm listening on address A.B.C.D:XXXX for a debugger" both to signify that's why the process is halted and also be a sort of "join point" for "if you're a test, now's when you can make your TCP connection" synchronization point.
alexcrichton created PR review comment:
Question for you: how come these aren't implemented with blocking I/O? My rough assumption is that blocking I/O is expected while gdbstub is doing stuff and the guest is halted, and the only async-y bit is "wait for I/O on the TCP connection or the guest to hit an event"
alexcrichton created PR review comment:
Could this perhaps support dual setting breakpoints on mapped addresses and seeing breakpoints on raw addresses in the code section?
alexcrichton created PR review comment:
stray debugging annotation?
alexcrichton created PR review comment:
This may want to print something about an error perhaps? Otherwise the guest is still running and it may be "violently deleted" when dropping
cfallin updated PR #12771.
cfallin requested alexcrichton for a review on PR #12771.
cfallin updated PR #12771.
cfallin requested wasmtime-compiler-reviewers for a review on PR #12771.
cfallin updated PR #12771.
cfallin updated PR #12771.
alexcrichton submitted PR review:
I'm realizing that I'm going to be gone for awhile after wasm.io and I don't want to leave this languishing. With the various comments I've left I think this is fine to land and iterate in-tree, and if you'd prefer feel free to defer anything to an issue and/or a follow-up PR.
Last updated: Mar 23 2026 at 16:19 UTC