diff --git a/frontend/wasm/src/module/mod.rs b/frontend/wasm/src/module/mod.rs index 7b97e6723f..a935c817fc 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; @@ -323,6 +326,77 @@ impl Module { .unwrap_or(Symbol::intern(format!("func{}", index.as_u32()))) } + /// Ensures each function in the module has a unique name. + /// + /// WebAssembly function names in the [name section] are not guaranteed to be unique. This + /// method rewrites duplicate names so that every function has a unique name. + /// + /// Intrinsics and Miden ABI linker stubs are recognized by their function name (see + /// [`maybe_lower_linker_stub`]), so a duplicated name that identifies a known stub is an error + /// rather than a silent rename that would break that recognition. + /// + /// The rename target `{name}_func{index}` is unique among renamed functions (the index suffix + /// differs per function), but can collide with a name that survives unchanged or with another + /// module-level symbol; in that case `_` is appended until the name is free. + /// + /// [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 sanitize_duplicate_func_names( + &mut self, + diagnostics: &DiagnosticsHandler, + ) -> WasmResult<()> { + let mut counts: FxHashMap = FxHashMap::default(); + for name in self.name_section.func_names.values() { + *counts.entry(*name).or_default() += 1; + } + let mut duplicates = FxHashSet::default(); + let mut survivors = FxHashSet::default(); + for (name, count) in counts { + if count > 1 { + duplicates.insert(name); + } else { + survivors.insert(name); + } + } + if duplicates.is_empty() { + return Ok(()); + } + + // Symbol names a renamed function may not take: survivor function names plus the names + // of the module's global variables (same symbol table as functions). + let mut taken: FxHashSet = survivors; + for index in self.globals.keys() { + taken.insert(self.global_name(index)); + } + + for (index, name) in self.name_section.func_names.iter_mut() { + if !duplicates.contains(name) { + continue; + } + let name_str = name.as_str(); + if let Ok(func_id) = FunctionIdent::from_str(name_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 '{name_str}' identifies an intrinsic or Miden \ + ABI linker stub, which midenc recognizes by name, so it cannot renamed" + ); + } + } + // The index suffix makes renames of distinct functions mutually unique, but the result + // can still collide with a name in `taken`. Append `_` until the name is free. + let mut candidate = format!("{name_str}_func{}", index.as_u32()); + while taken.contains(&Symbol::intern(candidate.as_str())) { + candidate.push('_'); + } + let unique = Symbol::intern(candidate); + taken.insert(unique); + *name = unique; + } + Ok(()) + } + /// Returns the name of the given data segment. /// /// If the wasm name section does not include an entry for this segment diff --git a/frontend/wasm/src/module/module_env.rs b/frontend/wasm/src/module/module_env.rs index 55346e4e30..7b02d1b4a6 100644 --- a/frontend/wasm/src/module/module_env.rs +++ b/frontend/wasm/src/module/module_env.rs @@ -352,6 +352,7 @@ impl<'a, 'data> ModuleEnvironment<'a, 'data> { for payload in parser.parse_all(data) { self.parse_payload(payload.into_diagnostic()?, diagnostics)?; } + self.result.module.sanitize_duplicate_func_names(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..6864645e84 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,94 @@ 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(); + 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_by_index() { + let mut module = module_with_func_names(&[(0, "foo"), (2, "foo"), (1, "bar")]); + + module.sanitize_duplicate_func_names(&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"); +} + +#[test] +fn unique_func_names_are_kept() { + let mut module = module_with_func_names(&[(0, "foo"), (1, "bar")]); + + module.sanitize_duplicate_func_names(&DiagnosticsHandler::default()).unwrap(); + module.sanitize_duplicate_func_names(&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"); +} + +#[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 + .sanitize_duplicate_func_names(&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.sanitize_duplicate_func_names(&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"); +} + +#[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.sanitize_duplicate_func_names(&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.sanitize_duplicate_func_names(&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"); +} diff --git a/tests/lit/debug/duplicate-func-names.wat b/tests/lit/debug/duplicate-func-names.wat new file mode 100644 index 0000000000..57a875f81d --- /dev/null +++ b/tests/lit/debug/duplicate-func-names.wat @@ -0,0 +1,37 @@ +;; 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( +;; CHECK: builtin.function private extern("C") @foo_func1( +;; The unique name is left untouched +;; CHECK: builtin.function public extern("C") @test( + +;; Calls resolve to the renamed functions +;; CHECK: hir.exec {{.*}}::@foo_func0( +;; CHECK: hir.exec {{.*}}::@foo_func1(