[pull] main from WebAssembly:main #66
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The 2nd ret type of these opcodes are different from the wasm spec
The interpreter could overflow the stack without trapping properly in `call_indirect` situations. While it would set the `out_trap` to the trap reason, it would return `RunResult::Ok` and the interpreter code would only check `RunResult::Ok` to decide whether or not to keep running. In other words, while the stack overflow meant the interpreter wouldn't push a frame onto the call stack, the interpreter loop would continue advancing instructions, resulting in instructions after the runaway `call_indirect` running. If the offending `call_indirect` didn't have return values, it would be as if the call returned normally. If it did have return values, nothing would be pushed onto the value stack, yet the return types would be pushed onto the type stack. With careful manipulation of the following instructions, this could be used to cause all sorts of memory corruption. As it turns out, the function exit code, as well as a handful of other instructions, do check the state of the value and type stacks and can safely reproduce the bug without the memory corruption, so that's what we made the test do. The obvious fix was to make `call_indirect` propagate `RunResult::Trap` properly. Additionally, we made it so `assert_exhaustion` checks both the `RunResult` *and* the `out_trap`, and asserts if they don't match. This should help catch similar bugs in the future. Closes #2462 Fixes #2398
This makes `wasm_rt_init()` properly reentrant. By setting `g_alt_stack = NULL`, we allow execution to continue beyond [wasm-rt-impl.c L171 ](https://github.com/WebAssembly/wabt/blob/main/wasm2c/wasm-rt-impl.c#L171) in `os_allocate_and_install_altstack`. Applies to debug builds only due to `assert`.
Clang 17(?) tightened UBSAN checks, so that you now get this: ``` - test/wasm2c/spec/call_indirect.txt expected error code 0, got 1. STDERR MISMATCH: --- expected +++ actual @@ -0,0 +1,3 @@ +out/test/wasm2c/spec/call_indirect/call_indirect.0.c:2144:12: runtime error: call to function w2c_call__indirect__0__wasm_f0 through pointer to incorrect function type 'unsigned int (*)(void *)' +/home/runner/work/wabt/wabt/out/test/wasm2c/spec/call_indirect/call_indirect.0.c:1925: note: w2c_call__indirect__0__wasm_f0 defined here +SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior out/test/wasm2c/spec/call_indirect/call_indirect.0.c:2144:12 STDOUT MISMATCH: --- expected +++ actual @@ -1 +0,0 @@ -134/134 tests passed. ``` This happens because emitted functions use a typed module instance, while function references use a `void*` instance. It is UB in C to call the former with the latter, so clang is correct here. We had to pick one of two ways to fix this: either emit `void*` wrapper functions that do the appropriate downcasting for any module functions that go into a table (potentially including imported functions), or the approach that takes significantly less effort of changing everything to `void*` and downcasting internally. ~~We obviously chose the latter.~~ We eventually started emitting wrapper functions.
Fixes #2453 in a bit of a silly way. (Conveniently, we already have tests for this, but nobody noticed they were broken.)
Same issue as #2471 but for `call_ref`. We don't believe there's a prior issue for this.
Fixes the value stack size of the catch handler. There were two (related) issues here: - The previous code used `func_->locals.size()` as soon as the function was available, but it hadn't processed the function's locals yet, so it was always empty. (This might not matter in practice, as it's only used by the "function-wide catch handler", which just rethrows.) - The previous code didn't take the function's locals into account when computing the value stack height (relative to the function frame) for a try-catch block. So, it would drop the locals when catching an exception. Closes #2476 (Split from #2470 )
It is UB to read local variables after a call to `setjmp` returns, if those variables have been modified between `setjmp` and `longjmp`, unless they're marked as `volatile`. This marks them as `volatile`. Closes #2469
local decl count != local count
Previously, the parser would return result::Error, but would not populate an error message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )