add basic coredump generation#5868
Conversation
| if let Some(coredump_path) = self.coredump_on_trap.as_ref() { | ||
| if coredump_path.contains("%") { | ||
| return Err(anyhow!( | ||
| "The coredump-on-trap path does not support patterns yet." |
There was a problem hiding this comment.
For context, unix kernels allow to template the coredump path (https://sigquit.wordpress.com/2009/03/13/the-core-pattern/) using variables starting with %. We might want to support them in the future.
github-actions
Bot
added
the
wasmtime:docs
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks!
One thought I have with this is that while placing it in the CLI is perhaps a good starting point we'll likely want this in a more libararified form at some point too because embeddings of Wasmtime don't necessarily all use the CLI. This current integration is also somewhat limited because the native call stack is gone and as a result impossible to ever recover locals and their information. Basically the "ideal" location for a core dump is probably at the place of the trap itself, which means that this wouldn't be possible to do from the CLI but rather would be required to be baked-in as well. Additionally being baked-in I think will be required to get access to non-exported globals/memories/etc.
None of that is to say though that this shouldn't be added here at this time. Moreso that a more "final" integration of core dumps I think will probably look significantly differently architected which isn't a problem but perhaps worth considering.
xtuc
force-pushed
the
sven/basic-coredump
branch
2 times, most recently
from
3d06eda to
0a1843b
Compare
|
I changed the coredump generation to gracefully handle errors, as you pointed out. The dependencies are now trimmed down. Could you please have another look @alexcrichton? |
|
|
||
| let coredump = coredump_builder | ||
| .serialize() | ||
| .map_err(|err| anyhow!("failed to serialize coredump: {}", err))?; |
There was a problem hiding this comment.
Could the .map_err here become .context(..)?
There was a problem hiding this comment.
The error is from wasm-coredump-builder and is type Box<dyn std::error::Error + Sync + Send> instead of the usual anyhow::Error. Is that a problem?
There was a problem hiding this comment.
It's a problem insofar as the error's contexet is all lost here, only displaying one error in a possible chain of errors contained within the trait object. Whether your library exercises that I'm not sure, though.
There was a problem hiding this comment.
It's not a problem for my library, for now, as it doesn't add context to errors.
xtuc
force-pushed
the
sven/basic-coredump
branch
3 times, most recently
from
db931d7 to
0712e8f
Compare
Required by bytecodealliance#5868
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
I've added the new versions to #5894
|
|
||
| let coredump = coredump_builder | ||
| .serialize() | ||
| .map_err(|err| anyhow!("failed to serialize coredump: {}", err))?; |
There was a problem hiding this comment.
It's a problem insofar as the error's contexet is all lost here, only displaying one error in a possible chain of errors contained within the trait object. Whether your library exercises that I'm not sure, though.
This change adds a basic coredump generation after a WebAssembly trap was entered. The coredump includes rudimentary stack / process debugging information. A new CLI argument is added to enable coredump generation: ``` wasmtime --coredump-on-trap=/path/to/coredump/file module.wasm ``` See ./docs/examples-coredump.md for a working example. Refs bytecodealliance#5732
xtuc
force-pushed
the
sven/basic-coredump
branch
from
0712e8f to
e9f89f1
Compare
3 participants
This change adds a basic coredump generation after a WebAssembly trap was entered. The coredump includes rudimentary stack / process debugging information.
A new CLI argument is added to enable coredump generation:
See ./docs/examples-coredump.md for a working example.
Refs #5732