-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Update preview1 to trap on misaligned pointers #6776
Update preview1 to trap on misaligned pointers #6776
Conversation
Previously Wasmtime would return `EINVAL` to a guest but the upstream documentation indicates that misaligned pointers should trap, so this commit updates the alignment error to get converted into a trap rather than than being returned to the guest.
@pchickey I'd also be interested in your thoughts on the behvaior of out-of-bounds pointers as well. The linked documentation doesn't specifically say what to do in that case so I left it as-is with EFAULT for now, but should the docs perhaps be updated to indicate that a trap should happen in that situation as well? |
There's actually been an outstanding PR to update that language to trap, as well: WebAssembly/WASI#536. I agreed then we should merge it, but it fell off my radar. cc @yamt |
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 merged the pointer out of bounds changes upstream, so you can also change the PtrOutOfBounds
and PtrOverflow
cases to trap.
The new preview 1 implementation inside wasi/src/preview2/preview1 should also be updated as well. Unfortunately, it looks like instead of using a From impl there, it instead uses a pattern of Result<_, GuestError>::or(Err(Errno::Inval)) at each call site, so that is a tougher job. Its OK to not take that on as part of this PR, but we should file an issue on it if you want to defer it.
Sounds good! I think I covered all the bases here, but would be good to double-check |
… feature/preview2 * 'feature/preview2' of github.com:geekbeast/wasmtime: Change preview2 builder methods to use `&mut self` (bytecodealliance#6770) Add a bindgen test that exercises using error types from a different interface (bytecodealliance#6802) Resolve trappable error types with fully qualified package paths (bytecodealliance#6795) Update the dev-dependency for wit-bindgen to 0.9.0 (bytecodealliance#6800) Fix incorrect sample code in documentation (bytecodealliance#6796) (bytecodealliance#6797) Update preview1 to trap on misaligned pointers (bytecodealliance#6776) Fix posix-signals-on-macos on aarch64-apple-darwin (bytecodealliance#6793) consistient WASI preview1 rights reporting (bytecodealliance#6784) Wasmtime: Introduce `{Module,Component}::resources_required` (bytecodealliance#6789)
* Update preview1 to trap on misaligned pointers Previously Wasmtime would return `EINVAL` to a guest but the upstream documentation indicates that misaligned pointers should trap, so this commit updates the alignment error to get converted into a trap rather than than being returned to the guest. * Change OOB errors to traps too * Update preview2's implementation of preview1 * Handle some more errors
Previously Wasmtime would return
EINVAL
to a guest but the upstream documentation indicates that misaligned pointers should trap, so this commit updates the alignment error to get converted into a trap rather than than being returned to the guest.