From d800198f515fdab2d85b42a6412748f8eac75d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 08:38:42 +0200 Subject: [PATCH 1/4] fix(codegen): make emitted IR run-to-run deterministic (#7622) Compiling one source twice with the same perry binary produced different LLVM IR. Two emission sites read their order straight out of a std::collections hash map, whose RandomState is seeded per process: * codegen/artifacts.rs - the `hir.closure_display_names` walk. Each entry mints a rodata constant via `add_string_constant` (whose `@.str.N` counter numbers in first-use order) and emits one `js_register_function_name` call, so both permuted every run. #7038 fixed the identical defect in the `closure_source_text` loop directly below it and left this one standing. * lower_call/property_get/dynamic_dispatch.rs - the `ctx.class_ids` walk that builds the interface dispatch tower. Each surviving entry is one icmp-guarded case block, so the map order WAS the arm order: the same three call sites named different `perry_method_*` callees run to run. Both reproduce 5/5 on the issue's own test files; both are byte-stable after the fix, and a 41-program `test_gap_*` sweep is 0/41 nondeterministic. Two mechanical siblings are sorted without a fixture and labelled as such in the test module: the virtual-override tower over the same map (emits zero arms across all 41 sampled programs), and method_registry.rs's `class_table` walk, whose insert/or_insert_with tie-breaks only diverge when two `&Class` contend for one registry key. The `max_explicit_arity` scan there also stops taking the first name that carries a class id - ids are not unique over `class_ids` keys - and takes the max instead, which is what the variable already means. --- crates/perry-codegen/src/codegen/artifacts.rs | 33 +- .../src/codegen/emission_order_tests.rs | 423 ++++++++++++++++++ .../src/codegen/method_registry.rs | 13 +- crates/perry-codegen/src/codegen/mod.rs | 2 + .../property_get/dynamic_dispatch.rs | 55 ++- 5 files changed, 504 insertions(+), 22 deletions(-) create mode 100644 crates/perry-codegen/src/codegen/emission_order_tests.rs diff --git a/crates/perry-codegen/src/codegen/artifacts.rs b/crates/perry-codegen/src/codegen/artifacts.rs index d5fe6bf440..26df4b38f6 100644 --- a/crates/perry-codegen/src/codegen/artifacts.rs +++ b/crates/perry-codegen/src/codegen/artifacts.rs @@ -1833,16 +1833,29 @@ pub(super) fn emit_module_artifacts(c: ModuleArtifactsCtx<'_>) -> Result<()> { // "use of undefined value" (regression class of #318/#343). let materialized_closure_ids: std::collections::HashSet = closures.iter().map(|(id, _)| *id).collect(); - for (func_id, display) in &hir.closure_display_names { - if !materialized_closure_ids.contains(func_id) { - continue; - } - if display.is_empty() || named_inline_closure_ids.contains(func_id) { - continue; - } - if registered_fn_ids.contains(func_id) { - continue; - } + // Sorted, NOT raw `HashMap` iteration (#7622) — the same defect #7038 fixed + // one loop down for `closure_source_text`, left standing here. Every entry + // mints a rodata constant through `add_string_constant`, whose `@.str.N` + // counter numbers in first-use order, and emits one + // `js_register_function_name` call in `__perry_init_strings_*`. Iterating + // the map directly made both a per-process permutation, so the same source + // compiled by the same binary produced different `.ll` on every run — which + // silently invalidates any A/B that compares raw IR (the primary evidence + // the #7615 rooting slices offer). Emission order is the only thing that + // changes; sorting by `FuncId` makes it stable without altering what is + // emitted. + let mut materialized_closure_display: Vec<(&perry_hir::types::FuncId, &String)> = hir + .closure_display_names + .iter() + .filter(|(func_id, display)| { + materialized_closure_ids.contains(*func_id) + && !display.is_empty() + && !named_inline_closure_ids.contains(*func_id) + && !registered_fn_ids.contains(*func_id) + }) + .collect(); + materialized_closure_display.sort_by_key(|(func_id, _)| **func_id); + for (func_id, display) in materialized_closure_display { let sym = format!("perry_closure_{}__{}", module_prefix, func_id); user_fn_display_names.push((sym, display.clone())); } diff --git a/crates/perry-codegen/src/codegen/emission_order_tests.rs b/crates/perry-codegen/src/codegen/emission_order_tests.rs new file mode 100644 index 0000000000..9cce4b581d --- /dev/null +++ b/crates/perry-codegen/src/codegen/emission_order_tests.rs @@ -0,0 +1,423 @@ +//! #7622 — the same HIR module must emit byte-identical LLVM IR every time. +//! +//! Two emission sites read their order straight out of a `std::collections` +//! hash map. Rust's default hasher is seeded per `RandomState`, so the order +//! was a fresh permutation on every compile: the same source, compiled twice by +//! the same `perry` binary, produced different `.ll`. +//! +//! * **Function-name registration** (`codegen/artifacts.rs`). Every inline +//! closure carrying a HIR display name mints a rodata constant through +//! `add_string_constant` — whose `@.str.N` counter numbers in first-use order +//! — and emits one `js_register_function_name` call into +//! `__perry_init_strings_*`. Iterating `hir.closure_display_names` permuted +//! both. (#7038 fixed the identical defect in the `closure_source_text` loop +//! directly below it and left this one standing.) +//! * **The dynamic method-dispatch tower** +//! (`lower_call/property_get/dynamic_dispatch.rs`). Every class implementing +//! the called property becomes one `icmp`-guarded case block; iterating +//! `ctx.class_ids` permuted the arms, so consecutive call sites named +//! different `perry_method_*` callees run to run. +//! +//! That is not cosmetic. It defeats the byte-level IR A/B that the #7615 +//! rooting-migration slices offer as their correctness evidence (three of nine +//! apparent diffs in #7620 were this, not the change), and the `.perry-cache` +//! object cache keys on a DETERMINISTIC fingerprint — so a cache hit and a +//! rebuild can legitimately hold different bytes for the same inputs. +//! +//! ## Why these tests are not vacuous +//! +//! Each case builds its `Module` FRESH for every compile. That matters: the +//! offending maps live in the HIR, so compiling one long-lived `Module` twice +//! would iterate the very same `RandomState` twice and pass no matter what. +//! `N = 16` entries makes a chance-ordered agreement a 1-in-16! event. +//! +//! Each case also asserts its subject was LIVE before it judges order — the +//! registration count, and the tower arm count. A fixture that stopped emitting +//! the construct under test would otherwise go green having proven nothing. +//! +//! Sabotage-verified: reverting either sort in isolation turns exactly that +//! shape's `…_are_emitted_in_…_order` and `…_is_run_to_run_deterministic` +//! tests red, and leaves the other shape's green. +//! +//! ## What is NOT covered here, and why +//! +//! The same commit sorts two further hash-order reads that this file does not +//! test, because no fixture was found that makes them emit anything: +//! +//! * the **virtual-override tower** (`vdispatch.*`, the sibling of the tower +//! above, in the same file, over the same map). `vdispatch` blocks appear in +//! ZERO of 41 sampled `test_gap_*` programs — every receiver-typed call +//! measured is claimed first by `method_override.rs`'s `method_direct` shape +//! guard. A green test over a fixture that emits no arms would assert +//! nothing, which is worse than no test, so there is none. +//! * `method_registry.rs`'s `class_table` walk, whose `insert` (last wins) and +//! `entry().or_insert_with` (first wins) tie-breaks only diverge when two +//! distinct `&Class` contend for one registry key. +//! +//! Both are mechanical siblings of the defects that ARE covered — and #7622 +//! exists precisely because #7038 fixed one such loop and left its neighbour — +//! so they are sorted, and labelled untested rather than left implied. + +use crate::{compile_module, AppMetadata, CompileOptions}; +use perry_hir::types::Type; +use perry_hir::{Class, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +/// Enough entries that an accidentally-sorted hash order is not a plausible +/// explanation for a green run. +const N: u32 = 16; + +fn ir_opts() -> CompileOptions { + CompileOptions { + target: None, + is_entry_module: false, + non_entry_module_prefixes: Vec::new(), + nextjs_path_init_modules: Vec::new(), + import_function_prefixes: std::collections::HashMap::new(), + import_function_ffi_aliases: std::collections::HashMap::new(), + import_function_origin_names: std::collections::HashMap::new(), + import_function_v8_specifiers: std::collections::HashMap::new(), + import_function_node_submodule: std::collections::HashMap::new(), + namespace_node_submodules: std::collections::HashMap::new(), + namespace_v8_specifiers: std::collections::HashMap::new(), + namespace_member_prefixes: std::collections::HashMap::new(), + namespace_member_origin_names: std::collections::HashMap::new(), + emit_ir_only: true, + verify_native_regions: false, + disable_buffer_fast_path: false, + namespace_imports: Vec::new(), + imported_classes: Vec::new(), + imported_enums: Vec::new(), + imported_async_funcs: std::collections::HashSet::new(), + type_aliases: std::collections::HashMap::new(), + imported_func_param_counts: std::collections::HashMap::new(), + imported_func_has_rest: std::collections::HashSet::new(), + imported_func_synthetic_arguments: std::collections::HashSet::new(), + imported_func_return_types: std::collections::HashMap::new(), + imported_vars: std::collections::HashSet::new(), + output_type: "executable".to_string(), + needs_stdlib: false, + needs_ui: false, + needs_geisterhand: false, + geisterhand_port: 7676, + enabled_features: Vec::new(), + native_module_init_names: Vec::new(), + js_module_specifiers: Vec::new(), + bundled_extensions: Vec::new(), + native_library_functions: Vec::new(), + i18n_table: None, + fast_math: false, + fp_contract_mode: crate::FpContractMode::Off, + app_metadata: AppMetadata::default(), + namespace_entries: Vec::new(), + dynamic_import_path_to_prefix: std::collections::HashMap::new(), + deferred_module_prefixes: std::collections::HashSet::new(), + module_init_deps: Vec::new(), + is_dynamic_import_target: false, + debug_locations: false, + module_source: None, + debug_source_line_offset: 0, + } +} + +fn empty_module(name: &str) -> Module { + Module { + name: name.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: Vec::new(), + 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(), + } +} + +fn ir(module: &Module) -> String { + String::from_utf8(compile_module(module, ir_opts()).expect("codegen should succeed")) + .expect("LLVM IR should be UTF-8") +} + +// --------------------------------------------------------------------------- +// Shape 1: `js_register_function_name` / `@.str.N` +// --------------------------------------------------------------------------- + +/// `let _fK = () => {}` for `K` in `0..N`, each carrying a HIR display name. +/// +/// The `_` prefix is load-bearing: the top-level `let`-bound arm of the +/// display-name collection skips underscore names outright, so every entry +/// falls through to the arm that reads `hir.closure_display_names` — the one +/// under test. +fn closure_display_module() -> Module { + let mut m = empty_module("emission_order_names.ts"); + for k in 0..N { + let func_id = 100 + k; + m.init.push(Stmt::Let { + id: 900 + k, + name: format!("_f{:02}", k), + ty: Type::Any, + mutable: false, + init: Some(Expr::Closure { + func_id, + params: Vec::new(), + return_type: Type::Void, + body: Vec::new(), + captures: Vec::new(), + mutable_captures: Vec::new(), + captures_this: false, + captures_new_target: false, + enclosing_class: None, + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: true, + }), + }); + m.closure_display_names + .insert(func_id, format!("name{:02}", k)); + } + m +} + +/// The `func_id` of every `js_register_function_name` call, in emission order. +fn registered_closure_ids(ir: &str) -> Vec { + ir.lines() + .filter(|l| l.contains("call void @js_register_function_name(")) + .filter_map(|l| { + let at = l.find("@perry_closure_")?; + let rest = &l[at..]; + let end = rest.find(',')?; + rest[..end].rsplit("__").next()?.parse::().ok() + }) + .collect() +} + +#[test] +fn closure_display_names_are_emitted_in_func_id_order() { + let ids = registered_closure_ids(&ir(&closure_display_module())); + // Liveness: the construct under test was actually emitted. + assert_eq!( + ids.len(), + N as usize, + "expected one js_register_function_name per closure display name; \ + the fixture stopped exercising the emission path" + ); + let mut sorted = ids.clone(); + sorted.sort_unstable(); + assert_eq!( + ids, sorted, + "js_register_function_name calls must be emitted in FuncId order, not \ + `hir.closure_display_names` hash order (#7622)" + ); +} + +#[test] +fn closure_display_name_emission_is_run_to_run_deterministic() { + // A FRESH module per compile — same content, a different `RandomState` for + // its `closure_display_names` map. Reusing one module would re-iterate the + // same map and pass unconditionally. + let first = ir(&closure_display_module()); + let second = ir(&closure_display_module()); + assert!( + first.contains("call void @js_register_function_name("), + "liveness: fixture emitted no function-name registrations" + ); + assert_eq!( + first, second, + "two compiles of the same module must emit byte-identical IR (#7622)" + ); +} + +// --------------------------------------------------------------------------- +// Shape 2: the class-id dispatch tower +// --------------------------------------------------------------------------- + +fn method_fn(id: u32, name: &str) -> Function { + Function { + id, + name: name.to_string(), + type_params: Vec::new(), + params: Vec::new(), + return_type: Type::Number, + body: vec![Stmt::Return(Some(Expr::Number(1.0)))], + 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 plain_class(id: u32, name: &str, method: Function) -> Class { + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: Vec::new(), + constructor: None, + methods: vec![method], + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + computed_members: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + } +} + +/// `N` classes that all declare `m()`, plus `function callm(o: any) { o.m(); }`. +/// +/// `o` is an `any`-typed parameter, so `receiver_class_name` cannot name a +/// class and the call lowers through the dynamic-dispatch tower — one +/// `icmp`-guarded case per implementing class. +fn dispatch_tower_module() -> Module { + let mut m = empty_module("emission_order_tower.ts"); + for k in 0..N { + m.classes.push(plain_class( + k + 1, + &format!("C{:02}", k), + method_fn(200 + k, "m"), + )); + } + m.functions.push(Function { + id: 700, + name: "callm".to_string(), + type_params: Vec::new(), + params: vec![Param { + id: 701, + name: "o".to_string(), + ty: Type::Any, + default: None, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + }], + return_type: Type::Void, + body: vec![Stmt::Expr(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(701)), + property: "m".to_string(), + byte_offset: 0, + }), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + })], + 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(), + }); + m +} + +/// The class name of every `call … @perry_method_*__m(...)` inside the body of +/// `callm` — i.e. the dispatch tower's arm order. +/// +/// Scoped to that one function on purpose. Codegen also emits a per-class +/// closure-call wrapper (`artifacts.rs`, one `call @perry_method_…__m` each), +/// and those walk the `hir.classes` Vec, so they are already ordered and would +/// mask the tower's order if folded into the same list. +fn tower_arm_classes(ir: &str) -> Vec { + let mut in_callm = false; + let mut out = Vec::new(); + for l in ir.lines() { + if l.starts_with("define ") { + in_callm = l.contains("callm"); + continue; + } + if !in_callm || !l.contains("call") || !l.contains("@perry_method_") { + continue; + } + let Some(at) = l.find("@perry_method_") else { + continue; + }; + let rest = &l[at..]; + let Some(end) = rest.find('(') else { continue }; + // `@perry_method_____m` + let Some(class) = rest[..end] + .strip_suffix("__m") + .and_then(|s| s.rsplit("__").next()) + else { + continue; + }; + if class.starts_with('C') { + out.push(class.to_string()); + } + } + out +} + +#[test] +fn dispatch_tower_arms_are_emitted_in_class_id_order() { + let arms = tower_arm_classes(&ir(&dispatch_tower_module())); + // Liveness: the tower was actually emitted, with one arm per class. Without + // this the test would pass on a fixture that stopped producing a tower at + // all (e.g. if the receiver ever became statically typed). + assert_eq!( + arms.len(), + N as usize, + "expected one dispatch-tower arm per implementing class, got {:?}", + arms + ); + let mut sorted = arms.clone(); + sorted.sort(); + assert_eq!( + arms, sorted, + "dispatch-tower arms must be emitted in class-id order, not \ + `ctx.class_ids` hash order (#7622)" + ); +} + +#[test] +fn dispatch_tower_emission_is_run_to_run_deterministic() { + let first = ir(&dispatch_tower_module()); + let second = ir(&dispatch_tower_module()); + assert!( + first.contains("@perry_method_"), + "liveness: fixture emitted no class methods" + ); + assert_eq!( + first, second, + "two compiles of the same module must emit byte-identical IR (#7622)" + ); +} diff --git a/crates/perry-codegen/src/codegen/method_registry.rs b/crates/perry-codegen/src/codegen/method_registry.rs index 996ceafdc6..202a20a83c 100644 --- a/crates/perry-codegen/src/codegen/method_registry.rs +++ b/crates/perry-codegen/src/codegen/method_registry.rs @@ -39,7 +39,18 @@ pub(crate) fn build_method_names( // which mangled function name to call for `obj.method(args)`. Method // names are also scoped by module prefix. let mut method_names: HashMap<(String, String), String> = HashMap::new(); - for c in class_table.values() { + // Walk `class_table` by SORTED key, not in `HashMap` order (#7622). The + // body writes into `method_names` two order-dependent ways — plain + // `insert` (last writer wins) for the class's own keys, and + // `entry().or_insert_with` (first writer wins) for its `aliases` — and + // `class_table` is not key-unique over `&Class`: it mixes local classes, + // their self-binding alias keys, and imported class stubs, so two distinct + // `&Class` can contend for the same registry entry. `method_names` is what + // every call site consults to pick its `perry_method_*` callee, so a + // hash-order tie-break there is visible in the emitted IR. + let mut class_table_sorted: Vec<(&String, &&perry_hir::Class)> = class_table.iter().collect(); + class_table_sorted.sort_unstable_by_key(|(name, _)| *name); + for (_, c) in class_table_sorted { // Use the source module prefix for imported classes so the method // symbol name matches where the method was actually compiled. let class_prefix = imported_class_prefix diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 5c1e7e5fe2..d321d2152e 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -44,6 +44,8 @@ mod artifacts; mod boxed_locals; mod closure; mod closure_collect; +#[cfg(test)] +mod emission_order_tests; mod entry; mod func_registry; mod function; diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index bdf1b9ca56..92354eba0d 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -238,7 +238,21 @@ pub(crate) fn try_lower_instance_method_call( let mut impl_owner: Vec> = Vec::new(); let mut seen_pairs: std::collections::HashSet<(u32, String)> = std::collections::HashSet::new(); - for (start_cls, &start_cid) in ctx.class_ids.iter() { + // Walk `class_ids` in a FIXED order, not `HashMap` order (#7622). Each + // surviving entry becomes one `icmp`-guarded case block in the tower + // below, so the map's per-process iteration order was the tower's arm + // order: the same source compiled twice by the same binary emitted the + // same arms naming DIFFERENT `perry_method_*` callees per position. + // `seen_pairs` also dedups on `(class_id, fname)`, so with two names + // sharing a class id (a class-expression self-binding alias, or an + // imported class registered under both its own and its local-alias + // name) which arm is emitted FIRST is what the runtime's first-match + // tower actually executes — order-dependence that is behavioural, not + // just cosmetic. Sorting by `(class_id, name)` is total and stable. + let mut dispatch_roots: Vec<(&String, u32)> = + ctx.class_ids.iter().map(|(k, &v)| (k, v)).collect(); + dispatch_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0))); + for (start_cls, start_cid) in dispatch_roots { let mut cur: Option = Some(start_cls.clone()); while let Some(c) = cur { let key = (c.clone(), property.to_string()); @@ -691,7 +705,15 @@ pub(crate) fn try_lower_instance_method_call( // function than the static fallback, C needs an // explicit case in the dispatch table. let mut overrides: Vec<(u32, String)> = Vec::new(); - for (sub_name, &sub_id) in ctx.class_ids.iter() { + // Fixed order, not `HashMap` order — the virtual-override tower has + // the same #7622 defect as the interface tower above, and for the + // same reason: `overrides` is walked by index to emit the + // `vdispatch.caseN` blocks, the `icmp eq i32` chain and the phi + // incoming list, so the map's per-process order WAS the arm order. + let mut override_roots: Vec<(&String, u32)> = + ctx.class_ids.iter().map(|(k, &v)| (k, v)).collect(); + override_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0))); + for (sub_name, sub_id) in override_roots { if *sub_name == class_name { continue; } @@ -776,18 +798,29 @@ pub(crate) fn try_lower_instance_method_call( } walk = ctx.classes.get(&cur).and_then(|c| c.extends_name.clone()); } + // #7622: no `break` on the first name that carries `sub_id`. Class + // ids are not unique over `class_ids` KEYS — a class-expression + // self-binding alias (`var X = class _X`) and an imported class + // registered under both its own and its local-alias name both map + // two names to one id — so first-match-wins made this a hash-order + // tie-break, and the loser's arity is what decides how many + // TAG_UNDEFINED padding args EVERY emitted call in the tower + // carries. Taking the max over all names sharing the id is both + // order-independent and the direction this variable already means: + // under-padding is the #235 garbage-argument bug, over-padding just + // lets a default-param desugaring fire. for (sub_id, _) in &overrides { for (sub_name, &id) in ctx.class_ids.iter() { - if id == *sub_id { - if let Some(&n) = ctx - .method_param_counts - .get(&(sub_name.clone(), property.to_string())) - { - if n > max_explicit_arity { - max_explicit_arity = n; - } + if id != *sub_id { + continue; + } + if let Some(&n) = ctx + .method_param_counts + .get(&(sub_name.clone(), property.to_string())) + { + if n > max_explicit_arity { + max_explicit_arity = n; } - break; } } } From 644c14431f609444bb47c13f8b5449a15c8ff610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 08:57:17 +0200 Subject: [PATCH 2/4] docs(changelog): fragment for #7622 codegen determinism (#7625) --- changelog.d/7625-codegen-determinism.md | 78 +++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 changelog.d/7625-codegen-determinism.md diff --git a/changelog.d/7625-codegen-determinism.md b/changelog.d/7625-codegen-determinism.md new file mode 100644 index 0000000000..f6a3b8e370 --- /dev/null +++ b/changelog.d/7625-codegen-determinism.md @@ -0,0 +1,78 @@ +### Emitted LLVM IR is run-to-run deterministic again (#7622) + +Compiling one source twice with the same `perry` binary produced different IR. +Not a miscompile — but it defeats byte-level IR A/B, which is the primary +evidence the #7615 rooting-migration slices offer (three of nine apparent diffs +in #7620 were this, not the change), and it is a latent hazard on the object +cache. Both shapes were `std::collections` hash-map iteration order reaching the +emitter; Rust's default hasher is seeded per `RandomState`, so the order was a +fresh permutation every process. Six runs of each of the issue's two named +sources differed 5/5 from run 1 before the fix and 0/5 after. + +**Rayon was not the cause.** #7303 suspected module-codegen completion order; +`ctx.native_modules` is a `BTreeMap`, and parallel codegen +is untouched here. Nothing else varied either — no register renumbering, no +block reordering, no metadata churn, no `Instant`- or address-derived value. + +**Function-name registration** (`codegen/artifacts.rs`). The +`hir.closure_display_names` walk. Every entry mints a rodata constant through +`add_string_constant`, whose `@.str.N` counter numbers in first-use order, and +emits one `js_register_function_name` call into `__perry_init_strings_*`, so the +map's order set both. This is the same defect #7038 fixed 36 lines below it, in +the `closure_source_text` loop, and left standing here. Now sorted by `FuncId`. + +**The dispatch towers** (`lower_call/property_get/dynamic_dispatch.rs`). The +`ctx.class_ids` walk that builds `implementors` — each surviving entry is one +`icmp`-guarded case block, so map order *was* arm order, and consecutive call +sites named different `perry_method_*` callees run to run. The virtual-override +tower (`vdispatch.*`) reads the same map the same way. Both now walk +`(class_id, class_name)` order, which is total because the name is the map key. + +`max_explicit_arity`'s scan stopped at the **first** name carrying a class id, +and ids are not unique over `class_ids` keys — a class-expression self-binding +alias (`var X = class _X`) and an imported class registered under both its own +and its local-alias name each map two names to one id. So it was a hash-order +tie-break whose loser decides how many `TAG_UNDEFINED` padding args every +emitted call in the tower carries. It now takes the max over all names sharing +the id: what the variable already means, and the safe direction (under-padding +is the #235 garbage-argument bug; over-padding just lets a default-param +desugaring fire). + +**The method-symbol registry** (`codegen/method_registry.rs`). The +`class_table.values()` walk writes into the `(class, method) -> symbol` map both +`insert` (last writer wins) and `entry().or_insert_with` (first writer wins), and +`class_table` mixes local classes, their alias keys and imported stubs — so two +distinct `&Class` can contend for one entry, in the table every call site +consults to pick its callee. Now walked by sorted key. + +**The object cache was implicated.** `compute_object_cache_key` is a function of +`CompileOptions`, the post-transform HIR fingerprint, the perry version, a hash +of the perry binary and the codegen env vars — all deterministic — while the +emitted IR was not. Identical inputs therefore produced an identical key over +different `.o` bytes: a cache hit and a cold rebuild could hold different code. +For the shapes observed that difference is semantically neutral (the name +registry keys on distinct function pointers; tower arms key on distinct class +ids), but not by construction — the tower's `seen_pairs` dedups on +`(class_id, fname)`, which admits two arms sharing one class id with different +symbols, and there emission order *is* the behaviour. Same for the registry's +two tie-breaks. That hazard is now closed. + +**Tests** (`codegen/emission_order_tests.rs`, `--lib`, so they run on every PR +touching perry-codegen rather than only in the tag-gated integration tier). Four +cases over the two shapes with a reproducing fixture. Each builds its `Module` +fresh per compile — the offending maps live in the HIR, so double-compiling one +long-lived `Module` would re-iterate the same `RandomState` and pass +unconditionally — and each asserts its subject was live (registration count, +tower arm count) before judging order. Sabotage-verified in both directions: +reverting either sort alone turns exactly that shape's two tests red and leaves +the other shape's green. The virtual-override tower and the `method_registry` +walk are sorted with **no** test and labelled untested in the module prose: +`vdispatch` blocks appear in zero of 41 sampled `test_gap_*` programs (every +receiver-typed call measured is claimed earlier by `method_override.rs`'s +`method_direct` shape guard), and a green test over a fixture that emits no arms +asserts nothing. + +Validated locally: a 42-program sweep compiled 3x each is 0 nondeterministic / +41 compared; `cargo test -p perry-codegen --lib` 695 passed; the +gc-root-dominance corpus is green in both gated modes over 149 files with 40/40 +seeded violations caught. From ed7242b283b511fedae9790b1313ad38df85be6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 09:06:00 +0200 Subject: [PATCH 3/4] test(codegen): pin the tower fixture to class-id order, not name order CodeRabbit review: the fixture gave class C00..C15 ids 1..16, so class-id order and name order coincided and a name-keyed sort would have satisfied the assertion just as well. Ids now run opposite to names, and the expected arm sequence is spelled out rather than derived by sorting the observed list against itself. --- .../src/codegen/emission_order_tests.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/perry-codegen/src/codegen/emission_order_tests.rs b/crates/perry-codegen/src/codegen/emission_order_tests.rs index 9cce4b581d..0e61f142e3 100644 --- a/crates/perry-codegen/src/codegen/emission_order_tests.rs +++ b/crates/perry-codegen/src/codegen/emission_order_tests.rs @@ -305,11 +305,18 @@ fn plain_class(id: u32, name: &str, method: Function) -> Class { /// `o` is an `any`-typed parameter, so `receiver_class_name` cannot name a /// class and the call lowers through the dynamic-dispatch tower — one /// `icmp`-guarded case per implementing class. +/// +/// Class ids run OPPOSITE to class names on purpose: `C00` gets id `N`, `C15` +/// gets id 1. The emission order under test is by class id, and with ids +/// assigned in name order the two are indistinguishable — a sort keyed on the +/// class NAME would satisfy the assertion just as well, so the test would not +/// pin the property it claims. Reversed, only a class-id sort produces the +/// expected sequence. fn dispatch_tower_module() -> Module { let mut m = empty_module("emission_order_tower.ts"); for k in 0..N { m.classes.push(plain_class( - k + 1, + N - k, &format!("C{:02}", k), method_fn(200 + k, "m"), )); @@ -399,10 +406,13 @@ fn dispatch_tower_arms_are_emitted_in_class_id_order() { "expected one dispatch-tower arm per implementing class, got {:?}", arms ); - let mut sorted = arms.clone(); - sorted.sort(); + // Spelled out rather than derived by sorting `arms` itself: `C{:02}` runs + // opposite to the class ids, so this sequence is satisfied ONLY by a + // class-id ordering. A name-keyed sort — or `arms.sort()` compared against + // itself — would accept the reverse and prove nothing. + let expected: Vec = (0..N).rev().map(|k| format!("C{:02}", k)).collect(); assert_eq!( - arms, sorted, + arms, expected, "dispatch-tower arms must be emitted in class-id order, not \ `ctx.class_ids` hash order (#7622)" ); From 4904d056a445c53de95bc4a2933326c6e5363fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 8 Aug 2026 09:40:46 +0200 Subject: [PATCH 4/4] chore(version): bump to 0.5.1356 --- CLAUDE.md | 2 +- Cargo.lock | 152 ++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 3 files changed, 78 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9375e4e3b4..3c8c1b5eac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation. -**Current Version:** 0.5.1355 +**Current Version:** 0.5.1356 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 61bb49b197..7be84629bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1355" +version = "0.5.1356" [[package]] name = "perry-ui-tvos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1355" +version = "0.5.1356" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 8f91aab0c2..2ec8239abe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1355" +version = "0.5.1356" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry"