Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 42 additions & 10 deletions meld-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,16 +452,35 @@ impl Fuser {
}
return result;
}
// #326: explicit `--memory shared --address-rebase` is an opt-in to an
// unsound transform — address rebasing does not rebase computed memory
// addresses, so it silently corrupts real components. `auto` no longer
// selects this path; when a caller requests it explicitly, warn loudly
// rather than corrupt silently (LS-D-1). The build still proceeds.
if self.config.memory_strategy == MemoryStrategy::SharedMemory
&& self.config.address_rebasing
{
log::warn!(
"memory strategy: explicit shared-memory + address rebasing is UNSOUND \
for components that access memory via computed pointers (#326) — the \
dynamic address operand of ordinary loads/stores is not rebased, so \
per-component memory can silently collide. Prefer `--memory multi` \
unless every input addresses memory only via static offsets."
);
}
self.fuse_with_stats_resolved()
}

/// Resolve `MemoryStrategy::Auto` against the added components.
///
/// Shared memory + address rebasing is selected only when it is
/// statically sound AND buys anything: no input module can grow its
/// memory (`memory_probe`, merger Bug #7) and the inputs carry at
/// least two memories (with fewer, multi-memory output is already
/// single-memory). Any user-supplied `address_rebasing` value is
/// Auto always selects **multi-memory** — it is the sound strategy. Shared
/// memory + address rebasing was previously auto-selected for grow-free,
/// multi-memory inputs, but that path is **unsound** (#326): rebasing does
/// not relocate the dynamic address operand of ordinary loads/stores, so
/// components addressing memory via computed pointers silently collide.
/// Until correct dynamic rebasing lands, Auto never picks shared+rebase;
/// it remains reachable only via explicit `--memory shared --address-rebase`
/// (which warns loudly). Any user-supplied `address_rebasing` value is
/// overridden — Auto owns both knobs.
fn resolve_auto_memory_strategy(&mut self) {
let mut memory_count = 0usize;
Expand Down Expand Up @@ -495,12 +514,25 @@ impl Fuser {
self.config.memory_strategy = MemoryStrategy::MultiMemory;
self.config.address_rebasing = false;
} else {
log::info!(
"memory strategy auto: {memory_count} memories, no memory.grow; \
selecting shared memory with address rebasing"
// #326: address rebasing does NOT rebase the dynamic address
// operand of ordinary loads/stores (only the static memarg offset
// and bulk-memory ops) — so shared-memory fusion silently corrupts
// any component that addresses memory via a computed pointer (heap,
// shadow stack — i.e. all real components). Until correct dynamic
// rebasing lands, `auto` must NOT silently pick shared+rebase.
// Fall back to multi-memory, which is sound (LS-D-1: emit correct
// output, never a plausible-but-wrong one). Explicit
// `--memory shared --address-rebase` remains available as an
// opt-in, and warns loudly (see `warn_if_unsound_rebasing`).
log::warn!(
"memory strategy auto: {memory_count} memories, no memory.grow — \
shared-memory fusion would apply here, but address rebasing is \
unsound for computed memory addresses (#326); selecting \
multi-memory instead. Use `--memory shared --address-rebase` to \
override explicitly (unsound; corrupts computed-pointer access)."
);
self.config.memory_strategy = MemoryStrategy::SharedMemory;
self.config.address_rebasing = true;
self.config.memory_strategy = MemoryStrategy::MultiMemory;
self.config.address_rebasing = false;
}
}

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

const WASM_PAGE_SIZE: usize = 65_536;

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

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

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

assert_eq!(stats.memory_strategy, "shared");
assert_eq!(
stats.memory_strategy, "multi",
"auto must gate the unsound shared+rebase path (#326) and select multi"
);
assert_eq!(
output_memory_count(&fused),
1,
"auto-resolved shared fusion must produce a single memory"
2,
"multi-memory keeps each component's memory separate — no silent \
cross-component collision (the correctness #326 protects)"
);
assert!(
validates_without_multimemory(&fused),
"fused output must validate without --enable-multimemory (#172)"
);

// Behaviour: the single memory holds A's fill at 0 and B's data one
// page in (B's rebased base).
let engine = Engine::default();
let module = RuntimeModule::new(&engine, &fused).unwrap();
let mut store = Store::new(&engine, ());
let instance = Instance::new(&mut store, &module, &[]).unwrap();

instance
.get_typed_func::<(), ()>(&mut store, "a_fill")
.unwrap()
.call(&mut store, ())
.unwrap();
instance
.get_typed_func::<(), ()>(&mut store, "b_init")
.unwrap()
.call(&mut store, ())
.unwrap();

let memory = instance.get_memory(&mut store, "memory").unwrap();
let data = memory.data(&store);
assert_eq!(&data[0..4], &[0x11, 0x11, 0x11, 0x11], "A's region");
assert_eq!(
&data[WASM_PAGE_SIZE..WASM_PAGE_SIZE + 4],
&[1, 2, 3, 4],
"B's region must sit at its rebased base, not clobber A"
!validates_without_multimemory(&fused),
"sanity: the multi-memory form is the one a no-flags validator rejects"
);
}

Expand Down Expand Up @@ -265,11 +245,12 @@ fn auto_keeps_multi_for_single_memory_input() {
}

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

let (_, stats) = fuser.fuse_with_stats().unwrap();
assert_eq!(stats.memory_strategy, "shared", "grow-free pair → shared");
let (fused1, stats) = fuser.fuse_with_stats().unwrap();
assert_eq!(
stats.memory_strategy, "multi",
"grow-free pair → multi (#326 gates the unsound shared+rebase path)"
);
assert_eq!(
output_memory_count(&fused1),
2,
"grow-free pair fuses to 2 memories"
);

fuser
.add_component_named(&component_grow, Some("grower"))
.unwrap();
let (fused, stats) = fuser
.fuse_with_stats()
.expect("second fuse must re-resolve, not reuse the stale shared plan");
.expect("second fuse must re-resolve against the current component set");
assert_eq!(stats.memory_strategy, "multi");
assert_eq!(
stats.memory_strategy, "multi",
"a growing component added after a previous fuse must flip the \
resolution back to multi"
output_memory_count(&fused),
3,
"the added component must be re-probed: 3 memories, not a stale 2"
);
assert_eq!(output_memory_count(&fused), 3);
}

/// Explicit strategies are untouched by Auto: `MultiMemory` still produces
Expand Down
8 changes: 8 additions & 0 deletions meld-core/tests/cross_component_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,18 @@ fn test_304_identity_direct_adapter_is_inlined() {
return;
};

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