Skip to content

Commit e0f780d

Browse files
proggeramlugRalph Kuepper
andauthored
fix: #733 perry-main stack overflow on return self(...) (v0.5.900) (#743)
`transform_async_to_generator`'s inliner unconditionally substitutes single-Return-of-Call bodies at the call site. For a function whose body is `return self(args)`, the substituted result is another call to `self`, which the enclosing `inline_calls_in_expr` immediately re-inlines, and the recursion is unbounded → perry-main blows its 64 MB Windows stack. Two-line repro: `function foo(): any { return foo(); }; foo();` Surfaced via valibot-visit's transpiled `getSchemaByIssuePath` recursing through `findAsControlAction` → `schemaForEach`. The user's exact import (`import {} from "@piying/valibot-visit"` with valibot + valibot-visit in `compilePackages`) now compiles to a 3.6 MB binary instead of overflowing during HIR processing. Fix: add `body_calls_func(body, fn.id)` to `is_inlinable`. Walks statements/expressions for any `Expr::Call { callee: FuncRef(target) }` where `target == fn.id`. Closure bodies skipped — a self-reference inside a nested closure isn't a same-frame inlining target. `fib(10) === 55` still works: fib's body is `[If, Return(self+self)]` which had been rejected by other gates anyway (body.len() > 1). Co-authored-by: Ralph Kuepper <ralph@skelpo.com>
1 parent 8abac17 commit e0f780d

5 files changed

Lines changed: 181 additions & 70 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
Detailed changelog for Perry. See CLAUDE.md for concise summaries.
44

5+
## v0.5.900 — fix(transform): #733 perry-main stack overflow on self-recursive functions whose body is `return self(...)`. **Symptom (Windows).** The compiler crashed during HIR processing with `thread 'perry-main' (NNNNN) has overflowed its stack` and no other diagnostic. User's repro was `import {} from '@piying/valibot-visit'` with valibot-visit in `compilePackages`; bisected down to a 2-line minimal: `function foo(): any { return foo(); }; foo();` — any program compiling a function whose body is a single `return <self-call>(...)` blew the 64 MB perry-main stack. (More elaborate triggers — self-recursive function called from inside a loop, or a recursive caller of a self-recursive helper — were all variations on the same root cause. Surface area is broad: valibot-visit's `getSchemaByIssuePath` recursing through `findAsControlAction` → `schemaForEach` was the field repro.) **Root cause.** `crates/perry-transform/src/inline.rs::try_inline_simple_call` matches the "single Return(expr)" pattern by substituting the body's return expression at the call site — for `function foo() { return foo(); }` the substituted result is *another* call to `foo()`. The enclosing `inline_calls_in_expr` then recurses on the substituted result (line 2204 — "subsequent layers of small-function calls collapse cleanly"), sees `foo()` again, looks `foo` up in `func_candidates` (still present), inlines again, recurses, inlines, recurses... unbounded. `is_inlinable` had eight rejection criteria — async, generator, captures, rest params, body size, simple-control-flow, closure-capturing-params, super-call — but no self-recursion check. A self-recursive function with `body.len() == 1` and shape `[Return(Some(<call>))]` passed every gate. Surface comment at the inliner's outer recurse said "Termination relies on `is_inlinable` rejecting recursive functions in practice (cyclic call chains either don't form or get filtered out by other criteria)" — that assumption held for mutual recursion (because the called function's pattern usually didn't match) but missed the direct-self-recursive case where both the caller pattern AND the callee's body are the same `return self(...)` shape. Fixed-call-graph analyses for fibonacci-shaped recursion (`if base; return self(n-1) + self(n-2)`) survived because the body has more than one statement — `body.len() > 1` and `has_simple_control_flow` rejecting If-with-Return-in-both-branches both removed those from `func_candidates` indirectly. The catastrophic case was specifically the single-statement `return self(...)` shape, which is what valibot-visit's transpiled `getSchemaByIssuePath` reduces to through SWC's `function foo(x): any { return foo(x); }` synthesis path. **Fix.** Add `body_calls_func(&func.body, func.id)` to `is_inlinable`. The check walks statements + expressions recursively, returning true the moment it finds an `Expr::Call { callee: FuncRef(target_id) }` where `target_id == func.id`. Closure bodies are deliberately skipped — a self-reference inside a nested closure is a value-position read (the closure body becomes its own codegen unit, not an inlinable expression in the outer frame). Self-recursive functions are now never added to `func_candidates`; the inliner leaves them as ordinary calls and codegen emits a normal `call` instruction. **Validation.** Minimal repro (`function foo(): any { return foo(); }; foo();`) pre-fix: 64 MB stack overflow on perry-main during transform; post-fix: links and runs. User's exact reproduction (`import * as v from "valibot"; import {} from "@piying/valibot-visit"; v.parse(v.string(), "123")` with both packages in `compilePackages`) pre-fix: stack overflow after the two `Intl` HIR warnings; post-fix: compiles to a 3.6 MB binary. `fib(10) === 55` still inlines/dispatches correctly because fib's body is `[If { ... }, Return(self(n-1)+self(n-2))]` — body.len() > 1 path, and the addition contains two self-calls that the new check also rejects, so the inliner skips fib entirely (and codegen handles the recursion natively, which it always did for fib regardless). `cargo build --release -p perry-transform -p perry` clean. **Out of scope.** Mutual recursion between two functions (`f → g → f`) where both `f` and `g` happen to match the single-Return-of-Call pattern: theoretically possible but never observed in practice (mutual single-Return-of-Call cycles are vanishingly rare in real codebases because at least one side typically has a base case). If it surfaces, the fix is to graduate `body_calls_func` into a `body_in_call_cycle` SCC analysis on `func_candidates`. Tracked but not built yet. **Version-bump note.** Renumbered from v0.5.893 → v0.5.900 because the original v0.5.893 slot was taken by #736 (HarmonyOS Chart + TreeView) and main has since advanced to v0.5.899 via #734/#737/#735/#744 while this PR was in review.
6+
57
## v0.5.899 — deps: bump `toml` 0.8 → 1.1.2+spec-1.1.0 (#727) and `deno_core` 0.311 → 0.400 (#729). The two dependabot bumps that the v0.5.894 batch deferred as "real breakage" — addressed here with focused migrations rather than dropping them. **toml 1.x.** `<toml::Value as FromStr>` (called implicitly by `.parse::<toml::Value>()`) is no longer a document parser in 1.x — it's an inline-value parser that rejects anything with leading comments or whitespace ("unexpected content, expected nothing" at line 1, column 1 of `well_known_bindings.toml`, on every TOML file in the codebase that opens with the standard banner). Four callsites updated: `crates/perry/src/commands/compile/well_known.rs:100` (the well-known native-bindings registry — the loud one), `crates/perry/src/commands/run.rs:343`/`829`/`1453` (`perry.toml` reads for the `publish.exclude` list, icon source, and iOS bundle id), `crates/perry/src/commands/native/validate.rs:164` (`Cargo.toml` `[package].name` lookup for `perry native validate`). All swapped from `.parse::<toml::Value>()` to `toml::from_str::<toml::Value>(s)` — the crate-level `toml::from_str` still goes through the document deserializer + returns a `Value::Table`, which is the shape every callsite already expected to walk. Callsites that already use `.parse::<toml::Table>()` (the rest of `commands/setup.rs`, `commands/compile.rs`, `commands/i18n.rs`) are unaffected — `Table::from_str` still calls `crate::from_str` internally. Restores the 7 previously-failing tests (`shipped_toml_parses`, `dotenv_is_registered`, `node_prefix_stripped_on_lookup`, `unknown_package_returns_none`, `every_entry_references_a_workspace_crate`, `parser_rejects_missing_crate_field`, `read_crate_name_works`) and fixes the 31 compile-smoke regressions that were all the same panic surfacing from a different test path. **deno_core 0.400.** 89-patch jump that pulls in v8 0.106 → 147.4.0, a fundamental V8 binding revision: `v8::HandleScope` is replaced by `v8::PinScope` in nearly every signature; `JsRuntime::handle_scope()` is gone in favor of the `deno_core::scope!(scope, &mut runtime)` macro (which pins on the stack frame); `v8::TryCatch::new(scope)` returns a `ScopeStorage` that needs `v8::tc_scope!(tc_scope, scope)` to pin into a usable `&mut PinScope`; `Local<v8::String>::write_utf8` is replaced by `write_utf8_v2(buf, WriteFlags)` (different signature: drops the processed-chars `&mut usize` slot from the middle and uses `WriteFlags` instead of `WriteOptions`); `anyhow::Error` no longer implements `JsErrorClass` — op2-returning functions need `deno_error::JsErrorBox` or a typed wrapper; `extension!`'s `init_ops()` is renamed `init()`; `ModuleLoader::load` grew a 5th param via the new `ModuleLoadOptions` struct and `ModuleLoadReferrer` shape; `ModuleLoader::resolve` returns `ModuleLoaderError` (= `JsErrorBox`) instead of `AnyError`. Migration touches 5 source files in perry-jsruntime (`bridge.rs`, `interop.rs`, `lib.rs`, `modules.rs`, `ops.rs`) plus its `Cargo.toml` (adds `deno_error = "0.7"` as a direct dep so `JsErrorBox` is importable — was a transitive in 0.311). All `#[no_mangle] pub extern "C"` FFI surfaces keep identical signatures so perry-runtime, perry-codegen, and perry/src/commands keep their existing call shapes unchanged. The issue #255 re-entrancy escape hatch (`stash_trampoline_scope` / `try_trampoline_scope` — the `REENTRY_SCOPE_PTR` raw-pointer stash mechanism for crossing the V8 ↔ native trampoline) is preserved byte-for-byte at the memory level; only the typed view of the pointer migrates from `HandleScope` to `PinScope`. **Stack-limit override removed.** Pre-bump `JsRuntimeState::new` called `Isolate::SetStackLimit` (via the Itanium-mangled `_ZN2v87Isolate13SetStackLimitEm` because the v8 0.106 Rust bindings didn't expose it) right after `JsRuntime::new` returned, to fix arm64 SIGBUS on deep call chains. After the bump, calling that same exported symbol while the isolate is not entered (no `Isolate::Scope` on the stack) silently exits the process with code 0 — v8 147's stack-guard internals abort cleanly instead of crashing. The manifest of the silent-exit was the three parity tests (`test_issue_248_phase2_js_interop`, `test_issue_248_phase2b_js_callback`, `test_issue_255_jsruntime_reentrancy`) producing no output: every test that loaded a `.js` module called `js_runtime_init` (which constructed `JsRuntimeState` which hit the stack-limit setter and exited). Removed the manual override entirely — v8 147 picks a sane default stack limit from the calling thread's stack bounds and `deno_core::scope!` pins the isolate properly for each work scope, so the override is no longer needed. **Validation.** `cargo build --release -p perry-jsruntime -p perry-runtime -p perry-stdlib -p perry` clean. `cargo test --release -p perry --bin perry` 152 passed, 0 failed (was failing 7 tests pre-fix on the toml side). All three previously-failing parity tests now byte-equal Node. **Version-bump note.** Renumbered from v0.5.895 to v0.5.899 after #735 landed as v0.5.895/896/897/898 on main while this PR was in CI (four-patch sequence for #665 follow-ups).
68

79
## v0.5.898 — fix(hir): #665 — `compilePackages`-opted-in packages no longer trigger native-instance lowering, so `instance.method` returns a function instead of `number 0`. **Symptom.** With v0.5.895 (resolver opt-in), v0.5.896 (codegen side-map), and v0.5.897 (setTimeout trailing args) all applied, rate-limiter-flexible compiled and ran, but a probe reading `typeof limiter.consume` returned `"number"` instead of `"function"`. The value bits passed to `js_value_typeof` were `0x0000000000000000` (literal zero) — perry was evaluating `limiter.consume` as a zero-arg method *call* that returned 0.0, not as a *property read* yielding the function reference. Confusingly, `console.log("limiter.consume:", limiter.consume)` printed `[Function (anonymous)]`, so the same expression was being lowered consistently but its runtime path diverged. **Root cause.** Two-step trap. (1) `crates/perry-hir/src/ir.rs::is_native_module` only consulted the NATIVE_MODULES manifest. `rate-limiter-flexible` is in that manifest (the `perry-ext-ratelimit` binding exists, even if incomplete), so during HIR lowering the import statement was tagged `is_native = true`. (2) HIR lowering chain: `register_native_module(limiter_alias, "rate-limiter-flexible", ...)` → `new RateLimiterMemory(...)` in lower.rs:3848 → `lookup_native_module("RateLimiterMemory")` returns Some → `register_native_instance(limiter, "rate-limiter-flexible", "RateLimiterMemory")` → every subsequent `limiter.<prop>` member access in `expr_member::lower_member:475` matches the "native instance" arm and lowers as `NativeMethodCall { module, object: Some(limiter), method: <prop>, args: vec![] }` — a zero-arg FFI-getter call, on the assumption that `obj.prop` on a native binding is implemented as a getter function. For genuine FFI bindings (req.method on http.IncomingMessage etc.) this is correct. For a class compiled from JS source via cjs_wrap, it is wrong: `consume` is a real method, not a getter. The codegen routes the NativeMethodCall through `js_native_call_method` for `rate-limiter-flexible.<method>` — finds no matching FFI entry — and returns `0.0`. `typeof 0.0` is `"number"`. **Fix.** Two coordinated changes. (1) `crates/perry-hir/src/ir.rs`: add a thread-local `COMPILE_PACKAGES_OVERRIDE: RefCell<HashSet<String>>` plus setter/clearer (`set_compile_packages_override` / `clear_compile_packages_override`). Update `is_native_module` to: parse the package name out of the path via a new `package_name_of` helper (handles `node:` prefix + `@scope/pkg/subpath` scoped form), then return false when the package is in the override set even if it appears in NATIVE_MODULES. (2) `crates/perry/src/commands/compile/collect_modules.rs`: wrap the `lower_module_full` call with `set_compile_packages_override(ctx.compile_packages.clone())` before and `clear_compile_packages_override()` after. The thread-local is rayon-safe (each worker thread has its own copy) and cleared promptly so it can't leak to unrelated work. Together: HIR's import lowering now sees `is_native = false` for compile-package-opted-in packages, `register_native_module` and `register_native_instance` don't fire, `expr_member::lower_member` falls through to the regular property-access path, and codegen's static-class fast path at `expr.rs:3742-3772` emits `js_class_method_bind` — a bound-method closure NaN-boxed with POINTER_TAG. **Validation.** rate-limiter-flexible probe: `typeof limiter.consume: function` ✓ (was "number"); `limiter.consume: [Function (anonymous)]` unchanged ✓; `limiter.points: 3` and `limiter.duration: 60` unchanged ✓. Full workspace test suite exit 0. **Scope.** Closes the last user-visible gap from the #665 investigation. With this commit, `import { RateLimiterMemory } from "rate-limiter-flexible"; new RateLimiterMemory(...)` produces an instance whose methods are reachable as function values — the original blocking shape from the issue's repro. **Out of scope.** Calling `await limiter.consume(...)` end-to-end has to go through the bound-method-closure dispatch via `js_native_call_method`'s CLASS_VTABLE_REGISTRY lookup; that path works for the drizzle and hono fixtures so it should work here, but I haven't probed it through the full Promise resolution chain. The thread-local-based approach is the pragmatic choice over threading `compile_packages` through every HIR lowering API; if perry ever drops single-threaded sections of HIR lowering, the locking semantics may need to be revisited.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
88

99
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.
1010

11-
**Current Version:** 0.5.899
11+
**Current Version:** 0.5.900
1212

1313

1414
## TypeScript Parity Status

0 commit comments

Comments
 (0)