Wasmtime GitHub notifications bot (Jan 06 2023 at 23:32): Wasmtime GitHub notifications bot (Jan 06 2023 at 23:32):abrown opened PR #5543 from wiggle-copied-reads-fix-underflow to main:
This change fixes an issue identified when using wasi-threads to perform file reads. In order to maintain Rust safety guarantees in the presence of WebAssembly shared memory, which can be modified concurrently by any of the running threads, the WASI implementations of
fd_readandfd_preadwere given special code paths when shared memory is detected: in these cases, the data is first read into a host-limited buffer and then subsequently copied into linear memory. The problem was that the rather-complex logic for doing this "buffer then copy" idea for multiple IO vectors could fail due to buffer underflow. If, e.g., a read was limited by the host to 64K (or even if the read returned less than the total buffer size) theUnsafeGuestSlice::copy_from_slicelogic would fail, complaining that the sizes of both buffers were unequal.This change both simplifies and fixes the logic:
- only the first IO vector is filled; this could represent a performance penalty for threaded programs, but the "buffer then copy" idea already imposes a non-trivial overhead. This simplifies the logic, allowing us to...
- resize the shared memory buffer to the exact number of bytes read
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Jan 09 2023 at 14:48):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 09 2023 at 14:48):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 09 2023 at 14:48):alexcrichton created PR review comment:
I think in this case it's ok to do
return Ok(0)since that's really all that can happen here anyway.
Wasmtime GitHub notifications bot (Jan 09 2023 at 18:49):abrown updated PR #5543 from wiggle-copied-reads-fix-underflow to main.
Wasmtime GitHub notifications bot (Jan 09 2023 at 18:49):abrown has marked PR #5543 as ready for review.
Wasmtime GitHub notifications bot (Jan 09 2023 at 18:58):abrown updated PR #5543 from wiggle-copied-reads-fix-underflow to main.
Wasmtime GitHub notifications bot (Jan 09 2023 at 19:25):abrown has enabled auto merge for PR #5543.
Wasmtime GitHub notifications bot (Jan 09 2023 at 19:28):abrown merged PR #5543.
Last updated: Dec 23 2024 at 12:05 UTC