diff --git a/meld-core/src/merger.rs b/meld-core/src/merger.rs index 860afa5..7b169d1 100644 --- a/meld-core/src/merger.rs +++ b/meld-core/src/merger.rs @@ -1582,29 +1582,19 @@ impl Merger { .push(convert_table_type(table, comp_idx, mod_idx, merged)); } - // Merge globals (defined globals only; imported globals handled below) - let global_offset = merged.globals.len() as u32; - for (old_idx, global) in module.globals.iter().enumerate() { - let new_idx = merged.import_counts.global + global_offset + old_idx as u32; - merged.global_index_map.insert( - (comp_idx, mod_idx, import_global_count + old_idx as u32), - new_idx, - ); - let init_expr = convert_init_expr( - &global.init_expr_bytes, - comp_idx, - mod_idx, - merged, - &global.content_type, - ); - let ty = convert_global_type(global, comp_idx, mod_idx, merged); - merged.globals.push(MergedGlobal { ty, init_expr }); - } - // Resolve imported global indices via intra-component module_resolutions. // This mirrors how function imports are resolved below: if module A // imports a global that module B exports, map A's imported global index // to B's defined global's merged index. + // + // This MUST run before converting THIS module's defined-global init + // exprs below: an extended-const initializer may reference an imported + // global (`i32.const N; global.get $__memory_base; i32.add`, #338), and + // `convert_init_expr` remaps that global through `global_index_map`. If + // the imported-global entries were populated only afterwards (the prior + // ordering), the remap silently missed and emitted the un-remapped local + // index — reading the wrong global whenever fusion shifted the import + // off index 0. { let mut import_global_idx = 0u32; for imp in &module.imports { @@ -1658,6 +1648,27 @@ impl Merger { } } + // Merge globals (defined globals only; imported globals handled above). + // Runs AFTER imported-global resolution so init exprs can remap any + // `global.get` of an imported global (#338). + let global_offset = merged.globals.len() as u32; + for (old_idx, global) in module.globals.iter().enumerate() { + let new_idx = merged.import_counts.global + global_offset + old_idx as u32; + merged.global_index_map.insert( + (comp_idx, mod_idx, import_global_count + old_idx as u32), + new_idx, + ); + let init_expr = convert_init_expr( + &global.init_expr_bytes, + comp_idx, + mod_idx, + merged, + &global.content_type, + ); + let ty = convert_global_type(global, comp_idx, mod_idx, merged); + merged.globals.push(MergedGlobal { ty, init_expr }); + } + // Resolve imported table indices via intra-component module_resolutions. // Same pattern as global import resolution above. { @@ -3302,14 +3313,40 @@ fn convert_init_expr( // only the first op and silently dropped the rest, producing a // wrong-valued global (LS-A-11). wasmparser::Operator::I32Const { value } => { + let remap = |idx: u32| { + merged + .global_index_map + .get(&(comp_idx, mod_idx, idx)) + .copied() + .unwrap_or(idx) + }; match crate::segments::fold_extended_const_i32(&mut ops, value) { - Ok(folded) => ConstExpr::i32_const(folded), + Ok(crate::segments::ExtConstFold::Value(folded)) => ConstExpr::i32_const(folded), + // Const-first with an embedded `global.get` (`N + __memory_base`): + // preserve and remap the full sequence instead of falling back to + // un-remapped raw bytes, which would emit the wrong global index in + // genuine multi-module fusion (#338). + Ok(crate::segments::ExtConstFold::Extended(seq)) => { + let remapped: Vec<_> = seq.iter().map(|o| o.remap_global(remap)).collect(); + crate::segments::ext_const_to_expr(&remapped) + } Err(_) => ConstExpr::raw(bytes.iter().copied()), } } wasmparser::Operator::I64Const { value } => { + let remap = |idx: u32| { + merged + .global_index_map + .get(&(comp_idx, mod_idx, idx)) + .copied() + .unwrap_or(idx) + }; match crate::segments::fold_extended_const_i64(&mut ops, value) { - Ok(folded) => ConstExpr::i64_const(folded), + Ok(crate::segments::ExtConstFold::Value(folded)) => ConstExpr::i64_const(folded), + Ok(crate::segments::ExtConstFold::Extended(seq)) => { + let remapped: Vec<_> = seq.iter().map(|o| o.remap_global(remap)).collect(); + crate::segments::ext_const_to_expr(&remapped) + } Err(_) => ConstExpr::raw(bytes.iter().copied()), } } @@ -3323,12 +3360,26 @@ fn convert_init_expr( ConstExpr::v128_const(i128::from_le_bytes(*value.bytes())) } wasmparser::Operator::GlobalGet { global_index } => { - let remapped = merged - .global_index_map - .get(&(comp_idx, mod_idx, global_index)) - .copied() - .unwrap_or(global_index); - ConstExpr::global_get(remapped) + let remap = |idx: u32| { + merged + .global_index_map + .get(&(comp_idx, mod_idx, idx)) + .copied() + .unwrap_or(idx) + }; + // A `global.get`-first initializer may continue with extended-const + // arithmetic (`__memory_base + N`). Its value is runtime-dependent, + // so preserve and re-emit the COMPLETE operator sequence (global + // indices remapped) rather than reading only the leading + // `global.get` and dropping the trailing arithmetic (#338). + match crate::segments::read_extended_const_global_get(&mut ops, global_index) { + Ok(Some(seq)) => { + let remapped: Vec<_> = seq.iter().map(|o| o.remap_global(remap)).collect(); + crate::segments::ext_const_to_expr(&remapped) + } + Ok(None) => ConstExpr::global_get(remap(global_index)), + Err(_) => ConstExpr::raw(bytes.iter().copied()), + } } wasmparser::Operator::RefFunc { function_index } => { let remapped = merged diff --git a/meld-core/src/segments.rs b/meld-core/src/segments.rs index d14471e..5f87ab3 100644 --- a/meld-core/src/segments.rs +++ b/meld-core/src/segments.rs @@ -25,6 +25,131 @@ use wasm_encoder::{ }; use wasmparser::{DataSectionReader, ElementItems, ElementKind, ElementSectionReader, Operator}; +/// A single operator inside a preserved wasm-2.0 extended-const expression +/// whose value is runtime-dependent because it begins with `global.get` +/// (the position-independent `__memory_base + N` / `__table_base + N` shape, +/// #338). Such an expression CANNOT be folded to a constant, so its operators +/// are kept verbatim and re-emitted, with `GlobalGet` indices remapped. +#[derive(Debug, Clone, PartialEq)] +pub enum ExtConstOp { + GlobalGet(u32), + I32Const(i32), + I64Const(i64), + I32Add, + I32Sub, + I32Mul, + I64Add, + I64Sub, + I64Mul, +} + +impl ExtConstOp { + /// Remap any `GlobalGet` index in this operator using `f`. + pub(crate) fn remap_global(&self, f: impl Fn(u32) -> u32) -> ExtConstOp { + match self { + ExtConstOp::GlobalGet(idx) => ExtConstOp::GlobalGet(f(*idx)), + other => other.clone(), + } + } + + /// Append this operator to a `wasm_encoder::ConstExpr` builder. + fn append_to(&self, ce: ConstExpr) -> ConstExpr { + match self { + ExtConstOp::GlobalGet(i) => ce.with_global_get(*i), + ExtConstOp::I32Const(v) => ce.with_i32_const(*v), + ExtConstOp::I64Const(v) => ce.with_i64_const(*v), + ExtConstOp::I32Add => ce.with_i32_add(), + ExtConstOp::I32Sub => ce.with_i32_sub(), + ExtConstOp::I32Mul => ce.with_i32_mul(), + ExtConstOp::I64Add => ce.with_i64_add(), + ExtConstOp::I64Sub => ce.with_i64_sub(), + ExtConstOp::I64Mul => ce.with_i64_mul(), + } + } +} + +/// Build a `wasm_encoder::ConstExpr` from a full preserved extended-const +/// operator sequence (leading `global.get` + trailing arithmetic). The +/// `End` opcode is appended by `ConstExpr`'s own encoder. +pub(crate) fn ext_const_to_expr(ops: &[ExtConstOp]) -> ConstExpr { + let mut ce = ConstExpr::empty(); + for op in ops { + ce = op.append_to(ce); + } + ce +} + +/// The outcome of reading a `const`-first extended-const expression: either a +/// pure-constant fold (no `global.get` anywhere) that collapses to a single +/// value, or a runtime-dependent sequence that embeds a `global.get` partway +/// through (the operand-swapped `N + __memory_base` shape) and therefore must +/// be preserved verbatim so its global index can be remapped later (#338). +#[derive(Debug, Clone, PartialEq)] +pub(crate) enum ExtConstFold { + /// No `global.get` — the expression folded to this constant. + Value(T), + /// An embedded `global.get` was found; the COMPLETE operator sequence + /// (in input order) is preserved for later index remapping. The value is + /// runtime-dependent and stays `None` at the parse layer. + Extended(Vec), +} + +/// Read wasm-2.0 extended-const operators until `End`, appending each to `seq` +/// (which already holds the operators consumed so far, in INPUT ORDER). Used +/// by both the `global.get`-first path and the const-first path once an +/// embedded `global.get` has forced the preserve-and-remap route. +/// +/// Rejects any operator outside the extended-const set with a clear error +/// (mirrors `fold_extended_const_*`) so audit and CI surface the path instead +/// of silently dropping operators (#338 / LS-A-11). Operand order is preserved +/// verbatim — non-commutative `sub`/`mul` must not be reordered. +fn read_remaining_ext_const( + ops: &mut wasmparser::OperatorsReader<'_>, + mut seq: Vec, +) -> Result> { + loop { + let op = ops.read()?; + match op { + Operator::End => break, + Operator::GlobalGet { global_index } => seq.push(ExtConstOp::GlobalGet(global_index)), + Operator::I32Const { value } => seq.push(ExtConstOp::I32Const(value)), + Operator::I64Const { value } => seq.push(ExtConstOp::I64Const(value)), + Operator::I32Add => seq.push(ExtConstOp::I32Add), + Operator::I32Sub => seq.push(ExtConstOp::I32Sub), + Operator::I32Mul => seq.push(ExtConstOp::I32Mul), + Operator::I64Add => seq.push(ExtConstOp::I64Add), + Operator::I64Sub => seq.push(ExtConstOp::I64Sub), + Operator::I64Mul => seq.push(ExtConstOp::I64Mul), + other => { + return Err(Error::UnsupportedFeature(format!( + "unsupported extended-const operator after global.get: {other:?}" + ))); + } + } + } + Ok(seq) +} + +/// Read the remainder of a wasm-2.0 extended-const expression that begins +/// with `global.get`, after the leading `GlobalGet(first_index)` has already +/// been consumed by the caller. Returns the FULL operator sequence (leading +/// `global.get` + trailing arithmetic) up to but not including `End`. +/// +/// If the only remaining operator is `End` (a bare `global.get`), returns +/// `Ok(None)` so callers preserve the existing single-`GlobalGet` behaviour +/// verbatim. Otherwise returns `Ok(Some(ops))` with the complete sequence. +pub(crate) fn read_extended_const_global_get( + ops: &mut wasmparser::OperatorsReader<'_>, + first_index: u32, +) -> Result>> { + let seq = read_remaining_ext_const(ops, vec![ExtConstOp::GlobalGet(first_index)])?; + if seq.len() == 1 { + Ok(None) + } else { + Ok(Some(seq)) + } +} + /// A structured constant expression that preserves the operator and operands, /// allowing index remapping before final encoding to `wasm_encoder::ConstExpr`. #[derive(Debug, Clone)] @@ -37,6 +162,14 @@ pub enum ParsedConstExpr { RefNull(wasm_encoder::HeapType), RefFunc(u32), GlobalGet(u32), + /// A wasm-2.0 extended-const expression that begins with `global.get` + /// and continues with arithmetic (the position-independent + /// `__memory_base + N` shape). Its value is runtime-dependent, so the + /// complete operator sequence is preserved and re-emitted rather than + /// folded to a constant (#338). The `Vec` always holds the leading + /// `GlobalGet` plus at least one trailing operator; a bare `global.get` + /// with no trailing arithmetic stays a plain [`ParsedConstExpr::GlobalGet`]. + ExtendedGlobalGet(Vec), } impl ParsedConstExpr { @@ -51,6 +184,7 @@ impl ParsedConstExpr { ParsedConstExpr::RefNull(ht) => ConstExpr::ref_null(*ht), ParsedConstExpr::RefFunc(idx) => ConstExpr::ref_func(*idx), ParsedConstExpr::GlobalGet(idx) => ConstExpr::global_get(*idx), + ParsedConstExpr::ExtendedGlobalGet(ops) => ext_const_to_expr(ops), } } @@ -59,6 +193,11 @@ impl ParsedConstExpr { match self { ParsedConstExpr::RefFunc(idx) => ParsedConstExpr::RefFunc(maps.remap_func(*idx)), ParsedConstExpr::GlobalGet(idx) => ParsedConstExpr::GlobalGet(maps.remap_global(*idx)), + ParsedConstExpr::ExtendedGlobalGet(ops) => ParsedConstExpr::ExtendedGlobalGet( + ops.iter() + .map(|o| o.remap_global(|i| maps.remap_global(i))) + .collect(), + ), ParsedConstExpr::RefNull(ht) => { let remapped_ht = match ht { wasm_encoder::HeapType::Concrete(idx) => { @@ -459,23 +598,53 @@ fn parse_const_expr(expr: &wasmparser::ConstExpr<'_>) -> Result /// `i32.mul` / further `i32.const` to a small evaluation stack with /// wrapping semantics per the wasm execution model. /// -/// Returns the final stack value if exactly one value remains at `End`. -/// Rejects unsupported operators (any non-extended-const op) with a -/// clear error so audit and CI surface the path instead of silently -/// dropping operators (LS-A-11). +/// If an embedded `global.get` is encountered partway through (the +/// operand-swapped `N + __memory_base` position-independent shape), the value +/// is runtime-dependent and CANNOT be folded: this captures the COMPLETE +/// operator sequence in input order (leading `i32.const` + ops already read + +/// the `global.get` + the remainder) and returns [`ExtConstFold::Extended`], +/// so the global index is remapped later instead of the un-remapped raw bytes +/// being emitted (#338). Pure-constant input still returns +/// [`ExtConstFold::Value`] — behaviourally identical to before. +/// +/// Rejects unsupported operators (any non-extended-const op) with a clear +/// error so audit and CI surface the path instead of silently dropping +/// operators (LS-A-11). pub(crate) fn fold_extended_const_i32( ops: &mut wasmparser::OperatorsReader<'_>, initial: i32, -) -> Result { +) -> Result> { let mut stack: Vec = vec![initial]; + // Operator sequence captured in INPUT ORDER, kept in lockstep with the + // fold so that if an embedded `global.get` appears we can hand back the + // exact operators verbatim (no fold, no reorder) for later remapping. + let mut seq: Vec = vec![ExtConstOp::I32Const(initial)]; loop { let op = ops.read()?; match op { Operator::End => break, - Operator::I32Const { value } => stack.push(value), - Operator::I32Add => fold_i32_binop(&mut stack, i32::wrapping_add)?, - Operator::I32Sub => fold_i32_binop(&mut stack, i32::wrapping_sub)?, - Operator::I32Mul => fold_i32_binop(&mut stack, i32::wrapping_mul)?, + Operator::I32Const { value } => { + stack.push(value); + seq.push(ExtConstOp::I32Const(value)); + } + Operator::I32Add => { + fold_i32_binop(&mut stack, i32::wrapping_add)?; + seq.push(ExtConstOp::I32Add); + } + Operator::I32Sub => { + fold_i32_binop(&mut stack, i32::wrapping_sub)?; + seq.push(ExtConstOp::I32Sub); + } + Operator::I32Mul => { + fold_i32_binop(&mut stack, i32::wrapping_mul)?; + seq.push(ExtConstOp::I32Mul); + } + Operator::GlobalGet { global_index } => { + // Runtime-dependent: abandon folding, preserve the full + // sequence (already-read ops + this global.get + remainder). + seq.push(ExtConstOp::GlobalGet(global_index)); + return read_remaining_ext_const(ops, seq).map(ExtConstFold::Extended); + } other => { return Err(Error::UnsupportedFeature(format!( "unsupported i32 extended-const operator: {other:?}" @@ -490,7 +659,7 @@ pub(crate) fn fold_extended_const_i32( stack.len() ))); } - Ok(stack[0]) + Ok(ExtConstFold::Value(stack[0])) } fn fold_i32_binop(stack: &mut Vec, op: fn(i32, i32) -> i32) -> Result<()> { @@ -504,20 +673,39 @@ fn fold_i32_binop(stack: &mut Vec, op: fn(i32, i32) -> i32) -> Result<()> { Ok(()) } -/// i64 counterpart to `fold_extended_const_i32`. +/// i64 counterpart to `fold_extended_const_i32`. An embedded `global.get` +/// likewise switches to the preserve-and-remap [`ExtConstFold::Extended`] +/// path instead of folding (#338). pub(crate) fn fold_extended_const_i64( ops: &mut wasmparser::OperatorsReader<'_>, initial: i64, -) -> Result { +) -> Result> { let mut stack: Vec = vec![initial]; + let mut seq: Vec = vec![ExtConstOp::I64Const(initial)]; loop { let op = ops.read()?; match op { Operator::End => break, - Operator::I64Const { value } => stack.push(value), - Operator::I64Add => fold_i64_binop(&mut stack, i64::wrapping_add)?, - Operator::I64Sub => fold_i64_binop(&mut stack, i64::wrapping_sub)?, - Operator::I64Mul => fold_i64_binop(&mut stack, i64::wrapping_mul)?, + Operator::I64Const { value } => { + stack.push(value); + seq.push(ExtConstOp::I64Const(value)); + } + Operator::I64Add => { + fold_i64_binop(&mut stack, i64::wrapping_add)?; + seq.push(ExtConstOp::I64Add); + } + Operator::I64Sub => { + fold_i64_binop(&mut stack, i64::wrapping_sub)?; + seq.push(ExtConstOp::I64Sub); + } + Operator::I64Mul => { + fold_i64_binop(&mut stack, i64::wrapping_mul)?; + seq.push(ExtConstOp::I64Mul); + } + Operator::GlobalGet { global_index } => { + seq.push(ExtConstOp::GlobalGet(global_index)); + return read_remaining_ext_const(ops, seq).map(ExtConstFold::Extended); + } other => { return Err(Error::UnsupportedFeature(format!( "unsupported i64 extended-const operator: {other:?}" @@ -532,7 +720,7 @@ pub(crate) fn fold_extended_const_i64( stack.len() ))); } - Ok(stack[0]) + Ok(ExtConstFold::Value(stack[0])) } fn fold_i64_binop(stack: &mut Vec, op: fn(i64, i64) -> i64) -> Result<()> { @@ -563,20 +751,23 @@ fn parse_const_expr_with_value( let op = ops.read()?; let (const_expr, value) = match op { - Operator::I32Const { value } => { - let folded = fold_extended_const_i32(&mut ops, value)?; - ( + Operator::I32Const { value } => match fold_extended_const_i32(&mut ops, value)? { + ExtConstFold::Value(folded) => ( ParsedConstExpr::I32Const(folded), Some(ConstExprValue::I32(folded)), - ) - } - Operator::I64Const { value } => { - let folded = fold_extended_const_i64(&mut ops, value)?; - ( + ), + // A const-first expr with an embedded `global.get` (`N + base`) is + // runtime-dependent: preserve the full sequence so the global index + // is remapped, and expose no constant value (#338). + ExtConstFold::Extended(seq) => (ParsedConstExpr::ExtendedGlobalGet(seq), None), + }, + Operator::I64Const { value } => match fold_extended_const_i64(&mut ops, value)? { + ExtConstFold::Value(folded) => ( ParsedConstExpr::I64Const(folded), Some(ConstExprValue::I64(folded)), - ) - } + ), + ExtConstFold::Extended(seq) => (ParsedConstExpr::ExtendedGlobalGet(seq), None), + }, Operator::F32Const { value } => ( ParsedConstExpr::F32Const(f32::from_bits(value.bits())), None, @@ -606,7 +797,19 @@ fn parse_const_expr_with_value( (ParsedConstExpr::RefNull(heap_type), None) } Operator::RefFunc { function_index } => (ParsedConstExpr::RefFunc(function_index), None), - Operator::GlobalGet { global_index } => (ParsedConstExpr::GlobalGet(global_index), None), + Operator::GlobalGet { global_index } => { + // A `global.get`-first expression may continue with extended-const + // arithmetic (the position-independent `__memory_base + N` shape). + // Its value is runtime-dependent, so it CANNOT be folded — the + // complete operator sequence is preserved and re-emitted with the + // global index remapped later during reindexing. Prior versions + // read only the leading `global.get` and silently dropped the + // trailing arithmetic, corrupting the offset/initializer (#338). + match read_extended_const_global_get(&mut ops, global_index)? { + Some(seq) => (ParsedConstExpr::ExtendedGlobalGet(seq), None), + None => (ParsedConstExpr::GlobalGet(global_index), None), + } + } _ => { return Err(Error::UnsupportedFeature(format!( "unsupported const expr operator: {:?}", diff --git a/meld-core/tests/const_expr_globalget.rs b/meld-core/tests/const_expr_globalget.rs new file mode 100644 index 0000000..69dfa93 --- /dev/null +++ b/meld-core/tests/const_expr_globalget.rs @@ -0,0 +1,472 @@ +//! #338 — extended-const expressions that BEGIN with `global.get` must be +//! preserved in full, not truncated to the leading `global.get`. +//! +//! meld used to read only the leading `global.get $base` of an initializer or +//! data/element offset of the position-independent `__memory_base + N` shape +//! (`global.get $base; i32.const N; i32.add`) and silently DROP the trailing +//! arithmetic — corrupting the value to `base + 0`. The fused module still +//! validates, so it is a silent miscompile. These executed-vs-wasmtime oracles +//! reproduce the bug: they PASS with the fix and FAIL (wrong value) without it. +//! +//! The value of a `global.get`-first expression is runtime-dependent (the +//! global's value), so meld cannot fold it to a constant; it must re-emit the +//! complete extended-const operator sequence with global indices remapped. +//! +//! `$base` is an IMPORTED immutable global (exactly `__memory_base`'s shape) — +//! a constant expression may only `global.get` an imported global. + +use meld_core::{Fuser, FuserConfig}; +use wasm_encoder::{ + CodeSection, Component, ConstExpr, DataSection, DataSegment, DataSegmentMode, ExportKind, + ExportSection, Function, FunctionSection, GlobalSection, GlobalType, ImportSection, + Instruction, MemorySection, MemoryType, Module, ModuleSection, TypeSection, ValType, +}; +use wasmtime::{Engine, ExternType, Global, Instance, Module as RuntimeModule, Store, Val}; + +fn build_component(module: Module) -> Vec { + let mut component = Component::new(); + component.section(&ModuleSection(&module)); + component.finish() +} + +fn imported_base_global() -> GlobalType { + GlobalType { + val_type: ValType::I32, + mutable: false, + shared: false, + } +} + +fn regular_memory_section() -> MemorySection { + let mut memory = MemorySection::new(); + memory.memory(MemoryType { + minimum: 1, + maximum: None, + memory64: false, + shared: false, + page_size_log2: None, + }); + memory +} + +fn fuse_single(module: Module) -> Vec { + let component = build_component(module); + let mut fuser = Fuser::new(FuserConfig::default()); + fuser + .add_component_named(&component, Some("component-a")) + .unwrap(); + fuser.fuse().unwrap() +} + +/// Instantiate the fused core module, supplying `base` for every imported +/// (i32) global — meld leaves `$base` as an unresolved import, so we bind it +/// generically regardless of the module/name meld emits. +fn instantiate(fused: &[u8], base: i32) -> (Store<()>, Instance) { + let engine = Engine::default(); + let module = RuntimeModule::new(&engine, fused).expect("fused module must validate"); + let mut store = Store::new(&engine, ()); + let mut externs = Vec::new(); + for imp in module.imports() { + match imp.ty() { + ExternType::Global(gt) => { + let g = Global::new(&mut store, gt, Val::I32(base)).unwrap(); + externs.push(g.into()); + } + other => panic!( + "unexpected import {}::{} of type {other:?}", + imp.module(), + imp.name() + ), + } + } + let instance = Instance::new(&mut store, &module, &externs).unwrap(); + (store, instance) +} + +const BASE: i32 = 1000; + +/// Test 1 — GLOBAL INITIALIZER (`meld-core/src/merger.rs::convert_init_expr`). +/// +/// `g (mut i32) = global.get $base; i32.const 100; i32.add` with `$base = 1000` +/// must initialise to 1100. Pre-fix, `convert_init_expr`'s `GlobalGet` arm +/// returned immediately with only `global.get $base`, dropping `+100`, so `g` +/// initialised to 1000 — this test FAILS (1000 != 1100) without the fix. +/// +/// Negatives (must stay unchanged): +/// * `h (mut i32) = i32.const 5; i32.const 10; i32.add` → 15 (pure const fold) +/// * `k (mut i32) = i32.const 100; global.get $base; i32.add` → 1100 (const-first +/// with an EMBEDDED global.get — the operand-swapped `N + base` shape). In +/// SINGLE-module fusion this happens to read correctly only because +/// `__memory_base` stays at import index 0, so the (formerly un-remapped) +/// global index still points at it. The multi-module test below shifts that +/// index off 0 and is the real regression guard for the embedded case (#338). +#[test] +fn test_338_global_initializer_extended_const_preserved() { + let mut types = TypeSection::new(); + types.ty().function([], [ValType::I32]); // type 0: () -> i32 + + let mut imports = ImportSection::new(); + imports.import("env", "__memory_base", imported_base_global()); // global 0 = $base + + let mut globals = GlobalSection::new(); + let mut_i32 = GlobalType { + val_type: ValType::I32, + mutable: true, + shared: false, + }; + // global 1: $g = global.get $base; i32.const 100; i32.add (=1100) + globals.global( + mut_i32, + &ConstExpr::global_get(0).with_i32_const(100).with_i32_add(), + ); + // global 2: $h = i32.const 5; i32.const 10; i32.add (=15, pure fold) + globals.global( + mut_i32, + &ConstExpr::i32_const(5).with_i32_const(10).with_i32_add(), + ); + // global 3: $k = i32.const 100; global.get $base; i32.add (=1100, const-first) + globals.global( + mut_i32, + &ConstExpr::i32_const(100).with_global_get(0).with_i32_add(), + ); + + let mut functions = FunctionSection::new(); + functions.function(0); + functions.function(0); + functions.function(0); + + let mut exports = ExportSection::new(); + exports.export("get_g", ExportKind::Func, 0); + exports.export("get_h", ExportKind::Func, 1); + exports.export("get_k", ExportKind::Func, 2); + + let mut code = CodeSection::new(); + for global_index in [1u32, 2, 3] { + let mut f = Function::new([]); + f.instruction(&Instruction::GlobalGet(global_index)); + f.instruction(&Instruction::End); + code.function(&f); + } + + let mut module = Module::new(); + module + .section(&types) + .section(&imports) + .section(&functions) + .section(&globals) + .section(&exports) + .section(&code); + + let fused = fuse_single(module); + let (mut store, instance) = instantiate(&fused, BASE); + + let get_g = instance + .get_typed_func::<(), i32>(&mut store, "get_g") + .unwrap(); + let get_h = instance + .get_typed_func::<(), i32>(&mut store, "get_h") + .unwrap(); + let get_k = instance + .get_typed_func::<(), i32>(&mut store, "get_k") + .unwrap(); + + // THE BUG: pre-fix this is 1000 (base + 0), the dropped `+100`. + assert_eq!( + get_g.call(&mut store, ()).unwrap(), + 1100, + "global.get-first extended-const initializer must be base+100=1100, \ + not the truncated base+0=1000 (#338)" + ); + // Negative: pure const-fold unchanged. + assert_eq!(get_h.call(&mut store, ()).unwrap(), 15); + // Negative: const-first with an embedded global.get unchanged. + assert_eq!(get_k.call(&mut store, ()).unwrap(), 1100); +} + +/// Test 2 — DATA-SEGMENT OFFSET (`meld-core/src/segments.rs`). +/// +/// An active data segment at offset `global.get $base; i32.const 100; i32.add` +/// (`$base = 1000`) must place its bytes at 1100. Pre-fix the offset truncated +/// to `global.get $base` = 1000, so the bytes landed at 1000 and address 1100 +/// read back zero — this test FAILS without the fix. +/// +/// Negative: a BARE `global.get $base` offset (no trailing arithmetic) must be +/// unchanged — its bytes land at exactly 1000. +#[test] +fn test_338_data_segment_offset_extended_const_preserved() { + const PAYLOAD: [u8; 4] = [0xDE, 0xAD, 0xBE, 0xEF]; + const BARE: [u8; 2] = [0x11, 0x22]; + + let mut imports = ImportSection::new(); + imports.import("env", "__memory_base", imported_base_global()); // global 0 = $base + + let mut exports = ExportSection::new(); + exports.export("mem", ExportKind::Memory, 0); + + // Extended offset: global.get $base; i32.const 100; i32.add → 1100. + let ext_offset = ConstExpr::global_get(0).with_i32_const(100).with_i32_add(); + // Bare offset: global.get $base → 1000 (unchanged control). + let bare_offset = ConstExpr::global_get(0); + + let mut data = DataSection::new(); + data.segment(DataSegment { + mode: DataSegmentMode::Active { + memory_index: 0, + offset: &ext_offset, + }, + data: PAYLOAD, + }); + data.segment(DataSegment { + mode: DataSegmentMode::Active { + memory_index: 0, + offset: &bare_offset, + }, + data: BARE, + }); + + // Section order: Type, Import, Memory, Global, Export, DataCount/Code, Data. + let mut module = Module::new(); + module + .section(&imports) + .section(®ular_memory_section()) + .section(&exports) + .section(&data); + + let fused = fuse_single(module); + let (mut store, instance) = instantiate(&fused, BASE); + + let mem = instance.get_memory(&mut store, "mem").expect("mem export"); + let data = mem.data(&store); + + // THE BUG: pre-fix the payload lands at 1000, so 1100..1104 reads zero. + assert_eq!( + &data[1100..1104], + &PAYLOAD, + "data segment must land at base+100=1100, not the truncated base+0=1000 (#338)" + ); + // Negative: bare global.get offset unchanged — bytes at exactly 1000. + assert_eq!( + &data[1000..1002], + &BARE, + "bare global.get offset must be unchanged" + ); +} + +/// Instantiate a fused module binding each imported (i32) global by NAME: +/// `__memory_base` -> `base`, anything else (e.g. `__stack_pointer`) -> +/// `other`. Distinct values let the test detect a wrong global index — reading +/// the wrong import yields the wrong arithmetic result. +fn instantiate_by_name(fused: &[u8], base: i32, other: i32) -> (Store<()>, Instance) { + let engine = Engine::default(); + let module = RuntimeModule::new(&engine, fused).expect("fused module must validate"); + let mut store = Store::new(&engine, ()); + let mut externs = Vec::new(); + for imp in module.imports() { + match imp.ty() { + ExternType::Global(gt) => { + let v = if imp.name() == "__memory_base" { + base + } else { + other + }; + let g = Global::new(&mut store, gt, Val::I32(v)).unwrap(); + externs.push(g.into()); + } + other_ty => panic!( + "unexpected import {}::{} of type {other_ty:?}", + imp.module(), + imp.name() + ), + } + } + let instance = Instance::new(&mut store, &module, &externs).unwrap(); + (store, instance) +} + +/// Component A: imports `env::__stack_pointer` and exports a getter for it. +/// Fused FIRST, so `__stack_pointer` claims merged import-global index 0 and +/// pushes B's `__memory_base` to a NON-zero merged index. +fn build_component_stack_pointer() -> Vec { + let mut types = TypeSection::new(); + types.ty().function([], [ValType::I32]); + + let mut imports = ImportSection::new(); + imports.import("env", "__stack_pointer", imported_base_global()); // global 0 + + let mut functions = FunctionSection::new(); + functions.function(0); + + let mut exports = ExportSection::new(); + exports.export("get_sp", ExportKind::Func, 0); + + let mut code = CodeSection::new(); + let mut f = Function::new([]); + f.instruction(&Instruction::GlobalGet(0)); + f.instruction(&Instruction::End); + code.function(&f); + + let mut module = Module::new(); + module + .section(&types) + .section(&imports) + .section(&functions) + .section(&exports) + .section(&code); + build_component(module) +} + +/// Component B: imports `env::__memory_base` as its ONLY global (local index 0) +/// and defines `$b (mut i32) = i32.const 100; global.get $__memory_base; +/// i32.add` — the operand-swapped `N + base` extended-const shape. +fn build_component_membase_initializer() -> Vec { + let mut types = TypeSection::new(); + types.ty().function([], [ValType::I32]); + + let mut imports = ImportSection::new(); + imports.import("env", "__memory_base", imported_base_global()); // local global 0 + + let mut globals = GlobalSection::new(); + let mut_i32 = GlobalType { + val_type: ValType::I32, + mutable: true, + shared: false, + }; + // local global 1: i32.const 100; global.get $__memory_base; i32.add (=base+100) + globals.global( + mut_i32, + &ConstExpr::i32_const(100).with_global_get(0).with_i32_add(), + ); + + let mut functions = FunctionSection::new(); + functions.function(0); + + let mut exports = ExportSection::new(); + exports.export("get_b", ExportKind::Func, 0); + + let mut code = CodeSection::new(); + let mut f = Function::new([]); + f.instruction(&Instruction::GlobalGet(1)); + f.instruction(&Instruction::End); + code.function(&f); + + let mut module = Module::new(); + module + .section(&types) + .section(&imports) + .section(&functions) + .section(&globals) + .section(&exports) + .section(&code); + build_component(module) +} + +/// Test 3 — MULTI-MODULE base-shift for the CONST-FIRST embedded `global.get` +/// (`meld-core/src/merger.rs::convert_init_expr`, i32.const arm). +/// +/// Two independent components are fused. Component A (fused first) imports +/// `__stack_pointer`, claiming merged import-global index 0. Component B's +/// `__memory_base` is therefore remapped to merged index 1, while B's stored +/// initializer bytes name its LOCAL index 0. B's global initializer is +/// `i32.const 100; global.get $__memory_base; i32.add`. +/// +/// Pre-fix, `convert_init_expr` folded `i32.const 100`, hit the embedded +/// `global.get`, its i32 arm returned `Err`, and the merger fell back to the +/// module's ORIGINAL un-remapped bytes — emitting `global.get 0`, which in the +/// merged module is `__stack_pointer` (bound to 5000 here), so `$b` initialised +/// to 5100. Observed pre-fix failure: `assertion left == right` with +/// `left: 5100, right: 1100`. Post-fix the sequence is preserved and the global +/// index remapped 0 -> 1, so `$b` reads `__memory_base` (1000) -> 1100. +#[test] +fn test_338_multimodule_const_first_embedded_globalget_remaps() { + const OTHER: i32 = 5000; // __stack_pointer sentinel, != BASE + + let component_a = build_component_stack_pointer(); + let component_b = build_component_membase_initializer(); + + let mut fuser = Fuser::new(FuserConfig::default()); + fuser + .add_component_named(&component_a, Some("comp-a")) + .unwrap(); + fuser + .add_component_named(&component_b, Some("comp-b")) + .unwrap(); + let fused = fuser.fuse().unwrap(); + + // Confirm the setup actually shifts __memory_base off import index 0 — + // otherwise the regression could not manifest. + let engine = Engine::default(); + let module = RuntimeModule::new(&engine, &fused).expect("fused validates"); + let membase_pos = module + .imports() + .position(|imp| imp.name() == "__memory_base") + .expect("__memory_base is an import"); + assert_ne!( + membase_pos, 0, + "test precondition: __memory_base must NOT be at import index 0, \ + else the base-shift regression cannot trigger" + ); + + let (mut store, instance) = instantiate_by_name(&fused, BASE, OTHER); + let get_b = instance + .get_typed_func::<(), i32>(&mut store, "get_b") + .unwrap(); + + assert_eq!( + get_b.call(&mut store, ()).unwrap(), + BASE + 100, + "const-first embedded global.get must remap $__memory_base to its \ + merged index (base+100=1100), not emit the un-remapped local index \ + that reads $__stack_pointer (5100) (#338)" + ); +} + +/// Test 4 — DATA-SEGMENT offset with a CONST-FIRST embedded `global.get` +/// (`meld-core/src/segments.rs`). +/// +/// An active data segment at offset `i32.const 100; global.get $base; i32.add` +/// (`$base = 1000`) must place its bytes at 1100. Pre-fix, the data path folded +/// `i32.const 100`, hit the embedded `global.get`, and `fold_extended_const_i32` +/// returned `Err`, which `parse_data_segments` propagated via `?` — making the +/// WHOLE fuse hard-fail (`fuser.fuse()` returned `Err`, rejecting valid input). +/// Post-fix the offset is preserved-and-remapped and the payload lands at 1100. +#[test] +fn test_338_data_segment_const_first_embedded_globalget() { + const PAYLOAD: [u8; 4] = [0xCA, 0xFE, 0xBA, 0xBE]; + + let mut imports = ImportSection::new(); + imports.import("env", "__memory_base", imported_base_global()); // global 0 = $base + + let mut exports = ExportSection::new(); + exports.export("mem", ExportKind::Memory, 0); + + // Const-first embedded offset: i32.const 100; global.get $base; i32.add → 1100. + let ext_offset = ConstExpr::i32_const(100).with_global_get(0).with_i32_add(); + + let mut data = DataSection::new(); + data.segment(DataSegment { + mode: DataSegmentMode::Active { + memory_index: 0, + offset: &ext_offset, + }, + data: PAYLOAD, + }); + + let mut module = Module::new(); + module + .section(&imports) + .section(®ular_memory_section()) + .section(&exports) + .section(&data); + + // Pre-fix this returned Err (hard-fail on valid input); post-fix it fuses. + let fused = fuse_single(module); + let (mut store, instance) = instantiate(&fused, BASE); + + let mem = instance.get_memory(&mut store, "mem").expect("mem export"); + let data = mem.data(&store); + assert_eq!( + &data[1100..1104], + &PAYLOAD, + "const-first embedded global.get data offset must land at base+100=1100 (#338)" + ); +} diff --git a/safety/requirements/safety-requirements.yaml b/safety/requirements/safety-requirements.yaml index 53cd362..4de0229 100644 --- a/safety/requirements/safety-requirements.yaml +++ b/safety/requirements/safety-requirements.yaml @@ -1817,4 +1817,42 @@ artifacts: KEEP `cabi_realloc` and do not defer grow. A live-allocator fuse still rejects `memory.grow` under rebasing (no behaviour change off the provably-safe path). + - id: SR-51 + type: sw-req + title: Extended-const exprs beginning with global.get are preserved, not truncated + description: > + meld shall preserve the FULL wasm-2.0 extended-const expression of a global + initializer or data/element-segment offset that begins with `global.get` — + e.g. `global.get $base; i32.const N; i32.add` (the position-independent + `__memory_base + N` / `__table_base + N` shape) — NOT truncate it to the + leading `global.get`, dropping the trailing arithmetic (#338). Because the + value is runtime-dependent, meld shall re-emit the complete extended-const + operator sequence (global indices remapped) rather than fold to a constant. + The const-FIRST case was already handled (#152/LS-A-11); this closes the + `global.get`-first case. A truncated offset is a silent miscompile: the + module still validates but places data / initialises globals at `base + 0` + instead of `base + N`. + status: proposed + tags: [const-expr, miscompile, pie, correctness, v0.41.0] + links: + - type: derives-from + target: SYS-1 + - type: mitigates + target: LS-A-11 + cited-source: + - uri: "https://github.com/pulseengine/meld/issues/338" + kind: github + last-checked: 2026-07-14 + release: v0.41.0 + fields: + implementation: + - meld-core/src/segments.rs + - meld-core/src/merger.rs + verification-method: test + verification-description: > + PLANNED. Executed-vs-wasmtime oracles reproducing #338: a global + initializer `g = base + 100` (base=1000) yields 1100 (not 1000) after + fuse; a data-segment active offset `base + N` places the segment at + base+N. Negative: the const-first case and a bare `global.get` (no + trailing arithmetic) are unchanged.