fix: sanitize Wasm function names

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<Symbol, usize> = 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<Symbol> = 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

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)
     }
 

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");
+}

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(