fix(codegen): unforgeable generated-clone symbol namespace (#6927)

benchmarks/repsel_census/README.md

@@ -101,7 +101,7 @@ still worked. It does not any more, and the version above is the one to copy.
 
 Byte-identical objects mean the promotions the report counted as wins changed
 nothing. `07_object_create` and `12_binary_trees` are byte-identical today.
-`09_method_calls` differs, but only by two `__pshape` clones with **zero call
+`09_method_calls` differs, but only by two `$pshape` clones with **zero call
 sites** — which is why the census reports its consumption as 0 and the object
 A/B alone would have been misleading.
 

changelog.d/7441-unforgeable-clone-suffixes.md

@@ -0,0 +1,20 @@
+Fixed #6927: user members whose names spell a generated clone suffix
+(`foo__generic`, `add__typed_f64`, `tick__pshape`, `A__dup1`, …) no longer
+collide with the compiler's clone symbols. The failure had silently worsened
+since the issue was filed: `deduped_function_refs` (first-define-wins, added
+for minified same-name classes) swallowed the duplicate definition, so the
+user function's public entry was usurped by its sibling's clone and indirect
+calls executed the wrong body (`const g = add__typed_f64; g(2, 3)` returned
+`add(2, 3)`). Generated suffixes now use a reserved `$` separator
+(`{public}$generic`, `$typed_*`, `$spec_<reps>`, `$pshape`, `$dupN`);
+`sanitize`/`sanitize_member` output is strictly `[A-Za-z0-9_]`, so no
+user-derived symbol can compose a clone symbol, by construction, for every
+current and future clone kind — including the cross-component shapes (class
+`C__foo` + method `pshape`) that member-name checks cannot see. The #6925
+`__pshape` collision prune is dead under this invariant and was reduced to
+its registry-presence filter (`prune_unregistered_clones`); both symbol
+reachability ratchets track the new spellings with unchanged allowlists.
+Regression coverage: `codegen/clone_suffix_tests.rs` (emitted-IR contract,
+per-PR visible), `helpers::sanitize_tests` (no-`$` mangling invariant), and
+gap test `test_gap_6927_clone_suffix_user_members.ts` (direct + indirect
+calls across the family, byte-identical to Node).

crates/perry-codegen/src/codegen/clone_suffix_tests.rs

@@ -0,0 +1,183 @@
+//! Issue #6927 — generated clone symbols must be unforgeable by user names.
+//!
+//! Clone/body symbols used to be `{public}__<suffix>` (`__generic`,
+//! `__typed_f64`, …). A user function literally named `add__typed_f64`
+//! composed the SAME LLVM symbol as `add`'s typed clone, and
+//! `deduped_function_refs` (first-define-wins, added for minified same-name
+//! classes) silently dropped one of the two definitions: the user function's
+//! public entry was usurped by `add`'s clone, so every indirect call through
+//! its registered wrapper executed `add`'s body instead — a SILENT wrong
+//! result, not a loud verifier error. (Witnessed on v0.5.1280:
+//! `const g = add__typed_f64; g(2, 3)` returned 5 instead of 6.)
+//!
+//! The fix reserves `$` as the generated-suffix separator (`{public}$generic`,
+//! `{public}$typed_f64`, `{public}$dupN`, the spec-ABI and proven-`this`
+//! suffixes): `sanitize`/`sanitize_member` output is strictly `[A-Za-z0-9_]`,
+//! so no user-derived public symbol can ever equal a generated one. These
+//! tests pin the emitted-IR side of that contract; the mangling side is pinned
+//! by `helpers::sanitize_tests`.
+
+use crate::{compile_module, CompileOptions};
+use perry_hir::types::Type;
+use perry_hir::{BinaryOp, Expr, Function, Module, ModuleInitKind, Param, Stmt};
+
+fn ir_opts() -> CompileOptions {
+    CompileOptions {
+        emit_ir_only: true,
+        output_type: "executable".to_string(),
+        ..CompileOptions::default()
+    }
+}
+
+fn number_param(id: u32, name: &str) -> Param {
+    Param {
+        id,
+        name: name.to_string(),
+        ty: Type::Number,
+        default: None,
+        decorators: Vec::new(),
+        is_rest: false,
+        arguments_object: None,
+    }
+}
+
+/// A typed-ABI clone candidate: straight-line `return a <op> b` over two
+/// `number` params, so codegen emits the public trampoline plus `$typed_f64`
+/// and `$generic` bodies.
+fn candidate_fn(id: u32, name: &str, op: BinaryOp) -> Function {
+    Function {
+        id,
+        name: name.to_string(),
+        type_params: Vec::new(),
+        params: vec![number_param(10, "a"), number_param(11, "b")],
+        return_type: Type::Number,
+        body: vec![Stmt::Return(Some(Expr::Binary {
+            op,
+            left: Box::new(Expr::LocalGet(10)),
+            right: Box::new(Expr::LocalGet(11)),
+        }))],
+        is_async: false,
+        is_generator: false,
+        is_strict: true,
+        was_plain_async: false,
+        was_unrolled: false,
+        is_exported: false,
+        captures: Vec::new(),
+        decorators: Vec::new(),
+    }
+}
+
+fn module_with(functions: Vec<Function>) -> Module {
+    Module {
+        name: "clone_suffix.ts".to_string(),
+        imports: Vec::new(),
+        exports: Vec::new(),
+        classes: Vec::new(),
+        interfaces: Vec::new(),
+        type_aliases: Vec::new(),
+        enums: Vec::new(),
+        globals: Vec::new(),
+        functions,
+        script_global_functions: Vec::new(),
+        references_global_this: false,
+        annexb_global_undefined_names: Vec::new(),
+        init: Vec::new(),
+        exported_native_instances: Vec::new(),
+        exported_func_return_native_instances: Vec::new(),
+        exported_objects: Vec::new(),
+        exported_functions: Vec::new(),
+        widgets: Vec::new(),
+        uses_fetch: false,
+        uses_webassembly: false,
+        extern_funcs: Vec::new(),
+        init_was_unrolled: false,
+        has_top_level_await: false,
+        init_kind: ModuleInitKind::Eager,
+        async_step_closures: std::collections::HashSet::new(),
+        closure_display_names: std::collections::HashMap::new(),
+        class_display_names: std::collections::HashMap::new(),
+        closure_source_text: std::collections::HashMap::new(),
+        async_generator_funcs: std::collections::HashSet::new(),
+        gen_param_prologue_len: std::collections::HashMap::new(),
+    }
+}
+
+/// `define` lines whose declared symbol is exactly `name`.
+fn define_count(ir: &str, name: &str) -> usize {
+    let needle = format!("@{name}(");
+    ir.lines()
+        .filter(|l| l.starts_with("define") && l.contains(&needle))
+        .count()
+}
+
+/// Slice out the body of the `define`d function whose signature line declares
+/// exactly `name`.
+fn function_body<'a>(ir: &'a str, name: &str) -> &'a str {
+    let needle = format!("@{name}(");
+    let start = ir
+        .match_indices("define")
+        .find(|(i, _)| {
+            let line_end = ir[*i..].find('\n').map(|n| i + n).unwrap_or(ir.len());
+            ir[*i..line_end].contains(&needle)
+        })
+        .map(|(i, _)| i)
+        .unwrap_or_else(|| panic!("no define for @{name} in module IR"));
+    let end = ir[start..].find("\n}").expect("unterminated function") + start;
+    &ir[start..end]
+}
+
+/// The #6927 witness family: `add` plus user functions whose names are the
+/// OLD (forgeable) spellings of `add`'s clone symbols. Every public entry and
+/// every clone must be a distinct symbol, each defined exactly once, with each
+/// body reached from its own trampoline.
+#[test]
+fn user_members_named_like_clone_suffixes_keep_their_own_symbols() {
+    let ir = String::from_utf8(
+        compile_module(
+            &module_with(vec![
+                candidate_fn(1, "add", BinaryOp::Add),
+                candidate_fn(2, "add__typed_f64", BinaryOp::Mul),
+                candidate_fn(3, "add__generic", BinaryOp::Sub),
+            ]),
+            ir_opts(),
+        )
+        .unwrap(),
+    )
+    .expect("LLVM IR should be UTF-8");
+
+    // Publics and clones are all distinct symbols, each defined exactly once.
+    // Pre-fix, `add`'s clone was literally `perry_fn_clone_suffix_ts__add__typed_f64`
+    // — the user function's public symbol — and first-define-wins dedup
+    // silently dropped the user function's own entry.
+    for name in [
+        "perry_fn_clone_suffix_ts__add",
+        "perry_fn_clone_suffix_ts__add__typed_f64",
+        "perry_fn_clone_suffix_ts__add__generic",
+        "perry_fn_clone_suffix_ts__add$typed_f64",
+        "perry_fn_clone_suffix_ts__add$generic",
+        "perry_fn_clone_suffix_ts__add__typed_f64$typed_f64",
+        "perry_fn_clone_suffix_ts__add__typed_f64$generic",
+        "perry_fn_clone_suffix_ts__add__generic$typed_f64",
+        "perry_fn_clone_suffix_ts__add__generic$generic",
+    ] {
+        assert_eq!(
+            define_count(&ir, name),
+            1,
+            "@{name} must be defined exactly once"
+        );
+    }
+
+    // Each trampoline routes to ITS OWN clones — `add`'s fast arm computes
+    // a + b, the user `add__typed_f64`'s computes a * b.
+    let add_public = function_body(&ir, "perry_fn_clone_suffix_ts__add");
+    assert!(add_public.contains("@perry_fn_clone_suffix_ts__add$typed_f64("));
+    assert!(add_public.contains("@perry_fn_clone_suffix_ts__add$generic("));
+    let user_public = function_body(&ir, "perry_fn_clone_suffix_ts__add__typed_f64");
+    assert!(user_public.contains("@perry_fn_clone_suffix_ts__add__typed_f64$typed_f64("));
+    assert!(user_public.contains("@perry_fn_clone_suffix_ts__add__typed_f64$generic("));
+
+    let add_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add$typed_f64");
+    assert!(add_clone.contains("fadd"), "add's clone body is a + b");
+    let user_clone = function_body(&ir, "perry_fn_clone_suffix_ts__add__typed_f64$typed_f64");
+    assert!(user_clone.contains("fmul"), "add__typed_f64's clone body is a * b");
+}

crates/perry-codegen/src/codegen/closure.rs

@@ -1318,7 +1318,7 @@ mod tests {
         let _shadow = crate::codegen::helpers::NativeRootsPin::shadow();
         let ir = one_capture_closure_ir();
         // The public `perry_closure_*` symbol can be a typed trampoline over a
-        // straight-line `__typed_f64` clone; the real body is the one that
+        // straight-line `$typed_f64` clone; the real body is the one that
         // carries a shadow frame. (The typed clone lowers arithmetic-only,
         // loop-free, call-free statements — `lower_typed_f64_body_*` bails on
         // anything else — so it contains no safepoint and its `%this_closure`

crates/perry-codegen/src/codegen/func_registry.rs

@@ -24,8 +24,11 @@ pub(crate) struct FuncRegistry {
 
 /// Resolve user function names + signatures up front. Names are scoped by
 /// module prefix; distinct functions that mangle to the same symbol get a
-/// numeric `__dupN` suffix (exported functions reserve their canonical name
-/// first and never get suffixed).
+/// numeric `$dupN` suffix (exported functions reserve their canonical name
+/// first and never get suffixed). The `$` separator keeps the uniquifier in
+/// the reserved generated-suffix namespace (issue #6927): `sanitize` output
+/// is `[A-Za-z0-9_]`-only, so a user function literally named `A__dup1`
+/// cannot collide with the disambiguated symbol of a duplicate `A`.
 pub(crate) fn build_func_registry(hir: &HirModule, module_prefix: &str) -> FuncRegistry {
     let mut func_names: HashMap<u32, String> = HashMap::new();
     let mut func_signatures: HashMap<u32, (usize, bool, bool, bool)> = HashMap::new();
@@ -34,8 +37,8 @@ pub(crate) fn build_func_registry(hir: &HirModule, module_prefix: &str) -> FuncR
     // Distinct functions can mangle to the same symbol: minified code reuses
     // short names (`function A`) across scopes, and perry lambda-lifts nested
     // functions to module level, so two module functions can share a name — clang
-    // then rejects the duplicate `define perry_fn_<mod>__A`. Disambiguate with a
-    // numeric suffix, keyed by the mangled symbol. Exported functions are
+    // then rejects the duplicate `define perry_fn_<mod>__A`. Disambiguate with
+    // a numeric suffix, keyed by the mangled symbol. Exported functions are
     // referenced cross-module by their canonical `scoped_fn_name` and are unique
     // per module, so they reserve that name first and never get suffixed.
     let mut used_fn_symbols: HashMap<String, u32> = HashMap::new();
@@ -56,7 +59,7 @@ pub(crate) fn build_func_registry(hir: &HirModule, module_prefix: &str) -> FuncR
             let s = if *n == 0 {
                 base.clone()
             } else {
-                format!("{base}__dup{n}")
+                format!("{base}$dup{n}")
             };
             *n += 1;
             s

crates/perry-codegen/src/codegen/helpers.rs

@@ -666,6 +666,13 @@ pub(super) fn scoped_method_name(
 /// a digit, so prefix with `_` if the first character would be one (this
 /// happens with module names like `05_fibonacci.ts`).
 ///
+/// The output alphabet being strictly `[A-Za-z0-9_]` is a load-bearing
+/// invariant (issue #6927): `$` is reserved for compiler-generated clone /
+/// uniquifier suffixes (`$generic`, `$typed_*`, `$dupN`, the spec-ABI and
+/// proven-`this` suffixes), so a user-derived symbol component can never
+/// forge a generated symbol. Never emit `$` from this function or from
+/// [`sanitize_member`].
+///
 /// NOTE: this mapping is *lossy* — every special character collapses to `_`,
 /// so distinct inputs can share an output. That is fine for the module-prefix
 /// and static-field components (whose values are recorded once and re-derived
@@ -713,6 +720,9 @@ pub(super) fn sanitize(name: &str) -> String {
 ///
 /// Must be applied at BOTH the definition site and every reference site for a
 /// given symbol component, or the symbols desync and the linker fails.
+///
+/// Like [`sanitize`], the output is strictly `[A-Za-z0-9_]` — `$` is reserved
+/// for generated suffixes and must never appear here (issue #6927).
 pub(super) fn sanitize_member(name: &str) -> String {
     let is_plain = name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_');
     if is_plain {
@@ -1543,6 +1553,42 @@ pub(super) fn emit_namespace_populator(
     blk.call_void("js_gc_register_global_root", &[(I64, &addr_i64)]);
 }
 
+#[cfg(test)]
+mod sanitize_tests {
+    use super::{sanitize, sanitize_member, scoped_fn_name, scoped_method_name};
+
+    /// Issue #6927: the generated-clone namespace (`{public}$<suffix>`) is
+    /// unforgeable ONLY because these two functions never emit `$`. If either
+    /// ever lets a `$` through, a user member could compose a public symbol
+    /// equal to a generated clone symbol and silently usurp it
+    /// (`deduped_function_refs` keeps the first definition).
+    #[test]
+    fn sanitize_never_emits_the_reserved_generated_suffix_separator() {
+        for hostile in [
+            "foo$generic",
+            "$dup1",
+            "a$b$c",
+            "foo__generic", // old forgeable spelling — plain, passes through, harmless now
+            "#$",
+            "℘$typed_f64",
+        ] {
+            assert!(
+                !sanitize(hostile).contains('$'),
+                "sanitize({hostile:?}) leaked a `$`: {:?}",
+                sanitize(hostile)
+            );
+            assert!(
+                !sanitize_member(hostile).contains('$'),
+                "sanitize_member({hostile:?}) leaked a `$`: {:?}",
+                sanitize_member(hostile)
+            );
+        }
+        // And therefore no composed public symbol contains one either.
+        assert!(!scoped_fn_name("m", "add$typed_f64").contains('$'));
+        assert!(!scoped_method_name("m", "C$x", "foo$generic").contains('$'));
+    }
+}
+
 #[cfg(test)]
 mod resolve_target_triple_tests {
     use super::resolve_target_triple;

crates/perry-codegen/src/codegen/mod.rs

@@ -54,6 +54,8 @@ mod method;
 mod method_registry;
 mod module_globals_emit;
 #[cfg(test)]
+mod clone_suffix_tests;
+#[cfg(test)]
 mod number_exactness_tests;
 mod opts;
 mod spec_abi;
@@ -1916,13 +1918,13 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
     );
 
     // Representation-selection Phase 5a: now that the method registry exists,
-    // drop any proven-`this` clone whose composed symbol would collide with a
-    // symbol a real user member already owns (issue #6927 tracks the
-    // family-wide fix for every generated-clone suffix). Pruning HERE — before
-    // `emit_module_artifacts` reads `cross_module.pshape_methods` for both
-    // emission and call-site routing — keeps the two in lockstep, so a call
-    // site can never route to a clone the emission loop declined to produce.
-    crate::collectors::prune_colliding_clones(&mut cross_module.pshape_methods, &method_names);
+    // drop any proven-`this` clone whose pair never made it into it. (Symbol
+    // collisions with user members are impossible since #6927's reserved-`$`
+    // clone namespace, so registry presence is all that is checked.) Pruning
+    // HERE — before `emit_module_artifacts` reads `cross_module.pshape_methods`
+    // for both emission and call-site routing — keeps the two in lockstep, so
+    // a call site can never route to a clone the emission loop cannot produce.
+    crate::collectors::prune_unregistered_clones(&mut cross_module.pshape_methods, &method_names);
 
     // Resolve user function names + signatures up front. See
     // `func_registry::build_func_registry`.
@@ -1975,7 +1977,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
     // `closure.rs` filters module globals OUT of `closure_captures`, so the
     // closure is `alloc_singleton` with no capture slots. But advertising the
     // local's type to the typed-ABI closure specialization
-    // (`__typed_f64`/i32/…) made it read `js_closure_get_capture_bits(this,
+    // (`$typed_f64`/i32/…) made it read `js_closure_get_capture_bits(this,
     // 0)` — an UNSET slot (0) — while the generic variant correctly loads the
     // global; the dispatcher picked the typed body, so every closure returned
     // 0. Repro (bisected to #5466 representation lowering):
@@ -2337,7 +2339,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
     }
 
     // Representation-selection Phase 2: emit full-body specialized entries
-    // (`{public}__spec_...`, internal linkage) before the public bodies. Same
+    // (`{public}$spec_...`, internal linkage) before the public bodies. Same
     // real `compile_function`, parameterized on the plan's rep tuple.
     for f in &hir.functions {
         let Some(plan) = cross_module.spec_abi_functions.get(&f.id).cloned() else {

crates/perry-codegen/src/codegen/number_exactness_tests.rs

@@ -309,7 +309,7 @@ fn straight_line_number_function_takes_the_typed_f64_clone() {
     let ir = emitted_ir(vec![add_fn()]);
     assert!(!ir.contains("add_i64"), "{NO_I64_BODY}:\n{ir}");
     assert!(
-        ir.contains("__typed_f64"),
+        ir.contains("$typed_f64"),
         "the typed-f64 clone must now be reachable for a plain `a + b`:\n{ir}"
     );
 }