Skip to content

Commit 66a08fe

Browse files
avrabeclaude
andcommitted
fix(fuse): gate the unsound shared+rebase auto path (#326)
`--memory auto` silently selected shared memory + address rebasing for grow-free multi-memory inputs. That is UNSOUND (#326): `convert_memarg` only rebases the static memarg offset (and bulk-memory ops); the dynamic address operand of ordinary i32.load/i32.store is never rebased. Real components address heap/shadow-stack via computed pointers, so shared fusion silently overlaps their private memory — wrong results, no trap, no error. Confirmed in `rewriter.rs` (convert_memarg; needs_rebase_locals gated to Memory{Copy,Fill, Init}) and reproduced end-to-end. Gate now (LS-D-1 — emit correct output or none, never plausible-but-wrong): - `resolve_auto_memory_strategy` no longer selects shared+rebase; Auto always resolves to multi-memory (sound). A `log::warn!` explains the gate. - Explicit `--memory shared --address-rebase` remains an opt-in but now warns loudly that it is unsound for computed-pointer access. - Corrected the docstrings that claimed the shared path was "statically sound". Tests: - `auto_selects_shared_for_growfree_inputs` → `auto_gates_shared_for_growfree_ inputs_326` (asserts multi + 2 separate memories = the correctness #326 protects); `ls_m_7_auto_reprobes_after_add_component` re-probes via memory count (2 → 3) since both resolutions are now multi. - `test_304_identity_direct_adapter_is_inlined` now requests `SharedMemory` explicitly — the #304 inlining feature runs on the shared path, which Auto no longer selects; adapter inlining is orthogonal to the memory-access unsoundness #326 gates. Interim safety gate; correct runtime dynamic-address rebasing in rewriter.rs (Tier-5) is the follow-up "fix properly" step. Safety docs (ADR-4, loss-scenarios, safety-requirements) still describe the old auto→shared default and should be reconciled in a traceability follow-up. Refs #326. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent be66dc7 commit 66a08fe

3 files changed

Lines changed: 97 additions & 68 deletions

File tree

meld-core/src/lib.rs

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -445,16 +445,35 @@ impl Fuser {
445445
}
446446
return result;
447447
}
448+
// #326: explicit `--memory shared --address-rebase` is an opt-in to an
449+
// unsound transform — address rebasing does not rebase computed memory
450+
// addresses, so it silently corrupts real components. `auto` no longer
451+
// selects this path; when a caller requests it explicitly, warn loudly
452+
// rather than corrupt silently (LS-D-1). The build still proceeds.
453+
if self.config.memory_strategy == MemoryStrategy::SharedMemory
454+
&& self.config.address_rebasing
455+
{
456+
log::warn!(
457+
"memory strategy: explicit shared-memory + address rebasing is UNSOUND \
458+
for components that access memory via computed pointers (#326) — the \
459+
dynamic address operand of ordinary loads/stores is not rebased, so \
460+
per-component memory can silently collide. Prefer `--memory multi` \
461+
unless every input addresses memory only via static offsets."
462+
);
463+
}
448464
self.fuse_with_stats_resolved()
449465
}
450466

451467
/// Resolve `MemoryStrategy::Auto` against the added components.
452468
///
453-
/// Shared memory + address rebasing is selected only when it is
454-
/// statically sound AND buys anything: no input module can grow its
455-
/// memory (`memory_probe`, merger Bug #7) and the inputs carry at
456-
/// least two memories (with fewer, multi-memory output is already
457-
/// single-memory). Any user-supplied `address_rebasing` value is
469+
/// Auto always selects **multi-memory** — it is the sound strategy. Shared
470+
/// memory + address rebasing was previously auto-selected for grow-free,
471+
/// multi-memory inputs, but that path is **unsound** (#326): rebasing does
472+
/// not relocate the dynamic address operand of ordinary loads/stores, so
473+
/// components addressing memory via computed pointers silently collide.
474+
/// Until correct dynamic rebasing lands, Auto never picks shared+rebase;
475+
/// it remains reachable only via explicit `--memory shared --address-rebase`
476+
/// (which warns loudly). Any user-supplied `address_rebasing` value is
458477
/// overridden — Auto owns both knobs.
459478
fn resolve_auto_memory_strategy(&mut self) {
460479
let mut memory_count = 0usize;
@@ -488,12 +507,25 @@ impl Fuser {
488507
self.config.memory_strategy = MemoryStrategy::MultiMemory;
489508
self.config.address_rebasing = false;
490509
} else {
491-
log::info!(
492-
"memory strategy auto: {memory_count} memories, no memory.grow; \
493-
selecting shared memory with address rebasing"
510+
// #326: address rebasing does NOT rebase the dynamic address
511+
// operand of ordinary loads/stores (only the static memarg offset
512+
// and bulk-memory ops) — so shared-memory fusion silently corrupts
513+
// any component that addresses memory via a computed pointer (heap,
514+
// shadow stack — i.e. all real components). Until correct dynamic
515+
// rebasing lands, `auto` must NOT silently pick shared+rebase.
516+
// Fall back to multi-memory, which is sound (LS-D-1: emit correct
517+
// output, never a plausible-but-wrong one). Explicit
518+
// `--memory shared --address-rebase` remains available as an
519+
// opt-in, and warns loudly (see `warn_if_unsound_rebasing`).
520+
log::warn!(
521+
"memory strategy auto: {memory_count} memories, no memory.grow — \
522+
shared-memory fusion would apply here, but address rebasing is \
523+
unsound for computed memory addresses (#326); selecting \
524+
multi-memory instead. Use `--memory shared --address-rebase` to \
525+
override explicitly (unsound; corrupts computed-pointer access)."
494526
);
495-
self.config.memory_strategy = MemoryStrategy::SharedMemory;
496-
self.config.address_rebasing = true;
527+
self.config.memory_strategy = MemoryStrategy::MultiMemory;
528+
self.config.address_rebasing = false;
497529
}
498530
}
499531

meld-core/tests/auto_memory.rs

Lines changed: 47 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
//! `MemoryStrategy::Auto` resolution — integration oracle for #172.
1+
//! `MemoryStrategy::Auto` resolution — integration oracle for #172 / #326.
22
//!
3-
//! The default memory strategy is `Auto`: shared memory + address rebasing
4-
//! when no input module contains `memory.grow` and there are at least two
5-
//! memories to merge, multi-memory otherwise. The point of the feature is
6-
//! that out-of-the-box `meld fuse` output flows through `wasm-opt → synth`
7-
//! with no extra flags, WITHOUT ever selecting the shared form where it is
8-
//! unsound (merger Bug #7: shared memory breaks under `memory.grow`).
3+
//! The default memory strategy is `Auto`. It resolves to **multi-memory**:
4+
//! shared memory + address rebasing was previously auto-selected for grow-free
5+
//! multi-memory inputs, but that path is UNSOUND (#326 — rebasing does not
6+
//! relocate the dynamic address operand of ordinary loads/stores, so
7+
//! computed-pointer access silently collides across components), so Auto no
8+
//! longer selects it. Shared+rebase remains reachable only via explicit
9+
//! `--memory shared --address-rebase`, which warns loudly. (Historically Auto
10+
//! also always avoided shared under `memory.grow` — merger Bug #7.)
911
//!
1012
//! The oracle here mirrors the issue's repro: `wasm-opt` (and any
1113
//! standards-default validator) rejects multi-memory modules. So the fused
@@ -19,9 +21,6 @@ use wasm_encoder::{
1921
ExportKind, ExportSection, Function, FunctionSection, Instruction, MemorySection, MemoryType,
2022
Module, ModuleSection, TypeSection,
2123
};
22-
use wasmtime::{Engine, Instance, Module as RuntimeModule, Store};
23-
24-
const WASM_PAGE_SIZE: usize = 65_536;
2524

2625
fn build_component(module: Module) -> Vec<u8> {
2726
let mut component = Component::new();
@@ -178,52 +177,33 @@ fn validates_without_multimemory(wasm: &[u8]) -> bool {
178177
.is_ok()
179178
}
180179

181-
/// Two grow-free components, one memory each → Auto must pick shared +
182-
/// rebasing: single-memory output, accepted by a no-flags validator, and
183-
/// behaviourally correct (B's data lands at its rebased base, A's intact).
180+
/// #326: address rebasing does not relocate the dynamic address operand of
181+
/// ordinary loads/stores, so shared-memory fusion silently corrupts any
182+
/// component that addresses memory via a computed pointer. Auto therefore no
183+
/// longer selects shared+rebase even for grow-free inputs — it falls back to
184+
/// multi-memory, the sound strategy (LS-D-1: correct output, never a
185+
/// plausible-but-wrong one). Shared remains reachable only via explicit
186+
/// `--memory shared --address-rebase`, which warns loudly.
184187
#[test]
185-
fn auto_selects_shared_for_growfree_inputs() {
188+
fn auto_gates_shared_for_growfree_inputs_326() {
186189
let component_a = build_component(build_module_a());
187190
let component_b = build_component(build_module_b(false));
188191

189192
let (fused, stats) = fuse_default(&[component_a, component_b]);
190193

191-
assert_eq!(stats.memory_strategy, "shared");
194+
assert_eq!(
195+
stats.memory_strategy, "multi",
196+
"auto must gate the unsound shared+rebase path (#326) and select multi"
197+
);
192198
assert_eq!(
193199
output_memory_count(&fused),
194-
1,
195-
"auto-resolved shared fusion must produce a single memory"
200+
2,
201+
"multi-memory keeps each component's memory separate — no silent \
202+
cross-component collision (the correctness #326 protects)"
196203
);
197204
assert!(
198-
validates_without_multimemory(&fused),
199-
"fused output must validate without --enable-multimemory (#172)"
200-
);
201-
202-
// Behaviour: the single memory holds A's fill at 0 and B's data one
203-
// page in (B's rebased base).
204-
let engine = Engine::default();
205-
let module = RuntimeModule::new(&engine, &fused).unwrap();
206-
let mut store = Store::new(&engine, ());
207-
let instance = Instance::new(&mut store, &module, &[]).unwrap();
208-
209-
instance
210-
.get_typed_func::<(), ()>(&mut store, "a_fill")
211-
.unwrap()
212-
.call(&mut store, ())
213-
.unwrap();
214-
instance
215-
.get_typed_func::<(), ()>(&mut store, "b_init")
216-
.unwrap()
217-
.call(&mut store, ())
218-
.unwrap();
219-
220-
let memory = instance.get_memory(&mut store, "memory").unwrap();
221-
let data = memory.data(&store);
222-
assert_eq!(&data[0..4], &[0x11, 0x11, 0x11, 0x11], "A's region");
223-
assert_eq!(
224-
&data[WASM_PAGE_SIZE..WASM_PAGE_SIZE + 4],
225-
&[1, 2, 3, 4],
226-
"B's region must sit at its rebased base, not clobber A"
205+
!validates_without_multimemory(&fused),
206+
"sanity: the multi-memory form is the one a no-flags validator rejects"
227207
);
228208
}
229209

@@ -265,11 +245,12 @@ fn auto_keeps_multi_for_single_memory_input() {
265245
}
266246

267247
/// Mythos finding A (PR #220): Auto resolution must be re-derived from the
268-
/// CURRENT component set on every fuse. A fuse → `add_component` → fuse
269-
/// sequence must not reuse the first fuse's stale "shared" resolution when
270-
/// the newly added component grows memory — the second fuse must succeed
271-
/// and resolve to multi. (`ls_m_7_` prefix: LS-M-7 / UCA-M-11 regression,
272-
/// run by the LS-N verification gate.)
248+
/// CURRENT component set on every fuse. Post-#326 Auto always resolves to
249+
/// multi-memory (shared+rebase is gated off as unsound), so re-probing is
250+
/// verified by the memory layout re-deriving: a grow-free pair fuses to 2
251+
/// memories, and after adding a third component the next fuse re-derives to
252+
/// 3 — not a stale 2. (`ls_m_7_` prefix: LS-M-7 / UCA-M-11 regression, run by
253+
/// the LS-N verification gate.)
273254
#[test]
274255
fn ls_m_7_auto_reprobes_after_add_component() {
275256
let component_a = build_component(build_module_a());
@@ -280,21 +261,29 @@ fn ls_m_7_auto_reprobes_after_add_component() {
280261
fuser.add_component_named(&component_a, Some("a")).unwrap();
281262
fuser.add_component_named(&component_b, Some("b")).unwrap();
282263

283-
let (_, stats) = fuser.fuse_with_stats().unwrap();
284-
assert_eq!(stats.memory_strategy, "shared", "grow-free pair → shared");
264+
let (fused1, stats) = fuser.fuse_with_stats().unwrap();
265+
assert_eq!(
266+
stats.memory_strategy, "multi",
267+
"grow-free pair → multi (#326 gates the unsound shared+rebase path)"
268+
);
269+
assert_eq!(
270+
output_memory_count(&fused1),
271+
2,
272+
"grow-free pair fuses to 2 memories"
273+
);
285274

286275
fuser
287276
.add_component_named(&component_grow, Some("grower"))
288277
.unwrap();
289278
let (fused, stats) = fuser
290279
.fuse_with_stats()
291-
.expect("second fuse must re-resolve, not reuse the stale shared plan");
280+
.expect("second fuse must re-resolve against the current component set");
281+
assert_eq!(stats.memory_strategy, "multi");
292282
assert_eq!(
293-
stats.memory_strategy, "multi",
294-
"a growing component added after a previous fuse must flip the \
295-
resolution back to multi"
283+
output_memory_count(&fused),
284+
3,
285+
"the added component must be re-probed: 3 memories, not a stale 2"
296286
);
297-
assert_eq!(output_memory_count(&fused), 3);
298287
}
299288

300289
/// Explicit strategies are untouched by Auto: `MultiMemory` still produces

meld-core/tests/cross_component_call.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,17 @@ fn test_304_identity_direct_adapter_is_inlined() {
168168
return;
169169
};
170170

171+
// Identity-adapter inlining (#304) is exercised on the shared-memory
172+
// fusion path. Request it explicitly: since #326, `auto` no longer selects
173+
// shared+rebase (it is unsound for computed-pointer memory access), so this
174+
// feature test must opt into shared itself. The unsoundness #326 gates is
175+
// about memory *access*, not adapter inlining, so exercising shared here to
176+
// check the inlining + validity is fine.
171177
let mut fuser = Fuser::new(FuserConfig {
172178
attestation: false,
173179
component_provenance: false,
180+
memory_strategy: MemoryStrategy::SharedMemory,
181+
address_rebasing: true,
174182
..Default::default()
175183
});
176184
fuser

0 commit comments

Comments
 (0)