-
Notifications
You must be signed in to change notification settings - Fork 821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for GDB JIT debugging #1212
Conversation
@@ -78,6 +78,11 @@ where | |||
pub fn into_vec(self) -> Vec<V> { | |||
self.elems | |||
} | |||
|
|||
/// Iterate over the values of the map in order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Iterate over the values of the map in order | |
/// Iterate over the values of the map in order. |
Co-authored-by: Nick Lewycky <nick@wasmer.io>
lib/runtime-core/src/codegen.rs
Outdated
@@ -296,6 +364,7 @@ impl MiddlewareChain { | |||
fcg: Option<&mut FCG>, | |||
ev: Event, | |||
module_info: &ModuleInfo, | |||
loc: u32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we changing the public API? If so, is there any way to either:
- Have this added into the CHANGELOG as [BREAKING CHANGE]
- Minimize this? (not sure how, we will have to research)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work, thanks!
I left few notes. In jit_debug.rs
, sometimes we use JITFoo
, sometimes JitFoo
. We should make it uniform if you think it is worth the time.
I didn't test the code. I only made a review.
ATTRIBUTIONS.md
Outdated
of generating debug information for each function with Cranelift | ||
(for example, the sorting of the extended basic blocks before | ||
processing the instructions), and the API for transforming DWARF | ||
see wasm-debug's attribution file for more information (TODO: link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we address the TODO
before the merge?
CHANGELOG.md
Outdated
@@ -6,6 +6,7 @@ | |||
|
|||
- [#1233](https://github.com/wasmerio/wasmer/pull/1233) Improved Wasmer C API release artifacts. | |||
- [#1216](https://github.com/wasmerio/wasmer/pull/1216) `wasmer-interface-types` receives a binary encoder. | |||
- [#1212](https://github.com/wasmerio/wasmer/pull/1212) Add `--generate-debug-info` and `-g` flags to `wasmer run` to generate debug information during compilation that is passed via the GDB JIT interface to a debugger to allow source-level debugging of Wasm files. Currently only available on clif-backend, see PR for more information on its implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two spaces before “Currently”:
- [#1212](https://github.com/wasmerio/wasmer/pull/1212) Add `--generate-debug-info` and `-g` flags to `wasmer run` to generate debug information during compilation that is passed via the GDB JIT interface to a debugger to allow source-level debugging of Wasm files. Currently only available on clif-backend, see PR for more information on its implementation. | |
- [#1212](https://github.com/wasmerio/wasmer/pull/1212) Add `--generate-debug-info` and `-g` flags to `wasmer run` to generate debug information during compilation that is passed via the GDB JIT interface to a debugger to allow source-level debugging of Wasm files. Currently only available on clif-backend, see PR for more information on its implementation. |
Also, should we adopt the classic compiler -O
option? -O0
, -O1
, -O2
, -Oz
etc.? I'll open a new issue to discuss about that. I'm even thinking about a -C
option.
Please read #1237 before merging this PR 🙂.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a big fan of two spaces after periods. I try to suppress it when writing in a shared space because inconsistent style is worse, but I find it much more readable, it chunks the sentences in my brain better and helps me read more quickly.
From Googling it, using two spaces after a period apparently means I'm over 40 years old and stuck to my ways using the typewriter 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's quite a few instances of \.
in CHANGELOG.md haha, probably all from me
lib/clif-backend/src/code.rs
Outdated
@@ -71,6 +71,7 @@ impl ModuleCodeGenerator<CraneliftFunctionCodeGenerator, Caller, CodegenError> | |||
fn next_function( | |||
&mut self, | |||
module_info: Arc<RwLock<ModuleInfo>>, | |||
loc: (u32, u32), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we use .0
and .1
more than once in this code, can we imagine a LocationSpan
structure, with the start
and end
fields?
/// Data about the the compiled machine code. | ||
type CompileMetadata = ( | ||
LocalFuncIndex, | ||
Option<(CompiledFunctionData, ValueLabelsRanges, Vec<Option<i32>>)>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i32
in Vec<Option<i32>>
refers to locations? If yes, why not u32
instead?
Also, maybe it is interesting to add type Location = u32
. I think it would clarify the code for such complex type. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, it came from the stackslot type that wasm-debug
used to depend on. The offset it returns is a number. I believe they're usually negative numbers actually because they're in terms of RBP and not RSP 🤔
lib/runtime-core/src/jit_debug.rs
Outdated
@@ -0,0 +1,195 @@ | |||
//! Code for interacting with the | |||
//! [GDB JIT inteface](https://sourceware.org/gdb/current/onlinedocs/gdb.html#JIT-Interface). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
//! [GDB JIT inteface](https://sourceware.org/gdb/current/onlinedocs/gdb.html#JIT-Interface). | |
//! [GDB JIT interface](https://sourceware.org/gdb/current/onlinedocs/gdb.html#JIT-Interface). |
lib/runtime-core/src/jit_debug.rs
Outdated
/// Node of the doubly linked list that the GDB JIT interface reads from. | ||
#[no_mangle] | ||
#[repr(C)] | ||
pub(crate) struct JITCodeEntry { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want JITCodeEntry
to be public in the crate? I don't see any reason. Am I missing something?
pub(crate) struct JITCodeEntry { | |
struct JITCodeEntry { |
lib/runtime-core/src/jit_debug.rs
Outdated
/// Head node of the doubly linked list that the GDB JIT interface expects. | ||
#[no_mangle] | ||
#[repr(C)] | ||
pub(crate) struct JitDebugDescriptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want JitDebugDescriptor
to be public in the crate? I don't see any reason. Am I missing something?
pub(crate) struct JitDebugDescriptor { | |
struct JitDebugDescriptor { |
lib/runtime-core/src/jit_debug.rs
Outdated
/// debugger to perform. | ||
#[no_mangle] | ||
#[allow(non_upper_case_globals)] | ||
pub(crate) static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want __jit_debug_descriptor
to be public in the crate? I don't see any reason. Am I missing something?
pub(crate) static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor { | |
static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor { |
lib/runtime-core/src/jit_debug.rs
Outdated
|
||
/// Manager of debug info registered with the debugger. | ||
#[derive(Debug, Clone, Default)] | ||
pub struct JITCodeDebugInfoManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub struct JITCodeDebugInfoManager { | |
pub(crate) struct JITCodeDebugInfoManager { |
since there is only one field, which is private, and one method, which is visible to the crate only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of module which has all public fields. I wasn't sure if I should make the first private field in the struct or just make it public. Making it public seemed like the safer choice
lib/runtime-core/src/module.rs
Outdated
#[cfg(feature = "generate-debug-information")] | ||
#[serde(skip)] | ||
/// Resource manager of debug information being used by a debugger. | ||
pub debug_info_manager: jit_debug::JITCodeDebugInfoManager, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub debug_info_manager: jit_debug::JITCodeDebugInfoManager, | |
pub(crate) debug_info_manager: jit_debug::JITCodeDebugInfoManager, |
related to my comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was a bit worried about the implications of making the first non-public field, but I suppose it'll be fine
Co-authored-by: Ivan Enderlin <ivan.enderlin@wanadoo.fr>
1212: Add support for GDB JIT debugging r=MarkMcCaskey a=MarkMcCaskey This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of `wasmtime-debug`. The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's `wasmtime-debug` crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of the `wasmtime-debug` -- which will be published and uploaded in another repo. This PR started out implementing the [Wasm-DWARF](https://yurydelendik.github.io/webassembly-dwarf/) reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of `wasmtime-debug` and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring some `transmute`s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered. The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the `ebb`s in `clif_backend::resolver`. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration with `wasmtime-debug` and some bits from `cranelift-wasm` and `cranelift-codegen`. If there's interest from other people in working on the generic `wasmtime-debug` fork, I'm happy to get other maintainers involved and/or move it to a shared organization. Special thanks to [Yury Delendik](https://github.com/yurydelendik) and the other `wasmtime-debug` authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data. ### TODO: - [x] Update attributions file for LLVM, [wasm-dwarf](https://github.com/yurydelendik/wasm-dwarf), and Wasmtime/Cranelift and do another pass over code to make sure we're in compliance with the licenses from the relevant projects and have properly attributed the code used from other projects. - [x] Adjust API of wasm-debug based on feedback - [x] Discuss with Nick integration with LLVM - [x] Discuss with Heyang integration with Singlepass - [x] Adjust implementation based on feedback from team (traits modified, etc.) - [x] Clean up some pointer wrangling code - [x] Add opt-in feature to wasmer-runtime-core to enale wasm-debug so library users who won't use debug info are not affected # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Build failed
|
This is required because LLVM exposes its own
fed7473
to
a089cf5
Compare
bors r+ |
1212: Add support for GDB JIT debugging r=MarkMcCaskey a=MarkMcCaskey This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of `wasmtime-debug`. The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's `wasmtime-debug` crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of the `wasmtime-debug` -- which will be published and uploaded in another repo. This PR started out implementing the [Wasm-DWARF](https://yurydelendik.github.io/webassembly-dwarf/) reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of `wasmtime-debug` and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring some `transmute`s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered. The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the `ebb`s in `clif_backend::resolver`. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration with `wasmtime-debug` and some bits from `cranelift-wasm` and `cranelift-codegen`. If there's interest from other people in working on the generic `wasmtime-debug` fork, I'm happy to get other maintainers involved and/or move it to a shared organization. Special thanks to [Yury Delendik](https://github.com/yurydelendik) and the other `wasmtime-debug` authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data. ### TODO: - [x] Update attributions file for LLVM, [wasm-dwarf](https://github.com/yurydelendik/wasm-dwarf), and Wasmtime/Cranelift and do another pass over code to make sure we're in compliance with the licenses from the relevant projects and have properly attributed the code used from other projects. - [x] Adjust API of wasm-debug based on feedback - [x] Discuss with Nick integration with LLVM - [x] Discuss with Heyang integration with Singlepass - [x] Adjust implementation based on feedback from team (traits modified, etc.) - [x] Clean up some pointer wrangling code - [x] Add opt-in feature to wasmer-runtime-core to enale wasm-debug so library users who won't use debug info are not affected # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Build failed |
bors r+ |
1212: Add support for GDB JIT debugging r=MarkMcCaskey a=MarkMcCaskey This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of `wasmtime-debug`. The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's `wasmtime-debug` crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of the `wasmtime-debug` -- which will be published and uploaded in another repo. This PR started out implementing the [Wasm-DWARF](https://yurydelendik.github.io/webassembly-dwarf/) reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of `wasmtime-debug` and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring some `transmute`s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered. The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the `ebb`s in `clif_backend::resolver`. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration with `wasmtime-debug` and some bits from `cranelift-wasm` and `cranelift-codegen`. If there's interest from other people in working on the generic `wasmtime-debug` fork, I'm happy to get other maintainers involved and/or move it to a shared organization. Special thanks to [Yury Delendik](https://github.com/yurydelendik) and the other `wasmtime-debug` authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data. ### TODO: - [x] Update attributions file for LLVM, [wasm-dwarf](https://github.com/yurydelendik/wasm-dwarf), and Wasmtime/Cranelift and do another pass over code to make sure we're in compliance with the licenses from the relevant projects and have properly attributed the code used from other projects. - [x] Adjust API of wasm-debug based on feedback - [x] Discuss with Nick integration with LLVM - [x] Discuss with Heyang integration with Singlepass - [x] Adjust implementation based on feedback from team (traits modified, etc.) - [x] Clean up some pointer wrangling code - [x] Add opt-in feature to wasmer-runtime-core to enale wasm-debug so library users who won't use debug info are not affected # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Build failed
|
bors r+ |
1212: Add support for GDB JIT debugging r=MarkMcCaskey a=MarkMcCaskey This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of `wasmtime-debug`. The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's `wasmtime-debug` crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of the `wasmtime-debug` -- which will be published and uploaded in another repo. This PR started out implementing the [Wasm-DWARF](https://yurydelendik.github.io/webassembly-dwarf/) reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of `wasmtime-debug` and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring some `transmute`s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered. The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the `ebb`s in `clif_backend::resolver`. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration with `wasmtime-debug` and some bits from `cranelift-wasm` and `cranelift-codegen`. If there's interest from other people in working on the generic `wasmtime-debug` fork, I'm happy to get other maintainers involved and/or move it to a shared organization. Special thanks to [Yury Delendik](https://github.com/yurydelendik) and the other `wasmtime-debug` authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data. ### TODO: - [x] Update attributions file for LLVM, [wasm-dwarf](https://github.com/yurydelendik/wasm-dwarf), and Wasmtime/Cranelift and do another pass over code to make sure we're in compliance with the licenses from the relevant projects and have properly attributed the code used from other projects. - [x] Adjust API of wasm-debug based on feedback - [x] Discuss with Nick integration with LLVM - [x] Discuss with Heyang integration with Singlepass - [x] Adjust implementation based on feedback from team (traits modified, etc.) - [x] Clean up some pointer wrangling code - [x] Add opt-in feature to wasmer-runtime-core to enale wasm-debug so library users who won't use debug info are not affected # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Build failed |
bors r+ |
1212: Add support for GDB JIT debugging r=MarkMcCaskey a=MarkMcCaskey This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of `wasmtime-debug`. The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's `wasmtime-debug` crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of the `wasmtime-debug` -- which will be published and uploaded in another repo. This PR started out implementing the [Wasm-DWARF](https://yurydelendik.github.io/webassembly-dwarf/) reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of `wasmtime-debug` and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring some `transmute`s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered. The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the `ebb`s in `clif_backend::resolver`. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration with `wasmtime-debug` and some bits from `cranelift-wasm` and `cranelift-codegen`. If there's interest from other people in working on the generic `wasmtime-debug` fork, I'm happy to get other maintainers involved and/or move it to a shared organization. Special thanks to [Yury Delendik](https://github.com/yurydelendik) and the other `wasmtime-debug` authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data. ### TODO: - [x] Update attributions file for LLVM, [wasm-dwarf](https://github.com/yurydelendik/wasm-dwarf), and Wasmtime/Cranelift and do another pass over code to make sure we're in compliance with the licenses from the relevant projects and have properly attributed the code used from other projects. - [x] Adjust API of wasm-debug based on feedback - [x] Discuss with Nick integration with LLVM - [x] Discuss with Heyang integration with Singlepass - [x] Adjust implementation based on feedback from team (traits modified, etc.) - [x] Clean up some pointer wrangling code - [x] Add opt-in feature to wasmer-runtime-core to enale wasm-debug so library users who won't use debug info are not affected # Review - [x] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Build failed
|
bors r+ |
Build succeeded
|
How should I interprete the new We're implementing a impl FunctionMiddleware for DeterministicMiddleware {
type Error = CompileError;
fn feed_event<'a, 'b: 'a>(
&mut self,
event: Event<'a, 'b>,
_module_info: &ModuleInfo,
sink: &mut EventSink<'a, 'b>,
source_loc: u32,
) -> Result<(), Self::Error> {
match event {
Event::Wasm(op) => parse_wasm_opcode(op)?,
Event::WasmOwned(ref op) => parse_wasm_opcode(op)?,
_ => (),
};
sink.push(event);
Ok(())
}
} and could pass the new value into the error message if that makes sense. |
@webmaster128 |
Thanks a lot, @MarkMcCaskey, that's very helpful. I'll ignore the value then. I've no place to forward it to. |
Sure! We'll update the CHANGELOG.md with any breaking changes, so if the Wasm DWARF standard does change, we'd document it there. So if you check that when updating Wasmer, you shouldn't have any bad surprises like the meaning of |
Hi, was this feature removed? I can't seem to trace why it was deprecated, was it due to lack of maintenance? Seems like a pretty important feature to leave out. |
It was removed, but that looks like a nice feature, so I'll check if it can be added back. |
This PR adds support for JIT debugging to Wasmer with the Cranelift backend using a fork of
wasmtime-debug
.The motivation for this change is partially inspired by the feature in Wasmtime and the implementation is largely derived from Wasmtime's
wasmtime-debug
crate and not included in this PR. This implementation is currently Cranelift-only (if LLVM has value tracking we can add this there too without too much effort; we'd have to do the value tracking ourselves in Singlepass and I don't have enough context to know how hard that would be) and is based on a generic fork of thewasmtime-debug
-- which will be published and uploaded in another repo.This PR started out implementing the Wasm-DWARF reading and writing with gimli but after working on it for a few days, reading a chunk of the DWARF spec, seeing that Wasmtime had solved this well, and realizing how long this would likely take, I decided that it didn't make sense to spend the engineering effort there so I made a copy of
wasmtime-debug
and removed some of the less portable Cranelift pieces (very minor changes) and all code relying on data structures from wasmtime. The resulting crate is completely generic and would work fine with Wasmtime or any other Wasm runtime at the cost of requiring sometransmute
s or a linear pass over the debug data to reconstruct it in terms of the new types exposed by the fork. Perhaps there's a cleaner way to handle that that I haven't considered.The integration with the GDB JIT interface is from the LLVM examples (I don't remember if I properly attributed everything in this PR/version of the code -- I still have the other branches locally though which I'll review before shipping this) and some of the code in this PR is from Wasmtime/Cranelift source code such as the sorting of the
ebb
s inclif_backend::resolver
. I spent a long time debugging some subtle bugs and ended up using a few things from Wasmtime's integration withwasmtime-debug
and some bits fromcranelift-wasm
andcranelift-codegen
.If there's interest from other people in working on the generic
wasmtime-debug
fork, I'm happy to get other maintainers involved and/or move it to a shared organization.Special thanks to Yury Delendik and the other
wasmtime-debug
authors for their work on Wasm debugging. Also shout out to Cranelift for the nice API for tracking variables/data.TODO:
Review