diff --git a/CHANGELOG.md b/CHANGELOG.md index f6eebfd..28d8137 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,31 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed +- **`fuse --component` emitted an invalid component for non-empty export + signatures (#355).** Bare *world* exports (e.g. `world root { export get-b: + func(i: s32) -> u32 }`) were all lifted with an empty `(func)` component type, + so `wasm-tools validate` rejected the output ("lowered parameter types [] do + not match the core function") — silently, after printing "Fusion complete!". + Three latent defects underlay it: the parser did not record func export-aliases + (compacting the component-function index space so lookups mis-resolved); the + wrapper never terminated a no-result func type (truncating it); and the + bare-export emitter advanced the func index by 1 per export while each export + also binds an export-alias, shifting every export onto the previous one's type. + Now each export lifts with its real signature and the output validates. + **Falsification:** `component_bare_export_355.rs` fuses the exact fixtures and + asserts a valid component + correct per-export arities (get-b (1,1), set-b + (2,0), ptr-b (0,1)); the standard interface-export path is unaffected. +- **A component exporting ≥2 interfaces bound the wrong instance to the 2nd+ + export (found by the #355 Mythos pass).** The interface-export loop advanced + the component-instance index by 1 per exported interface, but exporting an + instance also binds an export-alias index — so the second and later interface + exports referenced the previous interface's alias and silently exported the + wrong functions (the output still validated). Now advances by 2. + **Falsification:** `component_multi_interface_instance_idx.rs` fuses a + two-interface component and asserts each exported interface resolves to its own + functions. + ## [0.41.2] - 2026-07-16 Soundness patch: stale relocation metadata can no longer silently miscompile diff --git a/meld-core/src/component_wrap.rs b/meld-core/src/component_wrap.rs index 52384e7..b07b394 100644 --- a/meld-core/src/component_wrap.rs +++ b/meld-core/src/component_wrap.rs @@ -2036,7 +2036,13 @@ fn assemble_component( None, ); component.section(&exp); - component_instance_idx += 1; + // Exporting the instance binds a NEW component-instance index (an export + // alias), same as the bare-func path (#355) — so the next interface's + // synthetic instance lands two indices later. `+= 1` made the 2nd and + // later exported interfaces reference the previous interface's alias, + // silently exporting the wrong functions (surfaced by the #355 Mythos + // pass; UCA-CP-1). The output still `validate`s, so it was silent. + component_instance_idx += 2; } // Handle bare function exports (e.g., "run" without an interface wrapper). @@ -2069,15 +2075,90 @@ fn assemble_component( let aliased_core_func = core_func_idx; core_func_idx += 1; - // Define the function type — use default run type (func() -> void) - let wrapper_func_type = define_bare_func_type(&mut component, &mut component_type_idx); + // #355: carry the SOURCE export's lift type through, instead of assuming + // a bare `func()`. The `func()` assumption only held for `run`-style + // exports; for any non-empty signature it produced an INVALID component + // (`wasm-tools validate`: "lowered parameter types [] do not match ..."). + // Bare *world* exports (e.g. `world root { export get-b: func(i:s32) + // -> u32 }`) hit exactly this path. `comp_export.index` now resolves + // correctly because the parser records func export-aliases so the + // component-function index space is aligned (see ComponentFuncDef::ExportAlias). + let source_lift = source + .component_func_defs + .get(comp_export.index as usize) + .and_then(|def| match def { + parser::ComponentFuncDef::Lift(canon_idx) => { + source.canonical_functions.get(*canon_idx) + } + _ => None, + }) + .and_then(|ce| match ce { + parser::CanonicalEntry::Lift { + type_index, + options, + .. + } => Some((*type_index, options.clone())), + _ => None, + }); + + // A string/list-returning bare export needs memory + encoding + + // post-return, same as the interface path. Alias the cleanup right + // after the main func so the core-func index stays contiguous. + let post_return_name = format!("cabi_post_{}", func_name); + let has_post_return = fused_info + .exports + .iter() + .any(|(n, k, _)| *k == wasmparser::ExternalKind::Func && *n == post_return_name); + let post_return_core_idx = if has_post_return { + let mut a = ComponentAliasSection::new(); + a.alias(Alias::CoreInstanceExport { + instance: fused_instance, + kind: ExportKind::Func, + name: &post_return_name, + }); + component.section(&a); + let idx = core_func_idx; + core_func_idx += 1; + Some(idx) + } else { + None + }; + + // Carry the source type; only fall back to `func()` when the export + // truly has no discoverable lift type (keeps the historical last resort). + let wrapper_func_type = match &source_lift { + Some((type_index, _)) => define_source_type_in_wrapper( + &mut component, + source, + *type_index, + &mut component_type_idx, + &mut type_remap, + )?, + None => define_bare_func_type(&mut component, &mut component_type_idx), + }; + + // Lift options: memory + string encoding + post-return when cleanup is + // needed (scalar exports keep the empty option list). + let mut lift_options: Vec = Vec::new(); + if let Some(pr_idx) = post_return_core_idx { + lift_options.push(CanonicalOption::Memory(0)); + let enc = source_lift + .as_ref() + .map(|(_, opts)| source_string_encoding_option(opts.string_encoding)) + .unwrap_or(CanonicalOption::UTF8); + lift_options.push(enc); + lift_options.push(CanonicalOption::PostReturn(pr_idx)); + } - // Canon lift (bare functions like `run` take no arguments and return nothing) let mut canon = CanonicalFunctionSection::new(); - canon.lift(aliased_core_func, wrapper_func_type, []); + canon.lift(aliased_core_func, wrapper_func_type, lift_options); component.section(&canon); - // Export as a bare function + // Export as a bare function. `component_func_idx` is the canon-lift's + // component-func index. Exporting a `Func` binds a NEW component-func + // index (an export alias), so the NEXT lift lands two indices later — + // advance by 2, not 1. (#355: `+= 1` made each export reference the + // previous export's alias instead of its own lift.) let mut exp = ComponentExportSection::new(); exp.export( func_name, @@ -2086,7 +2167,7 @@ fn assemble_component( None, ); component.section(&exp); - component_func_idx += 1; + component_func_idx += 2; } Ok(component.finish()) @@ -2882,6 +2963,13 @@ fn define_source_type_in_wrapper( // Component model now only supports a single anonymous result; // emit the first result type. func_enc.result(Some(enc_results[0].1)); + } else { + // No results (e.g. `set-b: func(i: s32, v: u32)` returning + // void). `result()` MUST still be called to terminate the + // func type — omitting it left the type unencoded, so the + // decoder hit EOF (#355). Latent until a no-result export + // was routed through this helper. + func_enc.result(None); } } component.section(&types); diff --git a/meld-core/src/p3_stream.rs b/meld-core/src/p3_stream.rs index d5a60fe..a27f1b1 100644 --- a/meld-core/src/p3_stream.rs +++ b/meld-core/src/p3_stream.rs @@ -791,6 +791,30 @@ pub fn export_stream_elements(comp: &ParsedComponent, export_name: &str) -> Vec< // in LS-R-11's "limits" block. Vec::new() } + ComponentFuncDef::ExportAlias(target) => { + // A func export-alias (#355) just re-binds another func index; the + // aliased func carries the real signature. Follow it one hop (no + // cycles: an export aliases an already-defined func). + match comp.component_func_defs.get(*target as usize) { + Some(ComponentFuncDef::Lift(canon_idx)) => { + match comp.canonical_functions.get(*canon_idx) { + Some(CanonicalEntry::Lift { type_index, .. }) => { + stream_elements_in_typeref( + comp, + &wasmparser::ComponentTypeRef::Func(*type_index), + ) + } + _ => Vec::new(), + } + } + Some(ComponentFuncDef::Import(import_idx)) => comp + .imports + .get(*import_idx) + .map(|imp| stream_elements_in_typeref(comp, &imp.ty)) + .unwrap_or_default(), + _ => Vec::new(), + } + } } } diff --git a/meld-core/src/parser.rs b/meld-core/src/parser.rs index 39c76cf..9ef3779 100644 --- a/meld-core/src/parser.rs +++ b/meld-core/src/parser.rs @@ -95,6 +95,14 @@ pub enum ComponentFuncDef { Lift(usize), /// An `InstanceExport { kind: Func }` alias. Index into `component_aliases`. InstanceExportAlias(usize), + /// A component-level `(export "name" (func N))` of a `Func` also binds a + /// NEW component-function index (an export alias of func `N`). wit-component + /// interleaves these with `canon lift`s, so failing to record them here + /// leaves `component_func_defs` compacted while `ComponentExport::index` + /// (and every other reference) uses the real, interleaved index space — + /// making `component_func_defs[export.index]` read the wrong slot (#355). + /// The `u32` is the exported func index this aliases. + ExportAlias(u32), } /// Records what created each component-level instance index. @@ -900,6 +908,16 @@ impl ComponentParser { .push(ComponentTypeDef::ExportAlias(export.index)); } } + // A `Func` export also binds a new component-function index + // (an export alias). Record it so `component_func_defs` stays + // aligned with the real, interleaved func index space that + // `export.index` uses (#355). Without this the index space is + // silently compacted and lookups by func index mis-resolve. + if export.kind == ComponentExternalKind::Func { + component + .component_func_defs + .push(ComponentFuncDef::ExportAlias(export.index)); + } component.exports.push(ComponentExport { name: export.name.0.to_string(), kind: export.kind, diff --git a/meld-core/tests/component_bare_export_355.rs b/meld-core/tests/component_bare_export_355.rs new file mode 100644 index 0000000..c201265 --- /dev/null +++ b/meld-core/tests/component_bare_export_355.rs @@ -0,0 +1,98 @@ +//! #355 — `fuse --component` must lift bare *world* exports with their real +//! signatures, not an empty `(func)`. +//! +//! avrabe's fixtures (`tests/reloc351/{a,b}.wasm`) expose bare world exports: +//! `get-b: func(i:s32)->u32`, `set-b: func(i:s32,v:u32)` (void), +//! `ptr-b: func()->u32`. Before the fix every export was lifted with an empty +//! component type, producing an INVALID component (`wasm-tools validate`: +//! "lowered parameter types [] do not match [I32] of core function 0"). +//! +//! The fix touched three latent defects, so the oracle checks two things: +//! 1. the output is a **valid** component (catches the empty-type bug and the +//! missing no-result func encoding), and +//! 2. each export lifts with its **correct arity** (catches the export-alias +//! index-skip that shifted every export onto the previous one's type). + +use meld_core::parser::{CanonicalEntry, ComponentFuncDef, ComponentParser, ComponentTypeKind}; +use meld_core::{Fuser, FuserConfig, OutputFormat}; + +fn fuse_as_component(paths: &[&str]) -> Vec { + let config = FuserConfig { + output_format: OutputFormat::Component, + ..Default::default() + }; + let mut fuser = Fuser::new(config); + for p in paths { + let bytes = std::fs::read(p).unwrap_or_else(|e| panic!("read {p}: {e}")); + fuser + .add_component_named(&bytes, Some(p)) + .unwrap_or_else(|e| panic!("add {p}: {e:?}")); + } + fuser.fuse().expect("fuse --component") +} + +/// (params, results) arity of each Func export, resolved through the component's +/// func-index space → lift → type. This is exactly the path the wrapper mis-used. +fn export_arities(bytes: &[u8]) -> std::collections::HashMap { + let pc = ComponentParser::new().parse(bytes).expect("parse output"); + let mut out = std::collections::HashMap::new(); + for e in &pc.exports { + if e.kind != wasmparser::ComponentExternalKind::Func { + continue; + } + if let Some(ComponentFuncDef::Lift(ci)) = pc.component_func_defs.get(e.index as usize) + && let Some(CanonicalEntry::Lift { type_index, .. }) = pc.canonical_functions.get(*ci) + && let Some(td) = pc.get_type_definition(*type_index) + && let ComponentTypeKind::Function { params, results } = &td.kind + { + out.insert(e.name.clone(), (params.len(), results.len())); + } + } + out +} + +fn assert_valid_component(bytes: &[u8]) { + let mut v = wasmparser::Validator::new_with_features(wasmparser::WasmFeatures::all()); + v.validate_all(bytes) + .expect("#355: fused --component output must be a valid component"); +} + +#[test] +fn test_355_bare_world_exports_single_component() { + let out = fuse_as_component(&["../tests/reloc351/b.wasm"]); + assert_valid_component(&out); + let sig = export_arities(&out); + assert_eq!(sig.get("get-b"), Some(&(1, 1)), "get-b: func(i:s32)->u32"); + assert_eq!( + sig.get("set-b"), + Some(&(2, 0)), + "set-b: func(i:s32,v:u32)->()" + ); + assert_eq!(sig.get("ptr-b"), Some(&(0, 1)), "ptr-b: func()->u32"); +} + +#[test] +fn test_355_bare_world_exports_two_components() { + // avrabe's exact repro: `meld fuse --component a.wasm b.wasm`. + let out = fuse_as_component(&["../tests/reloc351/a.wasm", "../tests/reloc351/b.wasm"]); + assert_valid_component(&out); + let sig = export_arities(&out); + // Whichever component's world is exported, the bare-export arities must be + // correct (the bug shifted them regardless of component count). + for (name, arity) in &sig { + let expected = match name.as_str() { + n if n.starts_with("get-") => (1, 1), + n if n.starts_with("set-") => (2, 0), + n if n.starts_with("ptr-") => (0, 1), + _ => continue, + }; + assert_eq!( + arity, &expected, + "export {name} must lift with its real signature" + ); + } + assert!( + !sig.is_empty(), + "at least one bare world export must be lifted" + ); +} diff --git a/meld-core/tests/component_multi_interface_instance_idx.rs b/meld-core/tests/component_multi_interface_instance_idx.rs new file mode 100644 index 0000000..a91ab4c --- /dev/null +++ b/meld-core/tests/component_multi_interface_instance_idx.rs @@ -0,0 +1,98 @@ +//! Regression for the interface-export instance-index desync surfaced by the +//! #355 Mythos pass: a component exporting ≥2 interfaces bound the 2nd and later +//! interface exports to the *previous* interface's export-alias instance, so +//! they silently exported the wrong functions. The output still validated, so it +//! was silent (UCA-CP-1). Fix: advance `component_instance_idx` by 2 per +//! exported interface (the synthetic instance + the export-alias it binds). + +use meld_core::{Fuser, FuserConfig, OutputFormat}; +use wasmparser::*; + +const TWO_IFACE: &str = r#"(component + (core module $main + (type (;0;) (func (param i32) (result i32))) + (type (;1;) (func (result i32))) + (memory (;0;) 2) (global (;0;) (mut i32) i32.const 65536) + (export "memory" (memory 0)) (export "__stack_pointer" (global 0)) + (export "a:p/one#fa" (func $fa)) (export "b:p/two#fb" (func $fb)) + (func $fa (type 0) (param i32) (result i32) local.get 0) + (func $fb (type 1) (result i32) i32.const 7)) + (core instance $main (;0;) (instantiate $main)) + (alias core export $main "memory" (core memory (;0;))) + (type (;0;) (func (param "x" s32) (result u32))) + (alias core export $main "a:p/one#fa" (core func (;0;))) + (func (;0;) (type 0) (canon lift (core func 0))) + (instance (;0;) (export "fa" (func 0))) + (export (;1;) "a:p/one" (instance 0)) + (type (;1;) (func (result u32))) + (alias core export $main "b:p/two#fb" (core func (;1;))) + (func (;1;) (type 1) (canon lift (core func 1))) + (instance (;2;) (export "fb" (func 1))) + (export (;3;) "b:p/two" (instance 2)))"#; + +#[test] +fn multi_interface_exports_reference_their_own_instance() { + let input = wat::parse_str(TWO_IFACE).unwrap(); + let config = FuserConfig { + output_format: OutputFormat::Component, + ..Default::default() + }; + let mut fuser = Fuser::new(config); + fuser + .add_component_named(&input, Some("two_iface.wasm")) + .unwrap(); + let out = fuser.fuse().expect("fuse --component"); + + // The bug is SILENT — the output validates either way. + Validator::new_with_features(WasmFeatures::all()) + .validate_all(&out) + .expect("output validates"); + + // Resolve each exported interface instance → the func names it provides. + let mut instance_funcs: std::collections::HashMap> = + Default::default(); + let mut next_instance = 0u32; + let mut iface_exports: Vec<(String, u32)> = Vec::new(); + for payload in Parser::new(0).parse_all(&out) { + match payload.unwrap() { + Payload::ComponentInstanceSection(r) => { + for inst in r { + if let ComponentInstance::FromExports(exs) = inst.unwrap() { + let names = exs + .iter() + .filter(|e| e.kind == ComponentExternalKind::Func) + .map(|e| e.name.0.to_string()) + .collect(); + instance_funcs.insert(next_instance, names); + } + next_instance += 1; + } + } + Payload::ComponentExportSection(r) => { + for e in r { + let e = e.unwrap(); + if e.kind == ComponentExternalKind::Instance { + iface_exports.push((e.name.0.to_string(), e.index)); + next_instance += 1; // an instance export binds a new instance index + } + } + } + _ => {} + } + } + let funcs = |name: &str| { + iface_exports + .iter() + .find(|(n, _)| n == name) + .and_then(|(_, i)| instance_funcs.get(i)) + .cloned() + .unwrap_or_default() + }; + + assert!(funcs("a:p/one").contains("fa"), "a:p/one must export fa"); + assert!( + funcs("b:p/two").contains("fb"), + "b:p/two must export its OWN fb, got {:?} (index-desync would export a:p/one's fa)", + funcs("b:p/two") + ); +} diff --git a/safety/requirements/safety-requirements.yaml b/safety/requirements/safety-requirements.yaml index 51e2262..6f08718 100644 --- a/safety/requirements/safety-requirements.yaml +++ b/safety/requirements/safety-requirements.yaml @@ -1973,3 +1973,51 @@ artifacts: oracle `test_326_reloc_const_rebasing_end_to_end` (consistent relocs) and a real clang-22.1.8 consistent-reloc fixture still rebase correctly (ptr_b = 196608), so the backstop does not over-reject valid modules. + + - id: SR-54 + type: sw-req + title: --component lifts bare world exports with their real signatures + description: > + When emitting a P2 component (`--component` / `OutputFormat::Component`), + meld shall lift every export with the component-level function type of the + source export (params + results), NOT an empty `(func)`. The bare + world-export path previously hardcoded `func()` (a `run`-style assumption), + so any export with a non-empty signature produced an INVALID component — + `wasm-tools validate` rejects it ("lowered parameter types [] do not match + the core function"), and it cannot be instantiated. Silent: fusion printed + "Fusion complete!" with no warning (#355). Three latent defects underlay + it: (a) the parser did not record func export-aliases, so the + component-function index space was compacted and `component_func_defs[ + export.index]` mis-resolved; (b) the wrapper's func-type emitter never + terminated a NO-RESULT func type (`func_enc.result(None)` was not called), + truncating the type; (c) the bare-export emitter advanced the component-func + index by 1 per export while each export also binds an export-alias index, + so every export referenced the previous export's alias. All three are + fixed; the standard `--component` interface path is unaffected. + status: verified + tags: [component-wrap, canonical-abi, correctness, validity, v0.41.3] + links: + - type: derives-from + target: SYS-1 + cited-source: + - uri: "https://github.com/pulseengine/meld/issues/355" + kind: github + last-checked: 2026-07-16 + release: v0.41.3 + fields: + implementation: + - meld-core/src/parser.rs + - meld-core/src/component_wrap.rs + - meld-core/src/p3_stream.rs + verification-method: test + verification-description: > + VERIFIED. meld-core/tests/component_bare_export_355.rs on avrabe's exact + fixtures (tests/reloc351/{a,b}.wasm; bare world exports get-b/set-b/ptr-b): + `test_355_bare_world_exports_single_component` and `..._two_components` + assert (1) the fused --component output is a VALID component + (`wasmparser::Validator` with all features — catches the empty-type bug + and the un-terminated no-result func) and (2) each export lifts with its + real arity — get-b (1,1), set-b (2,0), ptr-b (0,1) — catching the + export-alias index-skip that shifted every export onto the previous + one's type. Full meld-core suite green (no regression from the parser + component_func_defs change). The #355 Mythos pass also surfaced the sibling instance-index desync (a component exporting >=2 interfaces bound the 2nd+ export to the wrong instance); fixed (component_instance_idx += 2) and pinned by component_multi_interface_instance_idx.rs.