From 619b9f720910393a8d786405ffa31cbfd012fe2d Mon Sep 17 00:00:00 2001 From: Pepe Navarro Date: Wed, 16 Sep 2026 01:43:34 +0200 Subject: [PATCH] fix(frontend): anchor conditional-last-use frees at branch exit (I-182) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conditional-last-use re-anchor refused branches that may return, keeping the in-arm anchor so the return path stayed covered. But the in-arm anchor never fires on not-taken paths: an owner whose last read sits inside a returning arm leaked its buffer on every other path (steady 33 MB RSS growth over 500k calls, now flat), whether that path fell through to the function end or took a non-returning sibling arm. The refusal predates the owner return epilogue, which covers the return paths: its covered-check excludes ancestor branches, so a branch-exit anchor does not suppress the epilogue Free. Drop the refusal for both Inst and Param owners — the Free anchors at the branch exit on every fall-through path, and the epilogue fires on every return path. Regression coverage: two behavioral tests plus Valgrind/ASan fixtures for both arm shapes (last use in the returning arm; last use in a fall-through arm with a returning sibling). --- ISSUES.md | 6 ----- ryo-frontend/src/ownership/mod.rs | 36 +++++++++++++--------------- ryo/tests/asan_smoke.rs | 16 +++++++++++++ ryo/tests/common/mod.rs | 38 ++++++++++++++++++++++++++++++ ryo/tests/integration_ownership.rs | 29 +++++++++++++++++++++++ ryo/tests/valgrind_smoke.rs | 16 +++++++++++++ 6 files changed, 115 insertions(+), 26 deletions(-) diff --git a/ISSUES.md b/ISSUES.md index 1a8c35e..6ba4783 100644 --- a/ISSUES.md +++ b/ISSUES.md @@ -185,12 +185,6 @@ Resolved entries are **removed** from this file. Language-visible decisions behi **Summary:** When a call sits in true tail position — no pending drops, which is exactly what eager destruction arranges — codegen still emits a normal `call` followed by `return`, so every recursion frame is materialized and maximum depth is frame size × stack size. Measured 2026-09-15 on `benchmarks/eager_destruction`: ~80 B/frame → SIGSEGV at ~208k frames on an 8 MB stack, with the wall time dominated by first-touch cache misses and page faults on the ever-growing stack (CodSpeed: cache misses +400%, memory R/W +81%, while instructions fell −47%). Cranelift supports explicit `return_call` on aarch64/x86_64; emitting it for tail-position calls reuses the frame, giving O(1) stack, unbounded tail recursion, and collapsing those first-touch misses. The benchmark README already claims the tail-position story ("allowing the compiler to optimize the stack frames") — the compiler does not deliver it yet; Cranelift never performs tail-call optimization on its own. **Resolution:** In codegen, detect calls in tail position with no drops scheduled after them and emit Cranelift `return_call` instead of `call` + `return`. Requires the caller/callee signatures to satisfy `return_call` constraints, the ownership pass to guarantee no frees are pending after the call, and a CLIF-level test: `return_call` present for a self-tail-call after eager destruction, absent when a drop follows. Tail calls remove the overflow only for tail-recursive code — the guard-page diagnostic for non-tail recursion is still needed separately. -### I-182 — Owner free leaks on the not-taken path of a conditional last use when the taken arm returns - -**Files:** `ryo-frontend/src/ownership/mod.rs` (the `branch_may_not_return` conditional-last-use re-anchor in the last-use Free pass, ~:519-529) -**Summary:** When an owner's last read sits inside an if-arm that `return`s, the conditional-last-use re-anchor deliberately keeps the in-arm anchor (moving the Free to the branch exit would leave it unreachable on the return path). But the in-arm anchor then never fires on the NOT-taken path — the owner is still alive there and leaks its buffer. Confirmed with a control experiment: a heap-owning local whose only read is inside a returning if-arm leaks 32 B per call on the fall-through path (steady RSS growth), while the taken path frees correctly. The borrowed-param view-promotion frees inherited the same shape (a view whose last use is in a returning arm, or whose anchor is bypassed by an early return, leaked its promotion buffer); that side is covered by the promotion-free return epilogue, but the underlying owner-Free gap this entry tracks is pre-existing and orthogonal. -**Resolution:** For a conditional last use whose anchor arm may return, schedule the Free twice: keep the in-arm anchor (covers the taken path up to the return, alongside the return epilogue) AND add a branch-exit anchor gated to the arms that fall through (the `branch` field / arm-gated emission the `ConditionalDeadDrop` machinery already uses), so the not-taken path frees at the merge. Verify against the existing `last_use_in_if_fallthrough` and conditional-move Valgrind fixtures plus a new fixture pairing a returning arm with a live fall-through path. - --- ## 🟢 Cleanup diff --git a/ryo-frontend/src/ownership/mod.rs b/ryo-frontend/src/ownership/mod.rs index 83cee89..a361b7a 100644 --- a/ryo-frontend/src/ownership/mod.rs +++ b/ryo-frontend/src/ownership/mod.rs @@ -511,17 +511,19 @@ fn analyze_function( // exit — the earliest point where the value is dead // on ALL paths. Anchoring after the read itself // fires per-iteration in loops (UAF on later reads) - // and never fires on not-taken arms (leak). Skip - // the re-anchor when the branch may `return` (the - // exit anchor is unreachable on the return path) - // and for temps / branch-local bindings (their - // values don't exist on every exit path). + // and never fires on not-taken arms (leak). Arms + // that `return` never reach the branch exit, but + // the return epilogue owns those paths: its + // covered-check excludes ancestor branches, so the + // exit anchor does not suppress the epilogue Free. + // Skip the re-anchor only for temps / branch-local + // bindings (their values don't exist on every exit + // path). let anchor = match outermost_branch_of(tir, after) { Some(branch_stmt) - if branch_may_not_return(tir, branch_stmt) - && owner_binding_name(tir, *r).is_some_and(|name| { - declared_before_stmt(tir, name, branch_stmt) - }) => + if owner_binding_name(tir, *r).is_some_and(|name| { + declared_before_stmt(tir, name, branch_stmt) + }) => { branch_stmt } @@ -560,17 +562,11 @@ fn analyze_function( // the branch's exit. Anchoring after the read // itself fires per-iteration in loops (UAF on // later reads) and never fires on not-taken - // arms (leak). Skip when the branch may - // `return` (the exit anchor is unreachable on - // the return path). The declared-before check - // locals need is trivially true here: params - // precede the body. - match outermost_branch_of(tir, after) { - Some(branch_stmt) if branch_may_not_return(tir, branch_stmt) => { - Some(branch_stmt) - } - _ => Some(after), - } + // arms (leak); arms that `return` are covered + // by the return epilogue. The declared-before + // check locals need is trivially true here: + // params precede the body. + Some(outermost_branch_of(tir, after).unwrap_or(after)) } None => body_stmts.last().copied(), }) else { diff --git a/ryo/tests/asan_smoke.rs b/ryo/tests/asan_smoke.rs index c5dc9a6..c4c8b19 100644 --- a/ryo/tests/asan_smoke.rs +++ b/ryo/tests/asan_smoke.rs @@ -199,6 +199,22 @@ fn asan_early_return_live_local() { ); } +#[test] +fn asan_last_use_in_returning_arm_fallthrough() { + run_asan_smoke( + common::find_fixture("last_use_in_returning_arm_fallthrough"), + "last_use_in_returning_arm_fallthrough", + ); +} + +#[test] +fn asan_last_use_in_fallthrough_arm_sibling_returns() { + run_asan_smoke( + common::find_fixture("last_use_in_fallthrough_arm_sibling_returns"), + "last_use_in_fallthrough_arm_sibling_returns", + ); +} + #[test] fn asan_slice_view_no_free() { run_asan_smoke( diff --git a/ryo/tests/common/mod.rs b/ryo/tests/common/mod.rs index 6f4c544..ac4deac 100644 --- a/ryo/tests/common/mod.rs +++ b/ryo/tests/common/mod.rs @@ -396,6 +396,44 @@ fn f(): fn main(): \tf() +", + ), + ( + // Conditional last use inside an arm that RETURNS: the in-arm + // anchor covers the taken path (with the return epilogue), and + // the branch-exit anchor must cover the not-taken path, which + // falls through to the function end with the owner still live. + // The heap-backed initializer (runtime concat, > SSO inline + // capacity) gives ryo_str_free a real allocation to release. + "last_use_in_returning_arm_fallthrough", + "\ +fn f(): +\ts: str = int_to_str(42) + \"abcdefghijklmnopqrstuvwxyz0123456789\" +\td = false +\tif d: +\t\tprint(s) +\t\treturn + +fn main(): +\tf() +", + ), + ( + // Same family, mirrored arms: the last use sits in an arm that + // FALLS THROUGH while a sibling arm returns. The implicit-else + // path reaches the merge with the owner still live — only the + // branch-exit anchor frees it there. + "last_use_in_fallthrough_arm_sibling_returns", + "\ +fn f(x: int): +\ts: str = int_to_str(42) + \"abcdefghijklmnopqrstuvwxyz0123456789\" +\tif x == 1: +\t\tprint(s) +\telif x == 2: +\t\treturn + +fn main(): +\tf(3) ", ), ( diff --git a/ryo/tests/integration_ownership.rs b/ryo/tests/integration_ownership.rs index 64f30cf..4a96022 100644 --- a/ryo/tests/integration_ownership.rs +++ b/ryo/tests/integration_ownership.rs @@ -1148,3 +1148,32 @@ fn heap_str_last_use_in_inline_assert() { stderr ); } + +#[test] +fn last_use_in_returning_arm_fallthrough_runs_clean() { + // The owner's only read is inside a returning if-arm: the taken + // path frees in-arm (plus the return epilogue), the not-taken path + // must still free at the branch exit — the fallthrough reaches the + // function end with the owner live and no in-arm anchor firing. + // Heap-backed initializer (runtime concat, > SSO inline capacity) + // so ryo_str_free releases a real allocation. The Valgrind/ASan + // fixtures of the same name are the hard leak net; this is the + // behavioral guard. + assert_ryo_output( + "last_use_ret_arm.ryo", + "fn f(take: bool):\n\ts: str = int_to_str(42) + \"abcdefghijklmnopqrstuvwxyz0123456789\"\n\tif take:\n\t\tprint(s)\n\t\treturn\n\nfn main():\n\tf(true)\n\tf(false)\n\tprint(\"done\")\n", + "42abcdefghijklmnopqrstuvwxyz0123456789done", + ); +} + +#[test] +fn last_use_in_fallthrough_arm_sibling_returns_runs_clean() { + // Mirrored shape: the last use sits in an arm that falls through + // while a sibling arm returns — the implicit-else path reaches the + // merge with the owner live and must free at the branch exit. + assert_ryo_output( + "last_use_sibling_ret.ryo", + "fn f(x: int):\n\ts: str = int_to_str(42) + \"abcdefghijklmnopqrstuvwxyz0123456789\"\n\tif x == 1:\n\t\tprint(s)\n\telif x == 2:\n\t\treturn\n\nfn main():\n\tf(1)\n\tf(2)\n\tf(3)\n\tprint(\"done\")\n", + "42abcdefghijklmnopqrstuvwxyz0123456789done", + ); +} diff --git a/ryo/tests/valgrind_smoke.rs b/ryo/tests/valgrind_smoke.rs index c9c27d5..094f8c8 100644 --- a/ryo/tests/valgrind_smoke.rs +++ b/ryo/tests/valgrind_smoke.rs @@ -307,6 +307,22 @@ fn valgrind_early_return_live_local() { ); } +#[test] +fn valgrind_last_use_in_returning_arm_fallthrough() { + run_valgrind_smoke( + common::find_fixture("last_use_in_returning_arm_fallthrough"), + "last_use_in_returning_arm_fallthrough", + ); +} + +#[test] +fn valgrind_last_use_in_fallthrough_arm_sibling_returns() { + run_valgrind_smoke( + common::find_fixture("last_use_in_fallthrough_arm_sibling_returns"), + "last_use_in_fallthrough_arm_sibling_returns", + ); +} + #[test] fn valgrind_slice_view_no_free() { run_valgrind_smoke(