Add support for "expedited registers" in T-stop packets.#189
Conversation
This is required to work around an issue encountered with LLDB with a Wasm target over gdbstub: the Wasm-target client incorrectly caches (does not re-query) the PC, and needs to be updated after a stop either via a response with the `QListThreadsInStopReply` capability or by sending explicit updated register values directly in the T-stop packets. I opted for the latter here as it's a little simpler.
daniel5151
left a comment
There was a problem hiding this comment.
A few tweaks to tighten up the UX, but broadly speaking, LGTM
src/stub/state_machine.rs
Outdated
| mut self, | ||
| target: &mut T, | ||
| reason: impl IntoStopReason<T>, | ||
| regs: &mut dyn Iterator<Item = (u32, &[u8])>, |
There was a problem hiding this comment.
this should use the Target Arch RegId associated type (see https://docs.rs/gdbstub/0.7.6/gdbstub/arch/trait.Arch.html#associatedtype.RegId), as opposed to a bare u32
There was a problem hiding this comment.
It seems that this would require a breaking API change: RegId has from_raw_id, but not to_raw_id (nor is it constrained as a subtrait of any numeric trait or equivalent) -- in other words gdbstub currently requires arch implementers to parse from raw integers to registers, but not to provide raw integers for registers. (The ordering is implicit in the gdb_serialize/gdb_deserialize impls provided by the arch.)
I'm happy to add to_raw_id to RegId and then use it here if you'd prefer! Otherwise this is why I had to use a raw u32...
There was a problem hiding this comment.
Ahh fooey.
Yeah, we def need a to_raw_id method on RegId to pull this off...
On the bright side, if we add to_raw_id with a default stub impl (i.e: returning None), we can avoid making this a breaking change.
Lets go ahead and add to_raw_id, with some docs on the new method + on report_stop_with_regs commenting on how Arch implementations only need to implement the method if they want to use the report_stop_with_regs API.
For in-tree validation, please update the armv4t example to use report_stop_with_regs (e.g: to send back the PC, or whatever), and update the ArmCoreRegId type) with a non-trivial impl of the new method
There was a problem hiding this comment.
Done, and added docs on both referring to each other (report_stop_with_regs re: the need for to_raw_id, and to_raw_id re: needed only if report_stop_with_regs used).
I wasn't able to add this to the in-tree example as-is because the API is not reflected up to the run_blocking API (this method here lives on the inner state machine, which I use directly). I'm not sure how you'd like to do so without carrying either an allocation (for a list of regs to send) or a &mut dyn callback or iterator (to pull regs to send) in BlockingEventLoopEvent, both of which seem like breaking changes -- requiring an allocator or adding a lifetime parameter respectively.
(A little more meta: given the above I'm kind of running out of steam on upstreaming-energy here -- if this can't go as-is because it needs more API surface to be included that I won't use in my use-case, I may just carry it as a patch in a local fork of gdbstub in Wasmtime. Sorry, just need to limit how much time I spend in side-adventures)
There was a problem hiding this comment.
Ah, yep, that's right. No worries - this implementation seems pretty straightforward, and I'll trust that it's working fine with your use-case.
(and yep, totally understand! I really appreciate the willingness to iterate here. I think we're just wrapping up with a few nits here, but the various PRs should land shortly)
|
Thanks! Updated -- everything addressed except for the bit about |
src/stub/state_machine.rs
Outdated
| mut self, | ||
| target: &mut T, | ||
| reason: impl IntoStopReason<T>, | ||
| regs: &mut dyn Iterator<Item = (u32, &[u8])>, |
There was a problem hiding this comment.
Ahh fooey.
Yeah, we def need a to_raw_id method on RegId to pull this off...
On the bright side, if we add to_raw_id with a default stub impl (i.e: returning None), we can avoid making this a breaking change.
Lets go ahead and add to_raw_id, with some docs on the new method + on report_stop_with_regs commenting on how Arch implementations only need to implement the method if they want to use the report_stop_with_regs API.
For in-tree validation, please update the armv4t example to use report_stop_with_regs (e.g: to send back the PC, or whatever), and update the ArmCoreRegId type) with a non-trivial impl of the new method
|
All feedback addressed modulo one thread above -- thanks. |
daniel5151
left a comment
There was a problem hiding this comment.
This type-based approach looks good!
Error handling needs to be touched up a bit, but aside from that, LGTM
|
Updated; thanks! |
6d12dab to
5dfc89d
Compare
daniel5151
left a comment
There was a problem hiding this comment.
Great! LGTM.
With these 3 PRs merged, I'll go ahead and cut a new gdbstub release shortly :)
(and if you do have some spare cycles - it'd be great if you could upstream your impl Arch for Wasm implementation to gdbstub_arch!)
|
|
Builds on daniel5151#188, daniel5151#189, daniel5151#190: add definitions for the WebAssembly architecture to `gdbstub_arch`. This includes all definitions needed for the `Arch` trait, as well as utility code for encoding/decoding the synthetic address space used by the gdbstub definitions for this architecture. Tested with Wasmtime using a (soon to be upstreamed) guest-debugging facility.
|
Thanks so much! I've just sent #192 to upstream the |
Builds on daniel5151#188, daniel5151#189, daniel5151#190: add definitions for the WebAssembly architecture to `gdbstub_arch`. This includes all definitions needed for the `Arch` trait, as well as utility code for encoding/decoding the synthetic address space used by the gdbstub definitions for this architecture. Tested with Wasmtime using a (soon to be upstreamed) guest-debugging facility.
Builds on daniel5151#188, daniel5151#189, daniel5151#190: add definitions for the WebAssembly architecture to `gdbstub_arch`. This includes all definitions needed for the `Arch` trait, as well as utility code for encoding/decoding the synthetic address space used by the gdbstub definitions for this architecture. Tested with Wasmtime using a (soon to be upstreamed) guest-debugging facility.
Builds on #188, #189, #190: add definitions for the WebAssembly architecture to `gdbstub_arch`. This includes all definitions needed for the `Arch` trait, as well as utility code for encoding/decoding the synthetic address space used by the gdbstub definitions for this architecture. Tested with Wasmtime using a (soon to be upstreamed) guest-debugging facility.
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
This adds a debug component that makes use of the debug-main world defined in bytecodealliance#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 `.wasm` adapter component. Instead, the user simply runs `wasmtime run -g <PORT> program.wasm ...` and Wasmtime will load and prepare to run `program.wasm` as 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 `gdbstub` third-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.)
Description
This is required to work around an issue encountered with LLDB with a Wasm target over gdbstub: the Wasm-target client incorrectly caches (does not re-query) the PC, and needs to be updated after a stop either via a response with the
QListThreadsInStopReplycapability or by sending explicit updated register values directly in the T-stop packets. I opted for the latter here as it's a little simpler.API Stability
Checklist
rustdocformatting looks good (viacargo doc)examples/armv4twithRUST_LOG=trace+ any relevant GDB output under the "Validation" section below./example_no_std/check_size.shbefore/after changes under the "Validation" section below[ ] Included a basic sample implementation inexamples/armv4t[ ] IDET can be optimized out (confirmed via./example_no_std/check_size.sh)[ ] OR implementation requires introducing non-optional binary bloat (please elaborate under "Description")Archimplementation (N/A)[ ] I have tested this code in my project, and to the best of my knowledge, it is working as intended.Validation
I'm not able to show validation with just this PR; it requires #188 as well as a number of other things I haven't upstreamed to Wasmtime yet.