Add Trap::trap_code #2309
Add Trap::trap_code #2309
Conversation
github-actions
bot
added
the
wasmtime:api
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
|
Thanks for the PR! I don't think we'll want to expose the cranelift-internal |
leoyvens
force-pushed
the
trap-is-user
branch
from
728493e
to
9dcb93b
Compare
|
@alexcrichton That's a good suggestion, for my personal use case I'd prefer that it were exhaustive but I get that for versioning it's best for it to not be. |
There was a problem hiding this comment.
Looking good, thanks! I left some comments about the documentation, but I think you'll need to reexport TrapCode from the top-level to make it accessbile as well. Can you additionally add some tests which assert expected values of the trap code from running wasm?
| BadSignature, | ||
|
|
||
| /// An integer arithmetic operation caused an overflow. | ||
| IntegerOverflow, |
There was a problem hiding this comment.
I'm not sure if this can actually arise from wasm code? We may not need to include this?
There was a problem hiding this comment.
This does happen for non-saturating truncation and this specific case of division https://github.com/WebAssembly/spec/blob/55a5e2c88890ccbe2b992f571b95704234977dac/interpreter/exec/int.ml#L131.
| @@ -174,6 +251,14 @@ impl Trap { | |||
| pub fn trace(&self) -> &[FrameInfo] { | |||
| &self.inner.wasm_trace | |||
| } | |||
|
|
|||
| /// Code of a trap that happened while executing a WASM instruction. | |||
There was a problem hiding this comment.
Could this be expanded a bit to explain when it's None and when it's Some?
|
@alexcrichton Thanks for the speedy review, I've addressed all suggested changes. |
github-actions
bot
added
the
cranelift
crates/wasmtime/src/trap.rs
Outdated
| }; | ||
| let trap = err.downcast_ref::<Trap>().unwrap(); | ||
| assert_eq!(trap.trap_code(), Some(TrapCode::MemoryOutOfBounds)); | ||
| } |
There was a problem hiding this comment.
Could this get moved to tests/all/*.rs? That's were we store most other tests.
There was a problem hiding this comment.
Also, while you're at it, mind simplifying this a bit with a helper function?
fn assert_trap_code(wat: &str, code: ir::TrapCode)or something like that?
|
@alexcrichton Done! |
|
Thanks again for this! |
* add Trap::trap_code * Add non-exhaustive wasmtime::TrapCode * wasmtime: Better document TrapCode * move and refactor test
The public
Trapin wasmtime is right now very opaque compared to the internal information which is available about the trap reason. This exposes the trap code of a wasm trap, which is useful to for example differentiate traps from host functions to traps coming from wasm itself.