Skip to content

Commit

Permalink
Fix combining cache_call_indirects and memory64 (bytecodealliance#8544
Browse files Browse the repository at this point in the history
)

This commit fixes a fuzz bug that popped up where the
`cache_call_indirects` feature wasn't reading memory64-based offsets
correctly. This is due to (not great) API design in `wasmparser`
(introduced by me) where wasmparser by default won't read 64-bit offsets
unless explicitly allowed to. This is to handle how spec tests assert
that overlong 32-bit encodings are invalid as the memory64 proposal
isn't merged into the spec yet.

The fix here is to call `allow_memarg64` with whether memory64 is
enabled or not and then that'll enable reading these overlong and/or
larger offsets correctly.
  • Loading branch information
alexcrichton authored May 3, 2024
1 parent 7e14537 commit c607704
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
8 changes: 4 additions & 4 deletions crates/environ/src/compile/module_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use std::collections::HashMap;
use std::mem;
use std::path::PathBuf;
use std::sync::Arc;
use wasmparser::OperatorsReader;
use wasmparser::{
types::Types, CustomSectionReader, DataKind, ElementItems, ElementKind, Encoding, ExternalKind,
FuncToValidate, FunctionBody, NameSectionReader, Naming, Operator, Parser, Payload, TypeRef,
Expand Down Expand Up @@ -543,6 +542,7 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {

Payload::CodeSectionEntry(mut body) => {
let validator = self.validator.code_section_entry(&body)?;
body.allow_memarg64(self.validator.features().contains(WasmFeatures::MEMORY64));
let func_index =
self.result.code_index + self.result.module.num_imported_funcs as u32;
let func_index = FuncIndex::from_u32(func_index);
Expand All @@ -567,7 +567,7 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> {
params: sig.params().into(),
});
}
self.prescan_code_section(body.get_operators_reader()?)?;
self.prescan_code_section(&body)?;
body.allow_memarg64(self.validator.features().contains(WasmFeatures::MEMORY64));
self.result.function_body_inputs.push(FunctionBodyData {
validator,
Expand Down Expand Up @@ -721,9 +721,9 @@ and for re-adding support for interface types you can see this issue:
/// index in this count for each function, so we can generate
/// its code (with accesses to its own `call_indirect` callsite
/// caches) in parallel.
fn prescan_code_section(&mut self, reader: OperatorsReader<'data>) -> Result<()> {
fn prescan_code_section(&mut self, body: &FunctionBody<'_>) -> Result<()> {
if self.tunables.cache_call_indirects {
for op in reader {
for op in body.get_operators_reader()? {
let op = op?;
match op {
// Check whether a table may be mutated by any
Expand Down
19 changes: 19 additions & 0 deletions tests/all/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,3 +250,22 @@ fn compile_a_component() -> Result<()> {
);
Ok(())
}

#[test]
fn call_indirect_caching_and_memory64() -> Result<()> {
let mut config = Config::new();
config.wasm_memory64(true);
config.cache_call_indirects(true);
let engine = Engine::new(&config)?;
Module::new(
&engine,
"(module
(memory i64 1)
(func (param i64) (result i32)
local.get 0
i32.load offset=0x100000000
)
)",
)?;
Ok(())
}

0 comments on commit c607704

Please sign in to comment.