Report a host limit for memories too large for the shell to allocate#8898
Conversation
The fuzzer found (in WebAssembly#8882) that a maximal memory64 (2^48 pages, exactly 2^64 bytes) instantiated in the shell interface with the byte size wrapped to 0: instantiation "succeeded" with an empty buffer and then every memory access spuriously trapped as out of bounds, including the memory.init of active segments. Optimizations that legitimately remove such accesses (like MemoryPacking dropping a provably in-bounds zero segment) then changed observable behavior, tripping the fuzzer. Instead, report a host limit during instantiation when the byte size is unrepresentable or the allocation fails, like a real VM whose instantiation fails. The fuzzer already ignores host limits reached during instance creation, and growMemory already applies a size limit, so this brings init in line with existing policy.
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Thanks, looks good with one minor comment.
| @@ -0,0 +1,16 @@ | |||
| ;; NOTE: Assertions have been generated by update_lit_checks.py --output=fuzz-exec and should not be edited. | |||
|
|
|||
| ;; RUN: wasm-opt %s -all --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s | |||
There was a problem hiding this comment.
| ;; RUN: wasm-opt %s -all --fuzz-exec -q -o /dev/null 2>&1 | filecheck %s | |
| ;; RUN: wasm-opt %s -all --fuzz-exec-before -q -o /dev/null 2>&1 | filecheck %s |
There was a problem hiding this comment.
This will execute it once, not twice (fuzz-exec runs one before opts, once after, but there are no opts here).
There was a problem hiding this comment.
Done, thanks. The checks regenerate to a single [host limit memory too large] line, which is also a nicer statement of what the test is about.
| uint64_t bytes = uint64_t(memory->initial) << memory->pageSizeLog2; | ||
| if ((bytes >> memory->pageSizeLog2) != uint64_t(memory->initial)) { | ||
| // The byte size is not even representable; we certainly cannot | ||
| // allocate it, just like a real VM could not. | ||
| hostLimit("memory too large"); | ||
| } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
That works for validated modules: as we established on #8882, under the validator's bound the only value the shift can wrap to is exactly 0. The round trip form additionally stays correct for modules that skipped validation (--no-validation), where an out of range initial can wrap to a nonzero value that !bytes would silently accept as a real size. Since both compile to the same couple of instructions I kept the self contained one, but happy to switch if @kripken prefers the shorter form.
There was a problem hiding this comment.
We can assume that modules being executed are valid. The only place a module might be invalid is during parsing, basically. But I think the current code is clearer as it stands, as it is self-contained - the short form would need a longer comment, in my opinion.
There was a problem hiding this comment.
Also, the validation limit might change if the spec changes, in theory...
Fixes the fuzzer finding from #8882 (comment): a maximal memory64 (2^48 pages, exactly 2^64 bytes) hit
shellMemory.resize(memory->initial << memory->pageSizeLog2)in the shell interface, where the byte size wrapped to 0. Instantiation then "succeeded" with an empty buffer, and every memory access spuriously trapped as out of bounds, including thememory.initof active segments during startup. When an optimization legitimately removes such an access (MemoryPacking dropping a provably in-bounds zero segment), the spurious trap disappears and the fuzzer sees a behavior change.The fix reports a host limit at instantiation when the byte size is unrepresentable or the allocation fails, like a real VM whose instantiation fails. This slots into existing machinery:
growMemoryalready applies a size limit, and the fuzz-exec harness already catchesHostLimitExceptionduring instance creation and excludes such runs from comparison, precisely because optimizations can change whether a host limit is reached.With this, the reproducer prints
[host limit memory too large]both before and after--memory-packing, so there is no differential. Added as a lit test undertest/lit/exec/.This is the same overflow pattern in the third of the three sites @MaxGraey pointed at on #8882 (the pass itself,
module-splitting.cpp, andshell-interface.h); module-splitting is now the only one left, and I can pick that up along with the sharedMemoryhelper discussed there if you would like.