From 66ddde16e721a641df042a3ff1973fc3f31007fa Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 27 Apr 2026 09:44:20 -0700 Subject: [PATCH] Require `memory` option if `realloc` is specified This commit closes a validation gap between `wasm-tools` and the upstream specification by ensuring that if the `realloc` option is specified at all that the `memory` option is additionally specified. This was discovered recently as a discrepancy with the specification itself, and I split this out to be able to sit down and think a bit more about this change. Specifically I've validated that `wit-component` cannot produce a component that uses `realloc` but does not use `memory` by accident. This means that in practice I'm expecting that this won't cause any accidentally-valid components in use today to stop running. Instead there are unlikely to be any components in the wild that this affects. Closes #2503 --- crates/wasmparser/src/validator/component.rs | 22 +++--- .../component-model/async/task-builtins.wast | 3 +- tests/cli/component-model/options.wast | 73 +++++++++++++++++++ .../async/task-builtins.wast.json | 72 +++++++++--------- .../cli/component-model/options.wast.json | 46 ++++++++++++ .../cli/component-model/options.wast/0.print | 32 ++++++++ 6 files changed, 201 insertions(+), 47 deletions(-) create mode 100644 tests/cli/component-model/options.wast create mode 100644 tests/snapshots/cli/component-model/options.wast.json create mode 100644 tests/snapshots/cli/component-model/options.wast/0.print diff --git a/crates/wasmparser/src/validator/component.rs b/crates/wasmparser/src/validator/component.rs index 1b4bd709d9..14ea06f83c 100644 --- a/crates/wasmparser/src/validator/component.rs +++ b/crates/wasmparser/src/validator/component.rs @@ -2793,16 +2793,18 @@ impl ComponentState { // Validate `realloc` if let Some(realloc_idx) = realloc { - let addr_type = match memory { - // If a memory was specified, `realloc` must match its address type. - Some(memory_idx) => match self.memory_at(memory_idx, offset)?.memory64 { - true => ValType::I64, - false => ValType::I32, - }, - // Backwards compatibility: Assume `i32` memory if none was specified. - // FIXME(#2503): The spec requires `memory` if `realloc` is specified, but this - // may break existing code. - None => ValType::I32, + let mty = match memory { + Some(i) => self.memory_at(i, offset)?, + None => { + return Err(BinaryReaderError::new( + "canonical option `realloc` requires `memory` to also be specified", + offset, + )); + } + }; + let addr_type = match mty.memory64 { + true => ValType::I64, + false => ValType::I32, }; let ty_id = self.core_function_at(realloc_idx, offset)?; let func_ty = types[ty_id].unwrap_func(); diff --git a/tests/cli/component-model/async/task-builtins.wast b/tests/cli/component-model/async/task-builtins.wast index ae8cca69f3..87a5694ac4 100644 --- a/tests/cli/component-model/async/task-builtins.wast +++ b/tests/cli/component-model/async/task-builtins.wast @@ -80,9 +80,10 @@ (component (core module $m (func (export "r") (param i32 i32 i32 i32) (result i32) unreachable) + (memory (export "m") 0) ) (core instance $m (instantiate $m)) - (core func $task-return (canon task.return (result u32) (realloc (func $m "r")))) + (core func $task-return (canon task.return (result u32) (realloc (func $m "r")) (memory $m "m"))) ) "cannot specify `realloc` option on `task.return`") diff --git a/tests/cli/component-model/options.wast b/tests/cli/component-model/options.wast new file mode 100644 index 0000000000..b2b60a91af --- /dev/null +++ b/tests/cli/component-model/options.wast @@ -0,0 +1,73 @@ +;; RUN: wast --assert default --snapshot tests/snapshots % -f custom-page-sizes + +(component + (core module $m + (func (export "r") (param i32 i32 i32 i32) (result i32) unreachable) + (memory (export "m") 0) + + (func (export "f")) + ) + (core instance $i (instantiate $m)) + + (func (canon lift (core func $i "f"))) + (func (canon lift (core func $i "f") (memory $i "m"))) + (func (canon lift (core func $i "f") (memory $i "m") (realloc (func $i "r")))) + (func (canon lift (core func $i "f") (realloc (func $i "r")) (memory $i "m"))) +) + +;; `realloc` requires `memory` +(assert_invalid + (component + (core module $m + (func (export "r") (param i32 i32 i32 i32) (result i32) unreachable) + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func (canon lift (core func $i "f") (realloc (func $i "r")))) + ) + "canonical option `realloc` requires `memory` to also be specified") + +;; even if `realloc` isn't needed, it's still validated +(assert_invalid + (component + (core module $m + (func (export "r")) + (memory (export "m") 0) + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func (canon lift (core func $i "f") (realloc (func $i "r")) (memory $i "m"))) + ) + "canonical option `realloc` uses a core function with an incorrect signature") + +;; even if `memory` isn't needed, it's still validated +(assert_invalid + (component + (core module $m + (memory (export "m") 0 (pagesize 1)) + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func (canon lift (core func $i "f") (memory $i "m"))) + ) + "mismatch in page size for memories") +(assert_invalid + (component + (core module $m + (memory (export "m") 0 1 shared) + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func (canon lift (core func $i "f") (memory $i "m"))) + ) + "mismatch in the shared flag for memories") +(assert_invalid + (component + (core module $m + (memory (export "m") i64 0) + (func (export "f")) + ) + (core instance $i (instantiate $m)) + (func (canon lift (core func $i "f") (memory $i "m"))) + ) + "64-bit memories require the `cm64` feature to be enabled") diff --git a/tests/snapshots/cli/component-model/async/task-builtins.wast.json b/tests/snapshots/cli/component-model/async/task-builtins.wast.json index 13cfdbf05c..2dddfa2e8d 100644 --- a/tests/snapshots/cli/component-model/async/task-builtins.wast.json +++ b/tests/snapshots/cli/component-model/async/task-builtins.wast.json @@ -70,237 +70,237 @@ }, { "type": "module", - "line": 89, + "line": 90, "filename": "task-builtins.10.wasm", "module_type": "binary" }, { "type": "module", - "line": 101, + "line": 102, "filename": "task-builtins.11.wasm", "module_type": "binary" }, { "type": "module", - "line": 110, + "line": 111, "filename": "task-builtins.12.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 118, + "line": 119, "filename": "task-builtins.13.wasm", "module_type": "binary", "text": "type mismatch for export `waitable-set.new` of module instantiation argument ``" }, { "type": "module", - "line": 127, + "line": 128, "filename": "task-builtins.14.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 139, + "line": 140, "filename": "task-builtins.15.wasm", "module_type": "binary", "text": "type mismatch for export `waitable-set.wait` of module instantiation argument ``" }, { "type": "module", - "line": 151, + "line": 152, "filename": "task-builtins.16.wasm", "module_type": "binary" }, { "type": "module", - "line": 162, + "line": 163, "filename": "task-builtins.17.wasm", "module_type": "binary" }, { "type": "module", - "line": 171, + "line": 172, "filename": "task-builtins.18.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 183, + "line": 184, "filename": "task-builtins.19.wasm", "module_type": "binary", "text": "type mismatch for export `waitable-set.poll` of module instantiation argument ``" }, { "type": "module", - "line": 196, + "line": 197, "filename": "task-builtins.20.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 204, + "line": 205, "filename": "task-builtins.21.wasm", "module_type": "binary", "text": "type mismatch for export `waitable-set.drop` of module instantiation argument ``" }, { "type": "module", - "line": 213, + "line": 214, "filename": "task-builtins.22.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 221, + "line": 222, "filename": "task-builtins.23.wasm", "module_type": "binary", "text": "type mismatch for export `waitable.join` of module instantiation argument ``" }, { "type": "module", - "line": 230, + "line": 231, "filename": "task-builtins.24.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 240, + "line": 241, "filename": "task-builtins.25.wasm", "module_type": "binary", "text": "type mismatch for export `subtask.drop` of module instantiation argument ``" }, { "type": "module", - "line": 251, + "line": 252, "filename": "task-builtins.26.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 261, + "line": 262, "filename": "task-builtins.27.wasm", "module_type": "binary", "text": "type mismatch for export `subtask.cancel` of module instantiation argument ``" }, { "type": "module", - "line": 272, + "line": 273, "filename": "task-builtins.28.wasm", "module_type": "binary" }, { "type": "module", - "line": 289, + "line": 290, "filename": "task-builtins.29.wasm", "module_type": "binary" }, { "type": "module", - "line": 297, + "line": 298, "filename": "task-builtins.30.wasm", "module_type": "binary" }, { "type": "assert_invalid", - "line": 307, + "line": 308, "filename": "task-builtins.31.wasm", "module_type": "binary", "text": "type mismatch for export `thread.yield` of module instantiation argument ``" }, { "type": "assert_invalid", - "line": 318, + "line": 319, "filename": "task-builtins.32.wasm", "module_type": "binary", "text": "found: (func (result i32))" }, { "type": "assert_invalid", - "line": 325, + "line": 326, "filename": "task-builtins.33.wasm", "module_type": "binary", "text": "found: (func (param i32))" }, { "type": "assert_invalid", - "line": 332, + "line": 333, "filename": "task-builtins.34.wasm", "module_type": "binary", "text": "immediate must be zero: 1" }, { "type": "assert_invalid", - "line": 336, + "line": 337, "filename": "task-builtins.35.wasm", "module_type": "binary", "text": "immediate must be zero: 1" }, { "type": "assert_invalid", - "line": 340, + "line": 341, "filename": "task-builtins.36.wasm", "module_type": "binary", "text": "immediate must be zero: 100" }, { "type": "assert_invalid", - "line": 344, + "line": 345, "filename": "task-builtins.37.wasm", "module_type": "binary", "text": "immediate must be zero: 100" }, { "type": "assert_invalid", - "line": 350, + "line": 351, "filename": "task-builtins.38.wasm", "module_type": "binary", "text": "64-bit `context.get` requires the component model 64-bit feature" }, { "type": "assert_invalid", - "line": 354, + "line": 355, "filename": "task-builtins.39.wasm", "module_type": "binary", "text": "64-bit `context.set` requires the component model 64-bit feature" }, { "type": "assert_invalid", - "line": 359, + "line": 360, "filename": "task-builtins.40.wasm", "module_type": "binary", "text": "64-bit `context.get` requires the component model 64-bit feature" }, { "type": "assert_invalid", - "line": 366, + "line": 367, "filename": "task-builtins.41.wasm", "module_type": "binary", "text": "64-bit `context.set` requires the component model 64-bit feature" }, { "type": "assert_invalid", - "line": 375, + "line": 376, "filename": "task-builtins.42.wasm", "module_type": "binary", "text": "`context.get` only supports `i32` or `i64`" }, { "type": "assert_invalid", - "line": 379, + "line": 380, "filename": "task-builtins.43.wasm", "module_type": "binary", "text": "`context.set` only supports `i32` or `i64`" }, { "type": "module", - "line": 385, + "line": 386, "filename": "task-builtins.44.wasm", "module_type": "binary" }, { "type": "module", - "line": 445, + "line": 446, "filename": "task-builtins.45.wasm", "module_type": "binary" } diff --git a/tests/snapshots/cli/component-model/options.wast.json b/tests/snapshots/cli/component-model/options.wast.json new file mode 100644 index 0000000000..d2e37ce680 --- /dev/null +++ b/tests/snapshots/cli/component-model/options.wast.json @@ -0,0 +1,46 @@ +{ + "source_filename": "tests/cli/component-model/options.wast", + "commands": [ + { + "type": "module", + "line": 3, + "filename": "options.0.wasm", + "module_type": "binary" + }, + { + "type": "assert_invalid", + "line": 20, + "filename": "options.1.wasm", + "module_type": "binary", + "text": "canonical option `realloc` requires `memory` to also be specified" + }, + { + "type": "assert_invalid", + "line": 32, + "filename": "options.2.wasm", + "module_type": "binary", + "text": "canonical option `realloc` uses a core function with an incorrect signature" + }, + { + "type": "assert_invalid", + "line": 45, + "filename": "options.3.wasm", + "module_type": "binary", + "text": "mismatch in page size for memories" + }, + { + "type": "assert_invalid", + "line": 55, + "filename": "options.4.wasm", + "module_type": "binary", + "text": "mismatch in the shared flag for memories" + }, + { + "type": "assert_invalid", + "line": 65, + "filename": "options.5.wasm", + "module_type": "binary", + "text": "64-bit memories require the `cm64` feature to be enabled" + } + ] +} \ No newline at end of file diff --git a/tests/snapshots/cli/component-model/options.wast/0.print b/tests/snapshots/cli/component-model/options.wast/0.print new file mode 100644 index 0000000000..40ce7533a1 --- /dev/null +++ b/tests/snapshots/cli/component-model/options.wast/0.print @@ -0,0 +1,32 @@ +(component + (core module $m (;0;) + (type (;0;) (func (param i32 i32 i32 i32) (result i32))) + (type (;1;) (func)) + (memory (;0;) 0) + (export "r" (func 0)) + (export "m" (memory 0)) + (export "f" (func 1)) + (func (;0;) (type 0) (param i32 i32 i32 i32) (result i32) + unreachable + ) + (func (;1;) (type 1)) + ) + (core instance $i (;0;) (instantiate $m)) + (type (;0;) (func)) + (alias core export $i "f" (core func (;0;))) + (func (;0;) (type 0) (canon lift (core func 0))) + (type (;1;) (func)) + (alias core export $i "f" (core func (;1;))) + (alias core export $i "m" (core memory (;0;))) + (func (;1;) (type 1) (canon lift (core func 1) (memory 0))) + (type (;2;) (func)) + (alias core export $i "f" (core func (;2;))) + (alias core export $i "m" (core memory (;1;))) + (alias core export $i "r" (core func (;3;))) + (func (;2;) (type 2) (canon lift (core func 2) (memory 1) (realloc 3))) + (type (;3;) (func)) + (alias core export $i "f" (core func (;4;))) + (alias core export $i "r" (core func (;5;))) + (alias core export $i "m" (core memory (;2;))) + (func (;3;) (type 3) (canon lift (core func 4) (realloc 5) (memory 2))) +)