4fcbd5bf23dcdd89e28e4d367d6cfe9f836e43f6
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Alex Crichton
|
2afaac5181 |
Return anyhow::Error from host functions instead of Trap, redesign Trap (#5149)
* Return `anyhow::Error` from host functions instead of `Trap` This commit refactors how errors are modeled when returned from host functions and additionally refactors how custom errors work with `Trap`. At a high level functions in Wasmtime that previously worked with `Result<T, Trap>` now work with `Result<T>` instead where the error is `anyhow::Error`. This includes functions such as: * Host-defined functions in a `Linker<T>` * `TypedFunc::call` * Host-related callbacks like call hooks Errors are now modeled primarily as `anyhow::Error` throughout Wasmtime. This subsequently removes the need for `Trap` to have the ability to represent all host-defined errors as it previously did. Consequently the `From` implementations for any error into a `Trap` have been removed here and the only embedder-defined way to create a `Trap` is to use `Trap::new` with a custom string. After this commit the distinction between a `Trap` and a host error is the wasm backtrace that it contains. Previously all errors in host functions would flow through a `Trap` and get a wasm backtrace attached to them, but now this only happens if a `Trap` itself is created meaning that arbitrary host-defined errors flowing from a host import to the other side won't get backtraces attached. Some internals of Wasmtime itself were updated or preserved to use `Trap::new` to capture a backtrace where it seemed useful, such as when fuel runs out. The main motivation for this commit is that it now enables hosts to thread a concrete error type from a host function all the way through to where a wasm function was invoked. Previously this could not be done since the host error was wrapped in a `Trap` that didn't provide the ability to get at the internals. A consequence of this commit is that when a host error is returned that isn't a `Trap` we'll capture a backtrace and then won't have a `Trap` to attach it to. To avoid losing the contextual information this commit uses the `Error::context` method to attach the backtrace as contextual information to ensure that the backtrace is itself not lost. This is a breaking change for likely all users of Wasmtime, but it's hoped to be a relatively minor change to workaround. Most use cases can likely change `-> Result<T, Trap>` to `-> Result<T>` and otherwise explicit creation of a `Trap` is largely no longer necessary. * Fix some doc links * add some tests and make a backtrace type public (#55) * Trap: avoid a trailing newline in the Display impl which in turn ends up with three newlines between the end of the backtrace and the `Caused by` in the anyhow Debug impl * make BacktraceContext pub, and add tests showing downcasting behavior of anyhow::Error to traps or backtraces * Remove now-unnecesary `Trap` downcasts in `Linker::module` * Fix test output expectations * Remove `Trap::i32_exit` This commit removes special-handling in the `wasmtime::Trap` type for the i32 exit code required by WASI. This is now instead modeled as a specific `I32Exit` error type in the `wasmtime-wasi` crate which is returned by the `proc_exit` hostcall. Embedders which previously tested for i32 exits now downcast to the `I32Exit` value. * Remove the `Trap::new` constructor This commit removes the ability to create a trap with an arbitrary error message. The purpose of this commit is to continue the prior trend of leaning into the `anyhow::Error` type instead of trying to recreate it with `Trap`. A subsequent simplification to `Trap` after this commit is that `Trap` will simply be an `enum` of trap codes with no extra information. This commit is doubly-motivated by the desire to always use the new `BacktraceContext` type instead of sometimes using that and sometimes using `Trap`. Most of the changes here were around updating `Trap::new` calls to `bail!` calls instead. Tests which assert particular error messages additionally often needed to use the `:?` formatter instead of the `{}` formatter because the prior formats the whole `anyhow::Error` and the latter only formats the top-most error, which now contains the backtrace. * Merge `Trap` and `TrapCode` With prior refactorings there's no more need for `Trap` to be opaque or otherwise contain a backtrace. This commit parse down `Trap` to simply an `enum` which was the old `TrapCode`. All various tests and such were updated to handle this. The main consequence of this commit is that all errors have a `BacktraceContext` context attached to them. This unfortunately means that the backtrace is printed first before the error message or trap code, but given all the prior simplifications that seems worth it at this time. * Rename `BacktraceContext` to `WasmBacktrace` This feels like a better name given how this has turned out, and additionally this commit removes having both `WasmBacktrace` and `BacktraceContext`. * Soup up documentation for errors and traps * Fix build of the C API Co-authored-by: Pat Hickey <pat@moreproductive.org> |
||
|
Alex Crichton
|
543a487939 |
Throw out fewer fuzz inputs with differential fuzzer (#4859)
* Throw out fewer fuzz inputs with differential fuzzer Prior to this commit the differential fuzzer would generate a module and then select an engine to execute the module against Wasmtime. This meant, however, that the candidate list of engines were filtered against the configuration used to generate the module to ensure that the selected engine could run the generated module. This commit inverts this logic and instead selects an engine first, allowing the engine to then tweak the module configuration to ensure that the generated module is compatible with the engine selected. This means that fewer fuzz inputs are discarded because every fuzz input will result in an engine being executed. Internally the engine constructors have all been updated to update the configuration to work instead of filtering the configuration. Some other fixes were applied for the spec interpreter as well to work around #4852 * Fix tests |
||
|
Alex Crichton
|
10dbb19983 |
Various improvements to differential fuzzing (#4845)
* Improve wasmi differential fuzzer * Support modules with a `start` function * Implement trap-matching to ensure that wasmi and Wasmtime both report the same flavor of trap. * Support differential fuzzing where no engines match Locally I was attempting to run against just one wasm engine with `ALLOWED_ENGINES=wasmi` but the fuzzer quickly panicked because the generated test case didn't match wasmi's configuration. This commit updates engine-selection in the differential fuzzer to return `None` if no engine is applicable, throwing out the test case. This won't be hit at all with oss-fuzz-based runs but for local runs it'll be useful to have. * Improve proposal support in differential fuzzer * De-prioritize unstable wasm proposals such as multi-memory and memory64 by making them more unlikely with `Unstructured::ratio`. * Allow fuzzing multi-table (reference types) and multi-memory by avoiding setting their maximums to 1 in `set_differential_config`. * Update selection of the pooling strategy to unconditionally support the selected module config rather than the other way around. * Improve handling of traps in differential fuzzing This commit fixes an issue found via local fuzzing where engines were reporting different results but the underlying reason for this was that one engine was hitting stack overflow before the other. To fix the underlying issue I updated the execution to check for stack overflow and, if hit, it discards the entire fuzz test case from then on. The rationale behind this is that each engine can have unique limits for stack overflow. One test case I was looking at for example would stack overflow at less than 1000 frames with epoch interruption enabled but would stack overflow at more than 1000 frames with it disabled. This means that the state after the trap started to diverge and it looked like the engines produced different results. While I was at it I also improved the "function call returned a trap" case to compare traps to make sure the same trap reason popped out. * Fix fuzzer tests |
||
|
Alex Crichton
|
fd98814b96 |
Port v8 fuzzer to the new framework (#4739)
* Port v8 fuzzer to the new framework This commit aims to improve the support for the new "meta" differential fuzzer added in #4515 by ensuring that all existing differential fuzzing is migrated to this new fuzzer. This PR includes features such as: * The V8 differential execution is migrated to the new framework. * `Config::set_differential_config` no longer force-disables wasm features, instead allowing them to be enabled as per the fuzz input. * `DiffInstance::{hash, hash}` was replaced with `DiffInstance::get_{memory,global}` to allow more fine-grained assertions. * Support for `FuncRef` and `ExternRef` have been added to `DiffValue` and `DiffValueType`. For now though generating an arbitrary `ExternRef` and `FuncRef` simply generates a null value. * Arbitrary `DiffValue::{F32,F64}` values are guaranteed to use canonical NaN representations to fix an issue with v8 where with the v8 engine we can't communicate non-canonical NaN values through JS. * `DiffEngine::evaluate` allows "successful failure" for cases where engines can't support that particular invocation, for example v8 can't support `v128` arguments or return values. * Smoke tests were added for each engine to ensure that a simple wasm module works at PR-time. * Statistics printed from the main fuzzer now include percentage-rates for chosen engines as well as percentage rates for styles-of-module. There's also a few small refactorings here and there but mostly just things I saw along the way. * Update the fuzzing README |