From 25c7d78c3928fef4f9c266151d1995c017e2dae4 Mon Sep 17 00:00:00 2001 From: Moritz Zielke Date: Fri, 4 Sep 2026 22:04:13 +0200 Subject: [PATCH] fix(frontend/wasm): handle export names and name-section duplicates --- CHANGELOG.md | 10 + Cargo.lock | 24 +- Cargo.toml | 1 + examples/auth-component-no-auth/Cargo.lock | 4 +- .../auth-component-rpo-falcon512/Cargo.lock | 4 +- examples/basic-wallet-tx-script/Cargo.lock | 4 +- examples/basic-wallet/Cargo.lock | 4 +- examples/counter-contract/Cargo.lock | 4 +- examples/counter-note/Cargo.lock | 4 +- examples/p2id-note/Cargo.lock | 4 +- examples/p2id-tx-script/Cargo.lock | 4 +- examples/p2ide-note/Cargo.lock | 4 +- examples/storage-example/Cargo.lock | 4 +- frontend/wasm/Cargo.toml | 6 +- frontend/wasm/src/component/translator.rs | 4 +- frontend/wasm/src/module/build_ir.rs | 4 +- frontend/wasm/src/module/debug_info.rs | 226 +++++++++++- frontend/wasm/src/module/linker_stubs.rs | 11 +- frontend/wasm/src/module/mod.rs | 234 +++++++++++- frontend/wasm/src/module/module_env.rs | 1 + frontend/wasm/src/module/module_env/tests.rs | 334 ++++++++++++++++++ .../src/module/module_translation_state.rs | 4 +- .../components/assert-debug-test/Cargo.lock | 4 +- .../component-macros-account/Cargo.lock | 4 +- .../component-macros-note/Cargo.lock | 5 +- .../cross-ctx-account-word-arg/Cargo.lock | 4 +- .../cross-ctx-account-word/Cargo.lock | 4 +- .../components/cross-ctx-account/Cargo.lock | 4 +- .../cross-ctx-note-word-arg/Cargo.lock | 4 +- .../components/cross-ctx-note-word/Cargo.lock | 4 +- .../components/cross-ctx-note/Cargo.lock | 4 +- .../Cargo.lock | 5 +- .../fixtures/components/swapp-note/Cargo.lock | 4 +- .../duplicate-func-names-debug-info.shtest | 25 ++ .../duplicate-func-names-debug-info.wasm | Bin 0 -> 311 bytes tests/lit/debug/duplicate-func-names.wat | 41 +++ tests/lit/midenc/export-names.wat | 34 ++ tests/lit/midenc/lit.suite.toml | 2 +- 38 files changed, 978 insertions(+), 69 deletions(-) create mode 100644 tests/lit/debug/duplicate-func-names-debug-info.shtest create mode 100644 tests/lit/debug/duplicate-func-names-debug-info.wasm create mode 100644 tests/lit/debug/duplicate-func-names.wat create mode 100644 tests/lit/midenc/export-names.wat diff --git a/CHANGELOG.md b/CHANGELOG.md index e8c56704b4..39c675a4b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Compiler and `midenc` + +- Support duplicate function names in the Wasm name section, which might occur for Wasm generated + by rustc for the Miden target. Functions with a duplicated name get a unique linkage name while + debug info keeps the original source name. DWARF subprograms with a duplicated name resolve to + their function through `DW_AT_low_pc` instead of the ambiguous name. +- Honor Wasm function export names as primary HIR linkage symbols, exposing exported functions + under their exact export name for linkage and CLI entrypoint resolution while retaining raw + name-section names for source metadata and debug info. + ## [0.10.0] ### Compiler and `midenc` diff --git a/Cargo.lock b/Cargo.lock index 76b88a13e1..48b2018791 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1922,6 +1922,9 @@ version = "0.33.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0bf7f043f89559805f8c7cacc432749b2fa0d0a0a9ee46ce47164ed5ba7f126c" dependencies = [ + "fnv", + "hashbrown 0.16.1", + "indexmap 2.14.0", "stable_deref_trait", ] @@ -2003,6 +2006,12 @@ dependencies = [ "foldhash 0.1.5", ] +[[package]] +name = "hashbrown" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" + [[package]] name = "hashbrown" version = "0.17.1" @@ -3218,9 +3227,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", @@ -3978,6 +3987,7 @@ dependencies = [ "midenc-hir", "midenc-hir-symbol", "midenc-session", + "wasm-encoder 0.248.0", "wasmparser 0.248.0", "wasmprinter", "wat", @@ -7078,6 +7088,16 @@ dependencies = [ "wasmparser 0.247.0", ] +[[package]] +name = "wasm-encoder" +version = "0.248.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac92cf547bc18d27ecc521015c08c353b4f18b84ab388bb6d1b6b682c620d9b6" +dependencies = [ + "leb128fmt", + "wasmparser 0.248.0", +] + [[package]] name = "wasm-encoder" version = "0.256.0" diff --git a/Cargo.toml b/Cargo.toml index 2a321a7168..464ee18f32 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ cranelift-entity = "0.135" compact_str = { version = "0.9", default-features = false } darling = { version = "0.23", features = ["diagnostics"] } flate2 = "1.1" +gimli = { version = "^0.33", default-features = false } hashbrown = { version = "0.17", features = ["nightly"] } Inflector = "0.11" intrusive-collections = "0.10" diff --git a/examples/auth-component-no-auth/Cargo.lock b/examples/auth-component-no-auth/Cargo.lock index f3ef9c100e..2ce74224f6 100644 --- a/examples/auth-component-no-auth/Cargo.lock +++ b/examples/auth-component-no-auth/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/auth-component-rpo-falcon512/Cargo.lock b/examples/auth-component-rpo-falcon512/Cargo.lock index 964a353654..ccadefd71d 100644 --- a/examples/auth-component-rpo-falcon512/Cargo.lock +++ b/examples/auth-component-rpo-falcon512/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/basic-wallet-tx-script/Cargo.lock b/examples/basic-wallet-tx-script/Cargo.lock index 51651bd2e6..53017fcecf 100644 --- a/examples/basic-wallet-tx-script/Cargo.lock +++ b/examples/basic-wallet-tx-script/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/basic-wallet/Cargo.lock b/examples/basic-wallet/Cargo.lock index c9648c396c..dd66208105 100644 --- a/examples/basic-wallet/Cargo.lock +++ b/examples/basic-wallet/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/counter-contract/Cargo.lock b/examples/counter-contract/Cargo.lock index 34dc5734ed..ac68dd653d 100644 --- a/examples/counter-contract/Cargo.lock +++ b/examples/counter-contract/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/counter-note/Cargo.lock b/examples/counter-note/Cargo.lock index d751b8136e..dee47e61d1 100644 --- a/examples/counter-note/Cargo.lock +++ b/examples/counter-note/Cargo.lock @@ -1210,9 +1210,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/p2id-note/Cargo.lock b/examples/p2id-note/Cargo.lock index 0d3c4ef203..cbf90d8771 100644 --- a/examples/p2id-note/Cargo.lock +++ b/examples/p2id-note/Cargo.lock @@ -1202,9 +1202,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/p2id-tx-script/Cargo.lock b/examples/p2id-tx-script/Cargo.lock index b86fffb4bb..98ab2be739 100644 --- a/examples/p2id-tx-script/Cargo.lock +++ b/examples/p2id-tx-script/Cargo.lock @@ -1192,9 +1192,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/p2ide-note/Cargo.lock b/examples/p2ide-note/Cargo.lock index 91c3b9cfcb..ed19457e42 100644 --- a/examples/p2ide-note/Cargo.lock +++ b/examples/p2ide-note/Cargo.lock @@ -1202,9 +1202,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/examples/storage-example/Cargo.lock b/examples/storage-example/Cargo.lock index d8e4262ae2..b8fa3bd60b 100644 --- a/examples/storage-example/Cargo.lock +++ b/examples/storage-example/Cargo.lock @@ -1202,9 +1202,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/frontend/wasm/Cargo.toml b/frontend/wasm/Cargo.toml index 7df2bf985a..793dd78e5f 100644 --- a/frontend/wasm/Cargo.toml +++ b/frontend/wasm/Cargo.toml @@ -22,7 +22,7 @@ std = ["wasmparser/std", "gimli/std", "midenc-hir-symbol/std", "dep:wasmprinter" anyhow.workspace = true addr2line = "^0.26" cranelift-entity.workspace = true -gimli = { version = "^0.33", default-features = false, features = ['read'] } +gimli = { workspace = true, features = ["read"] } indexmap = "2.7" log.workspace = true miden-core.workspace = true @@ -43,3 +43,7 @@ wasmprinter = { workspace = true, optional = true } # NOTE: Use local paths for dev-only dependency to avoid relying on crates.io during packaging wat.workspace = true midenc-expect-test = { path = "../../tools/expect-test" } +# Tests need the `write` feature to synthesize DWARF +gimli = { workspace = true, features = ["read", "write"] } +# Matching the wasmparser/wasmprinter pinned in the workspace +wasm-encoder = "^0.248" diff --git a/frontend/wasm/src/component/translator.rs b/frontend/wasm/src/component/translator.rs index 1591997e3e..e299d70098 100644 --- a/frontend/wasm/src/component/translator.rs +++ b/frontend/wasm/src/component/translator.rs @@ -1467,7 +1467,7 @@ mod tests { .find_module(SymbolName::intern("main")) .expect("main module should be translated"); let start = ModuleBuilder::new(main) - .get_function("actual-start") + .get_function("aliased-start") .expect("actual start definition should be translated"); assert!( start @@ -1492,7 +1492,7 @@ mod tests { .find_module(SymbolName::intern("main")) .expect("main module should be translated"); let start = ModuleBuilder::new(main) - .get_function("actual-start") + .get_function("aliased-start") .expect("actual start definition should be translated"); assert!( start diff --git a/frontend/wasm/src/module/build_ir.rs b/frontend/wasm/src/module/build_ir.rs index bee70f7369..c69be45b51 100644 --- a/frontend/wasm/src/module/build_ir.rs +++ b/frontend/wasm/src/module/build_ir.rs @@ -168,7 +168,7 @@ pub fn build_ir_module( } let func_index = parsed_module.module.func_index(defined_func_idx); - let func_name = parsed_module.module.func_name(func_index).as_str(); + let func_name = parsed_module.module.source_func_name(func_index).as_str(); // Try to parse the function name as a MASM function ident to get the symbol path let Ok(func_ident) = FunctionIdent::from_str(func_name) else { @@ -218,8 +218,10 @@ pub fn build_ir_module( // If this is a linker stub that needs a synthesized body (function-type intrinsics, // module-context stubs, or Miden ABI calls), handle it here. + let source_name = parsed_module.module.source_func_name(func_index); if maybe_lower_linker_stub( function_ref, + source_name, &body_data.body, module_state, parsed_module.component_frontend_metadata.as_ref(), diff --git a/frontend/wasm/src/module/debug_info.rs b/frontend/wasm/src/module/debug_info.rs index 5a02b7d1de..b5c91e493b 100644 --- a/frontend/wasm/src/module/debug_info.rs +++ b/frontend/wasm/src/module/debug_info.rs @@ -128,7 +128,7 @@ pub fn collect_function_debug_info( for (defined_idx, body) in parsed_module.function_body_inputs.iter() { let func_index = module.func_index(defined_idx); - let func_name = module.func_name(func_index); + let source_name = module.source_func_name(func_index); if let Some(info) = build_function_debug_info( parsed_module, module_types, @@ -142,12 +142,12 @@ pub fn collect_function_debug_info( ) { debug!( "Collected debug info for function {}: {} locals", - func_name.as_str(), + source_name.as_str(), info.locals.len() ); map.insert(func_index, Rc::new(RefCell::new(info))); } else { - debug!("No debug info collected for function {}", func_name.as_str()); + debug!("No debug info collected for function {}", source_name.as_str()); } } @@ -167,7 +167,8 @@ fn build_function_debug_info( dwarf_locals: Option<&FxHashMap>, scheduled_vars: Option<&Vec>, ) -> Option { - let func_name = module.func_name(func_index); + let source_name = module.source_func_name(func_index); + let linkage_name = module.func_name(func_index); let dwarf_offset = parsed_module.wasm_file.dwarf_offset(body.body_offset); let (file_symbol, directory_symbol) = @@ -178,8 +179,13 @@ fn build_function_debug_info( compile_unit.directory = directory_symbol; compile_unit.producer = Some(Symbol::intern("midenc-frontend-wasm")); - let mut subprogram = Subprogram::new(func_name, compile_unit.file, line, column); + // Name the subprogram after the source name and set the linkage name if it differs, + // mirroring `DW_AT_name` and `DW_AT_linkage_name`. + let mut subprogram = Subprogram::new(source_name, compile_unit.file, line, column); subprogram.is_definition = true; + if source_name != linkage_name { + subprogram.linkage_name = Some(linkage_name); + } let wasm_signature = module_types[module.functions[func_index].signature].clone(); let locals = build_local_debug_info( @@ -442,7 +448,21 @@ fn collect_dwarf_local_data( let mut func_by_name = FxHashMap::default(); for (func_index, _) in module.functions.iter() { - let name = module.func_name(func_index).as_str().to_owned(); + // DWARF subprogram names are producer-emitted source names which can only identify + // functions whose name-section name is unique. rustc (for debug builds targeting Miden) + // can duplicate names in the name section. A duplicated name would make every DWARF + // subprogram with that name resolve to whichever function this map kept last. Skip + // such functions so their subprograms fall through to `low_pc`-based resolution in + // `resolve_subprogram_target`, which is unambiguous. + // + // Functions without explicit source name are also skipped because they get a fallback + // name which producers would not emit as DWARF name. + if !module.has_explicit_source_func_name(func_index) + || module.is_duplicate_source_func_name(func_index) + { + continue; + } + let name = module.source_func_name(func_index).as_str().to_owned(); func_by_name.insert(name, func_index); } @@ -1050,7 +1070,15 @@ fn func_local_index(func_index: FuncIndex, module: &Module) -> Option { #[cfg(test)] mod tests { + use cranelift_entity::EntityRef; + use gimli::write as dwarf_write; + use wasm_encoder::{CustomSection, Encode, SectionId}; + use super::*; + use crate::module::{ + module_env::{ModuleEnvironment, ParsedModule}, + types::ModuleTypesBuilder, + }; #[test] fn dwarf_file_paths_include_the_line_program_directory() { @@ -1095,4 +1123,190 @@ mod tests { assert_eq!(schedule[2].offset, 8); assert!(schedule[2].storage.is_some()); } + + /// A module with two functions sharing the same name-section name. + /// + /// Used in tests to verify the corresponding DWARF entries are associated with the correct + /// function. + const DUPLICATE_FUNC_NAMES_WAT: &str = r#" + (module $duplicate_func_names_debug_info.wasm + (type (;0;) (func (param i32) (result i32))) + (type (;1;) (func (result i32))) + (memory (;0;) 16) + (global $__stack_pointer (;0;) (mut i32) i32.const 1048576) + (export "memory" (memory 0)) + (export "test" (func $test)) + + ;; Both functions carry the same name-section name + (func $first (@name "foo") (;0;) (type 0) (param i32) (result i32) + local.get 0 + local.tee 0 + ) + (func $second (@name "foo") (;1;) (type 0) (param i32) (result i32) + local.get 0 + local.tee 0 + ) + (func $test (;2;) (type 1) (result i32) + i32.const 1 + call $first + i32.const 2 + call $second + i32.add + ) + ) + "#; + + fn parse_module(wasm: &[u8]) -> ParsedModule<'_> { + let config = crate::WasmTranslationConfig::default(); + let mut validator = wasmparser::Validator::new_with_features(crate::supported_features()); + let mut types = ModuleTypesBuilder::default(); + ModuleEnvironment::new(&config, &mut validator, &mut types) + .parse(wasmparser::Parser::new(0), wasm, &DiagnosticsHandler::default()) + .expect("wasm fixture should parse") + } + + /// Serializes one DWARF compilation unit holding a subprogram per `(name, low_pc, param)`: + /// the subprogram is named `name`, carries `DW_AT_low_pc` when `low_pc` is `Some`, and has a + /// single formal parameter named `param`. Returns the non-empty sections as + /// `(section name, bytes)` pairs, built with gimli's writer. + fn fixture_dwarf_sections( + subprograms: &[(&str, Option, &str)], + ) -> Vec<(&'static str, Vec)> { + let encoding = gimli::Encoding { + format: gimli::Format::Dwarf32, + version: 5, + address_size: 4, + }; + let mut dwarf = dwarf_write::DwarfUnit::new(encoding); + let root = dwarf.unit.root(); + dwarf + .unit + .get_mut(root) + .set(gimli::DW_AT_name, dwarf_write::AttributeValue::String(b"fixture.rs".to_vec())); + for &(name, low_pc, param) in subprograms { + let subprogram = dwarf.unit.add(root, gimli::DW_TAG_subprogram); + dwarf.unit.get_mut(subprogram).set( + gimli::DW_AT_name, + dwarf_write::AttributeValue::String(name.as_bytes().to_vec()), + ); + if let Some(low_pc) = low_pc { + dwarf.unit.get_mut(subprogram).set( + gimli::DW_AT_low_pc, + dwarf_write::AttributeValue::Address(dwarf_write::Address::Constant(low_pc)), + ); + } + let parameter = dwarf.unit.add(subprogram, gimli::DW_TAG_formal_parameter); + dwarf.unit.get_mut(parameter).set( + gimli::DW_AT_name, + dwarf_write::AttributeValue::String(param.as_bytes().to_vec()), + ); + // Add a location, otherwise the drops the parameter. This mimics how rustc describes + // parameters. + dwarf.unit.get_mut(parameter).set( + gimli::DW_AT_location, + dwarf_write::AttributeValue::Exprloc(dwarf_write::Expression::raw(vec![ + gimli::DW_OP_WASM_location.0, + 0x00, // wasm local + 0x00, // local index 0, since parameters map to the first locals + ])), + ); + } + + let mut sections = + dwarf_write::Sections::new(dwarf_write::EndianVec::new(gimli::LittleEndian)); + dwarf.write(&mut sections).expect("fixture DWARF should serialize"); + let mut out = Vec::new(); + sections + .for_each(|id, data| { + let bytes = data.slice(); + if !bytes.is_empty() { + out.push((id.name(), bytes.to_vec())); + } + Ok::<(), dwarf_write::Error>(()) + }) + .expect("section enumeration cannot fail"); + out + } + + /// Appends custom sections (as consumed by [`super::collect_dwarf_local_data`]) to a wasm + /// binary. Appending never moves existing sections, so the DWARF address space stays valid. + fn append_custom_sections(wasm: &mut Vec, sections: Vec<(&str, Vec)>) { + for (name, data) in sections { + wasm.push(SectionId::Custom as u8); // section classifier + CustomSection { + name: name.into(), + data: data.into(), + } + .encode(wasm); + } + } + + /// Assembles the duplicate-name fixture: the [`DUPLICATE_FUNC_NAMES_WAT`] module plus a + /// DWARF unit with: + /// + /// - two subprograms named `foo` (the duplicated name) + /// - one named `test` (unique) + fn duplicate_func_names_fixture() -> Vec { + let wasm = wat::parse_str(DUPLICATE_FUNC_NAMES_WAT).expect("fixture WAT should assemble"); + // Parse the module to learn each body's address in the DWARF address space. + let parsed = parse_module(&wasm); + let body_dwarf_offsets: Vec = parsed + .function_body_inputs + .iter() + .map(|(_, body)| parsed.wasm_file.dwarf_offset(body.body_offset)) + .collect(); + + // Set different parameter names (`a, b, c`) to enable distinguishing functions. + let dwarf = fixture_dwarf_sections(&[ + ("foo", Some(body_dwarf_offsets[0]), "a"), + ("foo", Some(body_dwarf_offsets[1]), "b"), + ("test", None, "c"), + ]); + let mut wasm = wasm; + append_custom_sections(&mut wasm, dwarf); + wasm + } + + #[test] + fn dwarf_subprograms_with_duplicate_names_resolve_via_low_pc() { + let wasm = duplicate_func_names_fixture(); + let parsed = parse_module(&wasm); + + // Verify that `foo` is duplicated and `test` is unique. + assert!(parsed.module.is_duplicate_source_func_name(FuncIndex::new(0))); + assert!(parsed.module.is_duplicate_source_func_name(FuncIndex::new(1))); + assert!(!parsed.module.is_duplicate_source_func_name(FuncIndex::new(2))); + + let collected = + collect_dwarf_local_data(&parsed, &parsed.module, &DiagnosticsHandler::default()); + let first_param_name = |func: usize| { + collected + .by_local + .get(&FuncIndex::new(func)) + .and_then(|locals| locals.get(&0)) + .and_then(|data| data.name) + }; + + // Each `foo` subprogram resolved through the `low_pc` to the corresponding `foo` function + assert_eq!(first_param_name(0), Some(Symbol::intern("a"))); + assert_eq!(first_param_name(1), Some(Symbol::intern("b"))); + // The uniquely named subprogram keeps resolving by name. + assert_eq!(first_param_name(2), Some(Symbol::intern("c"))); + } + + /// Regenerates lit fixture `tests/lit/debug/duplicate-func-names-debug-info.wasm`. + /// + /// Run with: + /// + /// `cargo test -p midenc-frontend-wasm --lib -- --ignored write_duplicate_names_dwarf_fixture` + #[test] + #[ignore = "writes the committed lit fixture; run explicitly to regenerate it"] + fn write_duplicate_names_dwarf_fixture() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../tests/lit/debug/duplicate-func-names-debug-info.wasm" + ); + let wasm = duplicate_func_names_fixture(); + std::fs::write(path, wasm).expect("write the lit fixture"); + } } diff --git a/frontend/wasm/src/module/linker_stubs.rs b/frontend/wasm/src/module/linker_stubs.rs index bb74e44468..4b2ada28ca 100644 --- a/frontend/wasm/src/module/linker_stubs.rs +++ b/frontend/wasm/src/module/linker_stubs.rs @@ -13,6 +13,7 @@ use midenc_hir::{ FunctionType, Op, SmallVec, SymbolPath, ValueRef, Visibility, diagnostics::WrapErr, dialects::builtin::{BuiltinOpBuilder, FunctionRef, ModuleBuilder, attributes::Signature}, + interner::Symbol, }; use midenc_hir_symbol::symbols; use wasmparser::{FunctionBody, Operator}; @@ -59,13 +60,14 @@ pub fn is_unreachable_stub(body: &FunctionBody<'_>) -> bool { } /// If `body` looks like a linker stub, lowers `function_ref` to a call to the -/// MASM callee derived from the function name and applies the appropriate +/// MASM callee derived from the function source name and applies the appropriate /// TransformStrategy. Returns `true` if handled, `false` otherwise. /// /// `frontend_metadata` holds the parsed core module's frontend metadata entries; they are /// consulted by module-context stub intrinsics (note intrinsics). pub fn maybe_lower_linker_stub( function_ref: FunctionRef, + source_name: Symbol, body: &FunctionBody<'_>, module_state: &mut ModuleTranslationState, frontend_metadata: &[FrontendMetadata], @@ -74,11 +76,8 @@ pub fn maybe_lower_linker_stub( return Ok(false); } - // Parse function name as MASM function ident: "ns::...::func" - let name_string = { - let borrowed = function_ref.borrow(); - borrowed.name().as_str().to_string() - }; + // Parse function source name as MASM function ident: "ns::...::func" + let name_string = source_name.as_str().to_string(); // Expect stub export names to be fully-qualified MASM paths already (e.g. "intrinsics::felt::add"). let func_ident = match midenc_hir::FunctionIdent::from_str(&name_string) { Ok(id) => id, diff --git a/frontend/wasm/src/module/mod.rs b/frontend/wasm/src/module/mod.rs index d0384a85f9..17463be6dd 100644 --- a/frontend/wasm/src/module/mod.rs +++ b/frontend/wasm/src/module/mod.rs @@ -1,15 +1,18 @@ //! Data structures for representing parsed Wasm modules. use alloc::{borrow::Cow, collections::BTreeMap}; -use core::{fmt, ops::Range}; +use core::{fmt, ops::Range, str::FromStr}; use cranelift_entity::{EntityRef, PrimaryMap, packed_option::ReservedValue}; use indexmap::IndexMap; -use midenc_hir::{FxHashMap, Ident, interner::Symbol}; +use midenc_hir::{FunctionIdent, FxHashMap, FxHashSet, Ident, SymbolPath, interner::Symbol}; use midenc_session::DiagnosticsHandler; use self::types::*; -use crate::{component::SignatureIndex, error::WasmResult, unsupported_diag}; +use crate::{ + component::SignatureIndex, error::WasmResult, intrinsics::Intrinsic, + miden_abi::is_miden_abi_module, unsupported_diag, +}; pub mod build_ir; pub mod debug_info; @@ -150,8 +153,22 @@ pub struct Module { pub memories: PrimaryMap, /// Parsed names section. + /// + /// Wasm's [name section] may contain duplicate names. Therefore it is recommended to + /// call [`Self::resolve_func_symbols`] after parsing and then get the unique function + /// symbol with [`Self::func_name`]. + /// + /// [name section]: https://webassembly.github.io/spec/core/appendix/custom.html#name-section name_section: NameSection, + /// Linkage name per function. Linkage names are unique. + /// + /// Built by [`Self::resolve_func_symbols`]. + func_linkages: PrimaryMap, + + /// Names in the name section that are shared by more than one function. + duplicate_source_names: FxHashSet, + /// The fallback name of this module, used if there is no module name in the name section, /// and there is no override specified name_fallback: Option, @@ -314,13 +331,215 @@ impl Module { .expect("No module name in the name section and no fallback name is set") } - /// Returns the name of the given function + /// Returns the unique name of the given function pub fn func_name(&self, index: FuncIndex) -> Symbol { + if let Some(sym) = self.func_linkages.get(index).copied() { + return sym; + } + // Fallback for unnamed functions + self.source_func_name(index) + } + + /// Returns the name according to the name section. + /// + /// Use this when referring to the original source code, e.g. in diagnostics or debug info. + /// + /// The returned name might not be unique, see `Self::name_section`. + pub fn source_func_name(&self, index: FuncIndex) -> Symbol { self.name_section .func_names .get(&index) - .cloned() - .unwrap_or(Symbol::intern(format!("func{}", index.as_u32()))) + .copied() + .unwrap_or_else(|| Self::fallback_func_name(index)) + } + + /// Synthesized name for functions without a name-section entry (e.g. stripped binaries). + // TODO check if there are more places that could use this + fn fallback_func_name(index: FuncIndex) -> Symbol { + Symbol::intern(format!("func{}", index.as_u32())) + } + + /// Returns true if the function has an explicit entry in the name section. + pub fn has_explicit_source_func_name(&self, index: FuncIndex) -> bool { + self.name_section.func_names.contains_key(&index) + } + + /// Returns true if the function's name-section name is shared with at least one other function. + /// + /// Requires [`Self::resolve_func_symbols`] to have run, which the Wasm frontend does during + /// parsing. + pub fn is_duplicate_source_func_name(&self, index: FuncIndex) -> bool { + self.name_section + .func_names + .get(&index) + .is_some_and(|name| self.duplicate_source_names.contains(name)) + } + + /// Resolves unique HIR linkage names for all functions in the module. + /// + /// WebAssembly function export names define the public interface and take precedence as + /// the primary HIR linkage symbol. Unexported functions use their name-section name (or + /// `func{index}` fallback if absent), disambiguated via `{name}_func{index}` (with `_` appended + /// to resolve collisions) if they conflict with an export name, a global variable name, or + /// another function with the same source name. + /// + /// Intrinsics and Miden ABI linker stubs are identified by name (see + /// [`maybe_lower_linker_stub`]) and considered internal, so an export or duplicate name that + /// identifies a known stub is an error. + /// + /// This method is idempotent. + /// + /// [name section]: https://webassembly.github.io/spec/core/appendix/custom.html#name-section + /// [`maybe_lower_linker_stub`]: linker_stubs::maybe_lower_linker_stub + pub fn resolve_func_symbols(&mut self, diagnostics: &DiagnosticsHandler) -> WasmResult<()> { + self.func_linkages.clear(); + self.duplicate_source_names.clear(); + + // Collect and validate function exports + let mut exported_as: FxHashMap = FxHashMap::default(); + let mut export_names: FxHashSet = FxHashSet::default(); + for (export_name, entity) in &self.exports { + let EntityIndex::Function(func_idx) = entity else { + continue; + }; + let export_sym = Symbol::intern(export_name.as_str()); + + if exported_as.insert(*func_idx, export_sym).is_some() { + unsupported_diag!( + diagnostics, + "exporting a function under multiple names is not supported: function index \ + `{}`, `{export_name}`)", + func_idx.as_u32() + ); + } + export_names.insert(export_sym); + + if let Ok(func_ident) = FunctionIdent::from_str(export_name.as_str()) { + let path = SymbolPath::from_masm_function_id(func_ident); + if Intrinsic::try_from(&path).is_ok() || is_miden_abi_module(&path) { + unsupported_diag!( + diagnostics, + "export name '{export_name}' identifies an intrinsic or Miden ABI linker \ + stub, which cannot be exported" + ); + } + } + } + + // Reject collisions between export names and global variable names + let mut global_names: FxHashSet = FxHashSet::default(); + for global_idx in self.globals.keys() { + let name = self.global_name(global_idx); + if export_names.contains(&name) { + unsupported_diag!( + diagnostics, + "export name '{name}' conflicts with a global variable name" + ); + } + global_names.insert(name); + } + + // Source names are explicit name-section names only; fallbacks (`func{index}`) are unique + // by construction and handled below via `taken`. Counting explicit names ensures that + // explicit names win over fallbacks. + let mut counts: FxHashMap = FxHashMap::default(); + for (func_idx, name) in &self.name_section.func_names { + if func_idx.index() >= self.functions.len() { + continue; + } + *counts.entry(*name).or_default() += 1; + } + for (name, count) in &counts { + if *count > 1 { + self.duplicate_source_names.insert(*name); + } + } + + // Collect source names that don't need to change. Only explicit (non-fallback) names + // participate here. Fallbacks are handled below via `taken`. + let mut keep_source_name: FxHashSet = FxHashSet::default(); + for (func_idx, name) in &self.name_section.func_names { + if func_idx.index() >= self.functions.len() { + continue; + } + if !exported_as.contains_key(func_idx) + && counts.get(name) == Some(&1) + && !export_names.contains(name) + && !global_names.contains(name) + { + keep_source_name.insert(*name); + } + } + + // Exports take precedence and are never renamed. + let mut taken: FxHashSet = export_names; + taken.extend(global_names); + taken.extend(keep_source_name.iter().copied()); + + // Assign dense linkage names in deterministic `FuncIndex` order. + let mut linkages: Vec> = vec![None; self.functions.len()]; + for fidx in self.functions.keys() { + let linkage = if let Some(export_name) = exported_as.get(&fidx) { + // Export name must become linkage name for external calls to resolve. + *export_name + } else { + // Not exported: linkage defaults to the source name. Fallbacks are unique among + // themselves, but an explicit name may be `funcY` while `Y` is unnamed. Explicit + // names win regardless of index order because `keep` is pre-seeded into `taken`, so + // the fallback loses here and is renamed below. + let (candidate, is_explicit) = match self.name_section.func_names.get(&fidx) { + Some(name) => (*name, true), + None => (Self::fallback_func_name(fidx), false), + }; + let can_use_source_as_linkage = if is_explicit { + keep_source_name.contains(&candidate) + } else if taken.contains(&candidate) { + false + } else { + taken.insert(candidate); + true + }; + + if can_use_source_as_linkage { + candidate + } else { + // Need to construct a unique linkage name. + let cand_str = candidate.as_str(); + if let Ok(func_id) = FunctionIdent::from_str(cand_str) { + let path = SymbolPath::from_masm_function_id(func_id); + if Intrinsic::try_from(&path).is_ok() || is_miden_abi_module(&path) { + unsupported_diag!( + diagnostics, + "duplicated function name '{cand_str}' identifies an intrinsic or \ + Miden ABI linker stub, which midenc recognizes by name, so it \ + cannot be renamed" + ); + } + } + + // Interning in the loop is fine because either the string is already interned + // (if taken) or it is about to be used as new symbol. + let mut unique_str = format!("{cand_str}_func{}", fidx.as_u32()); + while taken.contains(&Symbol::intern(unique_str.as_str())) { + unique_str.push('_'); + } + let unique_sym = Symbol::intern(unique_str); + taken.insert(unique_sym); + unique_sym + } + }; + linkages[fidx.index()] = Some(linkage); + } + + self.func_linkages.clear(); + self.func_linkages.reserve(linkages.len()); + for (idx, slot) in linkages.into_iter().enumerate() { + let linkage = slot.expect("linkage assigned for every function"); + let key = self.func_linkages.push(linkage); + debug_assert_eq!(key, FuncIndex::new(idx)); + } + + Ok(()) } /// Returns the name of the given data segment. @@ -450,6 +669,9 @@ impl FuncRefIndex { } } +/// Parsed names from the Wasm [name section]. +/// +/// [name section]: https://webassembly.github.io/spec/core/appendix/custom.html#name-section #[derive(Debug, Default)] pub struct NameSection { pub module_name: Option, diff --git a/frontend/wasm/src/module/module_env.rs b/frontend/wasm/src/module/module_env.rs index ad9feb054a..c87bcd7634 100644 --- a/frontend/wasm/src/module/module_env.rs +++ b/frontend/wasm/src/module/module_env.rs @@ -358,6 +358,7 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> { for payload in parser.parse_all(data) { self.parse_payload(payload.into_diagnostic()?, diagnostics)?; } + self.result.module.resolve_func_symbols(diagnostics)?; Ok(self.result) } diff --git a/frontend/wasm/src/module/module_env/tests.rs b/frontend/wasm/src/module/module_env/tests.rs index 29fb31cba5..2e882c04b9 100644 --- a/frontend/wasm/src/module/module_env/tests.rs +++ b/frontend/wasm/src/module/module_env/tests.rs @@ -1,4 +1,7 @@ +use cranelift_entity::EntityRef; + use super::*; +use crate::module::types::Global; #[test] fn standalone_dwarf_offsets_are_code_section_relative() { @@ -91,3 +94,334 @@ fn component_frontend_metadata_reports_missing_account_procedure_export() { "unexpected error: {err:?}" ); } + +fn module_with_func_names(names: &[(u32, &str)]) -> Module { + let mut module = Module::default(); + let max = names.iter().map(|(index, _)| *index).max(); + if let Some(max) = max { + // Dummy signature since here we only care about function names + let sig = SignatureIndex::from_u32(0); + for _ in 0..=max { + module.push_function(sig); + } + } + for (index, name) in names { + module + .name_section + .func_names + .insert(FuncIndex::new(*index as usize), Symbol::intern(*name)); + } + module +} + +#[test] +fn duplicate_func_names_are_renamed() { + let mut module = module_with_func_names(&[(0, "foo"), (2, "foo"), (1, "bar")]); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo_func0"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "bar"); + assert_eq!(module.func_name(FuncIndex::new(2)).as_str(), "foo_func2"); + + // The name section is not modified + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "bar"); + assert_eq!(module.source_func_name(FuncIndex::new(2)).as_str(), "foo"); +} + +#[test] +fn unique_func_names_are_kept() { + let mut module = module_with_func_names(&[(0, "foo"), (1, "bar")]); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "bar"); + + // Nothing was renamed + assert_eq!(module.func_name(FuncIndex::new(0)), module.source_func_name(FuncIndex::new(0))); + assert_eq!(module.func_name(FuncIndex::new(1)), module.source_func_name(FuncIndex::new(1))); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "bar"); +} + +#[test] +fn linkage_names_do_not_modify_name_section() { + let mut module = module_with_func_names(&[(0, "foo"), (2, "foo")]); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + // Name section is unmodified + assert_eq!( + module.name_section.func_names.get(&FuncIndex::new(0)), + Some(&Symbol::intern("foo")) + ); + assert_eq!( + module.name_section.func_names.get(&FuncIndex::new(2)), + Some(&Symbol::intern("foo")) + ); + + // Sanitized names are recorded in the linkage map + assert_eq!( + module.func_linkages.get(FuncIndex::new(0)).copied(), + Some(Symbol::intern("foo_func0")) + ); + assert_eq!( + module.func_linkages.get(FuncIndex::new(2)).copied(), + Some(Symbol::intern("foo_func2")) + ); +} + +/// Functions without a name-section entry (e.g. stripped binaries) use `func{index}` as both source +/// and linkage name. They must not be renamed to `func{index}_func{index}`. +#[test] +fn unnamed_functions_keep_fallback_name() { + let mut module = Module::default(); + let sig = SignatureIndex::from_u32(0); + module.push_function(sig); + module.push_function(sig); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "func0"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "func1"); + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "func0"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "func1"); + assert_eq!(module.func_name(FuncIndex::new(0)), module.source_func_name(FuncIndex::new(0))); + assert_eq!(module.func_name(FuncIndex::new(1)), module.source_func_name(FuncIndex::new(1))); +} + +#[test] +fn source_func_name_falls_back_to_the_synthesized_name() { + // No name-section entry for the function: both accessors fall back to `func{index}` + let module = module_with_func_names(&[]); + + assert_eq!(module.func_name(FuncIndex::new(3)).as_str(), "func3"); + assert_eq!(module.source_func_name(FuncIndex::new(3)).as_str(), "func3"); +} + +#[test] +fn duplicated_intrinsic_stub_name_is_an_error() { + let mut module = + module_with_func_names(&[(0, "intrinsics::felt::add"), (1, "intrinsics::felt::add")]); + + let err = module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap_err(); + + assert!( + err.to_string().contains("identifies an intrinsic or Miden ABI linker stub"), + "unexpected error: {err:?}" + ); +} + +#[test] +fn renamed_func_name_colliding_with_a_survivor_gets_trailing_underscore() { + let mut module = module_with_func_names(&[(0, "foo"), (1, "foo"), (2, "foo_func1")]); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo_func0"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "foo_func1_"); + assert_eq!(module.func_name(FuncIndex::new(2)).as_str(), "foo_func1"); + + // Deduplication affects the linkage name only; the source names are unchanged + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(2)).as_str(), "foo_func1"); +} + +#[test] +fn renamed_func_name_appends_underscores_until_free() { + let mut module = + module_with_func_names(&[(0, "foo"), (1, "foo"), (2, "foo_func1"), (3, "foo_func1_")]); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo_func0"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "foo_func1__"); + assert_eq!(module.func_name(FuncIndex::new(2)).as_str(), "foo_func1"); + assert_eq!(module.func_name(FuncIndex::new(3)).as_str(), "foo_func1_"); +} + +#[test] +fn renamed_func_name_colliding_with_a_global_gets_trailing_underscore() { + let mut module = module_with_func_names(&[(0, "foo"), (1, "foo")]); + let global_idx = module.globals.push(Global { + ty: WasmType::I32, + mutability: false, + }); + module + .name_section + .globals_names + .insert(global_idx, Symbol::intern("foo_func1")); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo_func0"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "foo_func1_"); + assert_eq!(module.global_name(global_idx).as_str(), "foo_func1"); + + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "foo"); +} + +#[test] +fn export_name_becomes_linkage_name_while_source_name_is_kept() { + let mut module = module_with_func_names(&[(0, "foo_src")]); + module + .exports + .insert("foo_ex".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo_ex"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo_src"); + assert_eq!( + module.func_linkages.get(FuncIndex::new(0)).copied(), + Some(Symbol::intern("foo_ex")) + ); +} + +#[test] +fn export_name_without_name_section_becomes_linkage_name() { + let mut module = module_with_func_names(&[]); + // No name-section entries, but the exported function must still exist in `Module::functions` + // for the dense maps to cover it. + module.push_function(SignatureIndex::from_u32(0)); + module + .exports + .insert("foo".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "func0"); + assert_eq!( + module.func_linkages.get(FuncIndex::new(0)).copied(), + Some(Symbol::intern("foo")) + ); +} + +#[test] +fn export_name_identical_to_source_name_records_no_linkage_override() { + let mut module = module_with_func_names(&[(0, "foo")]); + module + .exports + .insert("foo".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); +} + +#[test] +fn multiple_exports_for_single_function_is_an_error() { + let mut module = module_with_func_names(&[(0, "foo")]); + module + .exports + .insert("export_1".to_string(), EntityIndex::Function(FuncIndex::new(0))); + module + .exports + .insert("export_2".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + let err = module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap_err(); + + assert!( + err.to_string() + .contains("exporting a function under multiple names is not supported"), + "unexpected error: {err:?}" + ); +} + +#[test] +fn export_name_colliding_with_global_name_is_an_error() { + let mut module = module_with_func_names(&[(0, "my_func")]); + let global_idx = module.globals.push(Global { + ty: WasmType::I32, + mutability: false, + }); + module + .name_section + .globals_names + .insert(global_idx, Symbol::intern("colliding_name")); + module + .exports + .insert("colliding_name".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + let err = module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap_err(); + + assert!( + err.to_string().contains("conflicts with a global variable name"), + "unexpected error: {err:?}" + ); +} + +#[test] +fn export_name_identifying_intrinsic_is_an_error() { + let mut module = module_with_func_names(&[(0, "my_func")]); + module + .exports + .insert("intrinsics::felt::add".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + let err = module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap_err(); + + assert!( + err.to_string().contains("identifies an intrinsic or Miden ABI linker stub"), + "unexpected error: {err:?}" + ); +} + +#[test] +fn export_name_takes_precedence_over_unexported_source_name_collision() { + let mut module = module_with_func_names(&[(0, "foo_src"), (1, "bar")]); + module + .exports + .insert("bar".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "bar"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "bar_func1"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo_src"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "bar"); +} + +#[test] +fn export_name_colliding_with_fallback_renames_fallback() { + let mut module = module_with_func_names(&[(0, "foo_src")]); + // Second function without a name-section entry: its fallback would be `func1`. + module.push_function(SignatureIndex::from_u32(0)); + module + .exports + .insert("func1".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + // Export wins, the unnamed function's fallback is renamed + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "func1"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "func1_func1"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo_src"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "func1"); +} + +#[test] +fn duplicate_source_names_with_one_member_exported() { + let mut module = module_with_func_names(&[(0, "foo"), (1, "foo")]); + module + .exports + .insert("foo".to_string(), EntityIndex::Function(FuncIndex::new(0))); + + module.resolve_func_symbols(&DiagnosticsHandler::default()).unwrap(); + + assert_eq!(module.func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.func_name(FuncIndex::new(1)).as_str(), "foo_func1"); + assert_eq!(module.source_func_name(FuncIndex::new(0)).as_str(), "foo"); + assert_eq!(module.source_func_name(FuncIndex::new(1)).as_str(), "foo"); + + // Both functions share the raw name-section name "foo", so DWARF subprogram resolution + // considers both duplicate (requiring low_pc fallback) + assert!(module.is_duplicate_source_func_name(FuncIndex::new(0))); + assert!(module.is_duplicate_source_func_name(FuncIndex::new(1))); +} diff --git a/frontend/wasm/src/module/module_translation_state.rs b/frontend/wasm/src/module/module_translation_state.rs index 23ae21444a..7e43a66788 100644 --- a/frontend/wasm/src/module/module_translation_state.rs +++ b/frontend/wasm/src/module/module_translation_state.rs @@ -259,7 +259,7 @@ impl<'a> ModuleTranslationState<'a> { "unsupported function table element: '{}' is an intrinsic whose signature \ '{callee_signature}' differs from the Wasm signature '{signature}' its \ table entry is typed by", - module.func_name(func_index) + module.source_func_name(func_index) ); } self.module_builder.append_function_table_entry( @@ -275,7 +275,7 @@ impl<'a> ModuleTranslationState<'a> { diagnostics, "unsupported function table element: '{}' is an inlined intrinsic without a \ procedure body", - module.func_name(func_index) + module.source_func_name(func_index) ); } } diff --git a/tests/fixtures/components/assert-debug-test/Cargo.lock b/tests/fixtures/components/assert-debug-test/Cargo.lock index 4e0d6b16cd..eb9b729e83 100644 --- a/tests/fixtures/components/assert-debug-test/Cargo.lock +++ b/tests/fixtures/components/assert-debug-test/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/component-macros-account/Cargo.lock b/tests/fixtures/components/component-macros-account/Cargo.lock index 1f7cbcfa62..250563684f 100644 --- a/tests/fixtures/components/component-macros-account/Cargo.lock +++ b/tests/fixtures/components/component-macros-account/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/component-macros-note/Cargo.lock b/tests/fixtures/components/component-macros-note/Cargo.lock index 6df6e9f18c..2760770866 100644 --- a/tests/fixtures/components/component-macros-note/Cargo.lock +++ b/tests/fixtures/components/component-macros-note/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", @@ -1601,6 +1601,7 @@ dependencies = [ name = "midenc-frontend-wasm-metadata" version = "0.14.0-rc.1" dependencies = [ + "miden-mast-package", "serde", "serde_json", ] diff --git a/tests/fixtures/components/cross-ctx-account-word-arg/Cargo.lock b/tests/fixtures/components/cross-ctx-account-word-arg/Cargo.lock index 4a5c46135f..f95276cafc 100644 --- a/tests/fixtures/components/cross-ctx-account-word-arg/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-account-word-arg/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/cross-ctx-account-word/Cargo.lock b/tests/fixtures/components/cross-ctx-account-word/Cargo.lock index 892f782e6d..9f13b14158 100644 --- a/tests/fixtures/components/cross-ctx-account-word/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-account-word/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/cross-ctx-account/Cargo.lock b/tests/fixtures/components/cross-ctx-account/Cargo.lock index b90f596cc7..8e1bb0bce3 100644 --- a/tests/fixtures/components/cross-ctx-account/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-account/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/cross-ctx-note-word-arg/Cargo.lock b/tests/fixtures/components/cross-ctx-note-word-arg/Cargo.lock index a1a9557dae..305f80c8f6 100644 --- a/tests/fixtures/components/cross-ctx-note-word-arg/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-note-word-arg/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/cross-ctx-note-word/Cargo.lock b/tests/fixtures/components/cross-ctx-note-word/Cargo.lock index d4300213e2..f0a5834224 100644 --- a/tests/fixtures/components/cross-ctx-note-word/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-note-word/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/cross-ctx-note/Cargo.lock b/tests/fixtures/components/cross-ctx-note/Cargo.lock index aa1c2c6616..83cb9436c7 100644 --- a/tests/fixtures/components/cross-ctx-note/Cargo.lock +++ b/tests/fixtures/components/cross-ctx-note/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/fixtures/components/issue-invalid-stack-offset-movup/Cargo.lock b/tests/fixtures/components/issue-invalid-stack-offset-movup/Cargo.lock index f9087b1547..df3ab4322a 100644 --- a/tests/fixtures/components/issue-invalid-stack-offset-movup/Cargo.lock +++ b/tests/fixtures/components/issue-invalid-stack-offset-movup/Cargo.lock @@ -1209,9 +1209,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", @@ -1601,6 +1601,7 @@ dependencies = [ name = "midenc-frontend-wasm-metadata" version = "0.14.0-rc.1" dependencies = [ + "miden-mast-package", "serde", "serde_json", ] diff --git a/tests/fixtures/components/swapp-note/Cargo.lock b/tests/fixtures/components/swapp-note/Cargo.lock index 4f7e15686f..55b419ecbd 100644 --- a/tests/fixtures/components/swapp-note/Cargo.lock +++ b/tests/fixtures/components/swapp-note/Cargo.lock @@ -1202,9 +1202,9 @@ dependencies = [ [[package]] name = "miden-field" -version = "0.29.1" +version = "0.29.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41be9f7f5c0ef020bcedf526afe54b3a97244e207a5385e4453ca47799fe2afe" +checksum = "58cf9de55b88ec86ad272ff4224d76b3e0cbda49e242e78776b0037ba9b0e857" dependencies = [ "miden-serde-utils", "num-bigint 0.5.1", diff --git a/tests/lit/debug/duplicate-func-names-debug-info.shtest b/tests/lit/debug/duplicate-func-names-debug-info.shtest new file mode 100644 index 0000000000..a9527c4107 --- /dev/null +++ b/tests/lit/debug/duplicate-func-names-debug-info.shtest @@ -0,0 +1,25 @@ +# RUN: midenc %S/duplicate-func-names-debug-info.wasm --entrypoint=test --emit=hir=- -Canalyze-only 2>&1 | filecheck %s + +# Operates on a Wasm fixture that contains two functions whose name-section name is duplicated +# (`foo`) and real DWARF with two subprograms also named `foo`, distinguishable only through their +# `DW_AT_low_pc` +# +# The prebuilt fixture can be regenerated with: +# +# `cargo test -p midenc-frontend-wasm --lib -- --ignored write_duplicate_names_dwarf_fixture` + +# The first duplicate keeps its source name in the subprogram and records the unique name as the +# linkage name. Its parameter `a` was resolved via `low_pc` and verifies that the function was +# resolved correctly. The parameter of the other `foo` function is called `b`. +# CHECK-LABEL: builtin.function private extern("C") @foo_func0( +# CHECK: di.debug_declare <{ variable = #di.variable<{ name = "a", file = "unknown", line = 0, arg = 0, ty = i32 }>, expression = #di.expression<[DW_OP_WASM_local(0)]> }>; +# CHECK: di.subprogram = #di.subprogram<{ name = "foo", file = "unknown", line = 0, linkage = "foo_func0", definition = true, local = false }> + +# The second duplicate resolves its own parameter `b` +# CHECK-LABEL: builtin.function private extern("C") @foo_func1( +# CHECK: di.debug_declare <{ variable = #di.variable<{ name = "b", file = "unknown", line = 0, arg = 0, ty = i32 }>, expression = #di.expression<[DW_OP_WASM_local(0)]> }>; +# CHECK: di.subprogram = #di.subprogram<{ name = "foo", file = "unknown", line = 0, linkage = "foo_func1", definition = true, local = false }> + +# The function with unique name still resolves by name and carries no linkage name +# CHECK-LABEL: builtin.function public extern("C") @test( +# CHECK: di.subprogram = #di.subprogram<{ name = "test", file = "unknown", line = 0, definition = true, local = false }> diff --git a/tests/lit/debug/duplicate-func-names-debug-info.wasm b/tests/lit/debug/duplicate-func-names-debug-info.wasm new file mode 100644 index 0000000000000000000000000000000000000000..c41e2553169debce27ecbfcb24836e5c717be05b GIT binary patch literal 311 zcmXYsO-{rx427TLwDVITT2@GG5Q1$vLl>L?vCgEO6cuPYN>YASouhFf?m#k~ji3Bk z_Va^wJprJt*wIY$4m7jm01_tT=_bJHc>G1SDn#0wezZRV(i`U?wl|#O7I(G&Cwwr}n0vUR5`=WBC<-C>Y$1BgG<4_g7bhOQoFG z&AU2`-QbPI<3+h_>bkb(lM3QBAj%*`$iWGCEv^9>Nyvql?7R@Io&rcfG7hNizWroP aX&po`g$R%}_%G-QY$B)^uq5VY>F^Ku9Y5Lt literal 0 HcmV?d00001 diff --git a/tests/lit/debug/duplicate-func-names.wat b/tests/lit/debug/duplicate-func-names.wat new file mode 100644 index 0000000000..1f6c9a0b7c --- /dev/null +++ b/tests/lit/debug/duplicate-func-names.wat @@ -0,0 +1,41 @@ +;; RUN: midenc %s --entrypoint=test --emit=hir=- -Canalyze-only 2>&1 | filecheck %s +;; +;; This test verifies that function names duplicated in the Wasm name section are made unique. + +(module $duplicate_func_names_test.wasm + (type (;0;) (func (param i32) (result i32))) + (type (;1;) (func (result i32))) + (memory (;0;) 16) + (global $__stack_pointer (;0;) (mut i32) i32.const 1048576) + (export "memory" (memory 0)) + (export "test" (func $test)) + + ;; Both functions carry the same name-section name + (func $first (@name "foo") (;0;) (type 0) (param i32) (result i32) + local.get 0 + ) + (func $second (@name "foo") (;1;) (type 0) (param i32) (result i32) + local.get 0 + ) + (func $test (;2;) (type 1) (result i32) + i32.const 1 + call $first + i32.const 2 + call $second + i32.add + ) +) + +;; Both members of the duplicate group are renamed with their function index +;; CHECK: builtin.function private extern("C") @foo_func0( +;; The subprogram keeps the source name and records the unique name as the linkage name +;; CHECK: di.subprogram = #di.subprogram<{ name = "foo", file = "unknown", line = 0, linkage = "foo_func0", definition = true, local = false }> +;; CHECK: builtin.function private extern("C") @foo_func1( +;; CHECK: di.subprogram = #di.subprogram<{ name = "foo", file = "unknown", line = 0, linkage = "foo_func1", definition = true, local = false }> +;; The unique name is not modified, so its subprogram carries no linkage name +;; CHECK: builtin.function public extern("C") @test( + +;; Calls resolve to the renamed functions +;; CHECK: hir.exec {{.*}}::@foo_func0( +;; CHECK: hir.exec {{.*}}::@foo_func1( +;; CHECK: di.subprogram = #di.subprogram<{ name = "test", file = "unknown", line = 0, definition = true, local = false }> diff --git a/tests/lit/midenc/export-names.wat b/tests/lit/midenc/export-names.wat new file mode 100644 index 0000000000..54f5a06b75 --- /dev/null +++ b/tests/lit/midenc/export-names.wat @@ -0,0 +1,34 @@ +;; RUN: midenc %s --emit=hir=- -Canalyze-only 2>&1 | filecheck %s --check-prefix=HIR +;; RUN: midenc %s --entrypoint=export_name_test::foo_3 --emit=masm=- 2>&1 | filecheck %s --check-prefix=MASM +;; RUN: midenc %s --entrypoint=export_name_test::foo_3 -o %t/out.masp +;; +;; Verify that export names become linkage names. + +(module $export_name_test.wasm + ;; WAT identifier: $foo_1 + ;; Name-section name: "foo_2" + ;; Export name: "foo_3" + (func $foo_1 (@name "foo_2") (result i32) + i32.const 42 + ) + (export "foo_3" (func $foo_1)) + + ;; Internal caller to verify calls resolve to the exported linkage symbol + (func $caller (@name "caller_source") (result i32) + call $foo_1 + ) + (export "caller" (func $caller)) +) + +;; HIR: builtin.function public extern("C") @foo_3() -> i32 +;; HIR: di.subprogram = #di.subprogram<{ name = "foo_2", file = "unknown", line = 0, linkage = "foo_3", definition = true, local = false }> +;; HIR: builtin.function public extern("C") @caller() -> i32 +;; HIR: hir.exec {{.*}}::@foo_3() : extern("C") () -> i32 +;; HIR: di.subprogram = #di.subprogram<{ name = "caller_source", file = "unknown", line = 0, linkage = "caller", definition = true, local = false }> +;; HIR-NOT: builtin.function {{.*}} @foo_2 + +;; MASM: pub mod export_name_test +;; MASM: pub proc foo_3 +;; MASM: pub proc caller +;; MASM: exec.::"root_ns:root@1.0.0"::export_name_test::foo_3 +;; MASM-NOT: proc foo_2 diff --git a/tests/lit/midenc/lit.suite.toml b/tests/lit/midenc/lit.suite.toml index dff60e6036..a5093a2f4e 100644 --- a/tests/lit/midenc/lit.suite.toml +++ b/tests/lit/midenc/lit.suite.toml @@ -1,5 +1,5 @@ name = "midenc" -patterns = ["*.masm", "*.shtest", "*.rs"] +patterns = ["*.masm", "*.shtest", "*.rs", "*.wat"] [substitutions] "%target_dir" = "$$CARGO_TARGET_DIR"