diff --git a/ISSUES.md b/ISSUES.md index 0b13f0b..1a8c35e 100644 --- a/ISSUES.md +++ b/ISSUES.md @@ -173,12 +173,6 @@ Resolved entries are **removed** from this file. Language-visible decisions behi **Summary:** The parser recovers at statement boundaries by emitting `StmtKind::Error` placeholders (R10), and sema's return-flow analysis already suppresses cascading `MissingReturn` diagnostics for *sema-level* errors via the TIR `Unreachable` sentinel. The parse-error path leaks between those two mechanisms: astgen lowers `StmtKind::Error` to *nothing*, so a function whose only `return` failed to parse reaches sema with a body that genuinely ends without returning, and the user gets a bogus E0036 stacked on the real parse diagnostic (reproduced 2026-09-11: a typo'd `return Person{name=p.name, age=.age + 1}` produced E0100 at the typo *and* E0036 "missing return" on the function signature, pointing the user at the wrong place). **Resolution:** Lower `StmtKind::Error` to a UIR error/unreachable sentinel (or have sema treat it as one) so the existing TIR `Unreachable` rule suppresses `MissingReturn` for parse-broken bodies, matching the cascade suppression sema tests already enforce for sema-internal errors. Regression test: a function whose only return statement fails to parse yields exactly the parse diagnostic, no E0036. -### I-176 — Slicing a borrowed `str`/`bytes` param whose argument is inline (SSO) leaks the promotion buffer - -**Files:** `ryo-backend/src/codegen/views.rs` (`emit_ensure_heap_for_view_base`), `runtime/src/lib.rs` (`__ryo_str_ensure_heap` / `__ryo_bytes_ensure_heap`), `ryo-frontend/src/ownership/` (no free is ever scheduled for borrowed params) -**Summary:** With the tagged-slot string runtime, creating a view from an owner-typed value promotes an inline (≤ 23 B) representation to heap so the view addresses memory that never moves. Plain locals write the promoted triple back into the binding's codegen locals (freed at last use) and struct fields promote in place (the struct drop frees the field) — but a *borrowed* parameter (`fn f(s: str): v = s[0:1]`) has no scheduled free: the callee promotes its by-value copy of the caller's inline triple into a fresh heap buffer that nothing owns. Reproduced under Valgrind (16 bytes definitely lost per call) with `fn scan(s: str): v = s[0:1]; print(v)` called on an `int_to_str` argument. Heap and static arguments are unaffected (promotion no-ops; the view borrows the caller's buffer). The naive fix is unsound: at any free site a callee-promoted buffer is indistinguishable from a caller-owned heap buffer — both are plain `(ptr, len, cap)` triples — so freeing the param would double-free caller memory whenever the argument was heap. -**Resolution:** The ownership pass already computes view liveness for the P2 freeze; use it to schedule a free at the view's last use when a slice/`ToView` base is a borrowed (non-`inout`) `str`/`bytes` param, and make the promotion distinguishable at runtime — e.g. promote through a callee that reports whether it allocated, or route borrowed-param view bases through `ryo_str_from_view`-style materialization as a tracked temporary owner instead of in-place promotion. Regression test: the repro above must come out Valgrind-clean. - ### I-177 — AOT binaries die with SIGSEGV on stack overflow; no guard-page detection or diagnostic **Files:** `ryo-backend/src/codegen.rs` (function prologue emission), `ryo-backend/src/linker.rs` (link-time stack size / guard-page setup), `runtime/src/` (no stack-limit check or signal handler exists) @@ -191,6 +185,12 @@ 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 @@ -405,6 +405,12 @@ Resolved entries are **removed** from this file. Language-visible decisions behi **Summary:** Slice results and literal values are packed `(ptr, len)` pairs represented as i128, so extracting one half is a 128-bit shift — which Cranelift legalizes into a ~9-instruction funnel-shift/select sequence (`lsr`/`lsl`/`orr`/`csel`) instead of the register move it already is. Disassembly of `benchmarks/string_slicing`'s `count_fox` (aarch64, 2026-09-15): two such sequences per scan iteration, one to unpack the `__ryo_slice` result and one to unpack the literal — ~18 wasted instructions × 700k iterations ≈ 12.6M instructions, on top of the extern-call overhead tracked separately. Inlining the slice/eq bodies will not remove this if the values keep flowing as i128. **Resolution:** Stop representing small pair values as packed i128 end to end. The C ABI does not require it: on aarch64/x86-64 SysV a `u128` return and a `#[repr(C)]` two-`u64` struct return occupy the same two registers, so changing the runtime signatures (`__ryo_slice` :427, `ryo_str_from_literal` :358, both currently `-> u128` via `pack_pair` :270) to return a repr(C) pair — and modeling views as two i64 SSA values in TIR/codegen — is machine-identical at the boundary while eliminating the i128 type that triggers the legalization. Verify Cranelift maps the two-register struct return correctly on the Windows x64 target (different struct-return convention there) before committing to the signature change. +### I-183 — View-liveness back-edge merge is one-pass first-wins; reads inside a loop are attributed to the pre-loop slice + +**Files:** `ryo-frontend/src/ownership/views.rs` (`collect_view_liveness` / `view_liveness_loop_body` back-edge merge :603-621), `ryo-frontend/src/ownership/mod.rs` (promo scheduling fallback that compensates :700-731) +**Summary:** The view-liveness pre-pass walks a loop body once and merges back-edge bindings first-wins, so a read of a view binding inside or after a loop is attributed to the binding's *pre-loop* slice inst, leaving an in-loop rebinding slice with no recorded last use. Consumers of `view_last_use` that release memory must compensate conservatively: the promotion-free scheduler anchors in-loop bound-never-read candidates at function end (over-liveness, sound) instead of at the true last read. The pre-loop slice's buffer can likewise be kept alive past its real last use. +**Resolution:** Rewrite the loop-body liveness walk as a fixpoint (re-walk until `last_use` assignments converge) so post-loop and in-loop reads attribute to the most recent slice inst. Once attribution is exact, tighten the promotion-free fallback from the function-end anchor back to the true last use. + --- ## Cross-References diff --git a/examples/substring_search.ryo b/examples/substring_search.ryo new file mode 100644 index 0000000..cd5c913 --- /dev/null +++ b/examples/substring_search.ryo @@ -0,0 +1,43 @@ +# Substring search — sliding a mutable strview window over a buffer. +# Shows the projection pattern at work: scanning is zero-copy (views +# borrow the owner's storage, never copy), and `mut` lets one name +# track the current position as it advances. +# +# Note: `rest[1:]` steps one byte at a time, which is exact for ASCII +# text. With multibyte UTF-8 the byte index can land inside a +# character, and slicing at a non-char-boundary index panics — proper +# code-point iteration comes with the planned `utf8` module. + +# Return the index of the first occurrence of `needle` in `haystack`, +# or -1 when there is no match. An empty needle matches at index 0. +fn find(haystack: strview, needle: strview) -> int: + if needle.len() == 0: + return 0 + # `rest` is the unsearched remainder: one mutable view, re-sliced a + # byte at a time. Views are read-only borrows, so the whole scan + # allocates nothing. + mut rest = haystack[:] + mut index: int = 0 + while rest.len() >= needle.len(): + if rest[0:needle.len()] == needle: + return index + rest = rest[1:] + index += 1 + return -1 + +fn main(): + text: str = "the quick brown fox jumps over the lazy dog" + + print(int_to_str(find(text, "fox"))) # 16 + print("\n") + print(int_to_str(find(text, "dog"))) # 40 + print("\n") + print(int_to_str(find(text, "cat"))) # -1 + print("\n") + print(int_to_str(find(text, ""))) # 0 — empty needle matches at the start + print("\n") + + # Owned str and strview both pass to strview parameters — no copies. + word = text[16:19] + print(int_to_str(find(word, "o"))) # 1 + print("\n") diff --git a/ryo-backend/src/codegen/expr.rs b/ryo-backend/src/codegen/expr.rs index d00f039..ecb39a7 100644 --- a/ryo-backend/src/codegen/expr.rs +++ b/ryo-backend/src/codegen/expr.rs @@ -430,7 +430,7 @@ impl Codegen { /// rather than `last() == Some(&b)` so a Free anchored to a /// parent arm still fires when codegen is inside a nested child /// arm of that parent. - fn branch_active( + pub(crate) fn branch_active( branch: Option, stack: &[ryo_core::ownership::BranchId], ) -> bool { @@ -551,7 +551,7 @@ impl Codegen { /// (`ryo_bytes_free` when `is_bytes`, else `ryo_str_free`). /// Resolved only at call sites that survive the cap==0 elision, so /// an all-static schedule never declares an unused import. - fn free_ref_for( + pub(crate) fn free_ref_for( builder: &mut FunctionBuilder, ctx: &mut FunctionContext<'_, M>, str_free_ref: &mut Option, diff --git a/ryo-backend/src/codegen/mod.rs b/ryo-backend/src/codegen/mod.rs index e7989a9..04f8911 100644 --- a/ryo-backend/src/codegen/mod.rs +++ b/ryo-backend/src/codegen/mod.rs @@ -23,7 +23,9 @@ //! / inline expansion lands. Zig calls the analogous mapping //! in `Air.zig` "liveness"; we don't need full liveness yet. -use cranelift::codegen::ir::{ArgumentPurpose, MemFlagsData}; +use cranelift::codegen::ir::{ + ArgumentPurpose, MemFlagsData, StackSlot, StackSlotData, StackSlotKind, +}; use cranelift::codegen::isa; use cranelift::codegen::settings::{self, Configurable}; use cranelift::prelude::*; @@ -32,7 +34,7 @@ use cranelift_module::{DataDescription, DataId, FuncId, Linkage, Module}; use cranelift_object::{ObjectBuilder, ObjectModule}; use ryo_core::tir::{ParamMode, Tir, TirData, TirRef, TirTag}; use ryo_core::types::{InternPool, StringId, TypeId, TypeKind}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use target_lexicon::Triple; mod arith; @@ -46,6 +48,11 @@ mod views; /// cap at 16. Derived from `RyoStrFat`, not re-hardcoded. const STR_SLOT_SIZE: u32 = 24; +/// Promotion scratch-slot layout for borrowed-param view bases: flag +/// word at 0 (1 = this callee promoted, 0 = pass-through), triple +/// ptr/len/cap at 8/16/24. +pub(crate) const PROMO_SLOT_SIZE: u32 = 32; + /// How a statement or body ended the current block, if it did. /// Replaces the `bool` that conflated Break/Continue with Return: /// callers distinguish "block ended" (`!= None`) from "the function @@ -292,6 +299,17 @@ pub(crate) struct FunctionContext<'a, M: Module> { /// an undo log, same scoping discipline as `locals`. fat_locals: Vec>, fat_locals_undo: Vec<(u32, Option)>, + /// Borrowed-param view bases (Slice/ToView) that were promoted + /// through a scratch slot instead of the param's FatLocals: base + /// inst → slot holding flag (offset 0) + promoted triple (8/16/24). + promo_slots: HashMap, + /// Dense flag per `sidecar.promotion_frees` index (emission-time + /// dedup, same discipline as `freed_at`). + promo_freed_at: Vec, + /// Anchor `TirRef` → indices into `sidecar.promotion_frees`. + promo_free_by_after: Vec>, + /// Unfired `promotion_frees` indices for the end-of-statement sweep. + pending_promo_sweep: Vec, /// `strview` view bindings (M8.4): two SSA `Variable`s per binding, /// mirroring `fat_locals`. Views are non-owning — they never /// appear in the free schedule. @@ -943,6 +961,30 @@ impl Codegen { let (free_binding_names, free_binding_param_names) = Self::build_free_binding_names(tir, pool); + let mut promo_free_by_after: Vec> = vec![Vec::new(); tir.instructions.len()]; + for (idx, pf) in func_sidecar.promotion_frees.iter().enumerate() { + debug_assert!( + !pf.after.is_param(), + "promotion-free anchors are never param sentinel refs" + ); + promo_free_by_after[pf.after.index()].push(idx); + } + let pending_promo_sweep: Vec = (0..func_sidecar.promotion_frees.len()).collect(); + + // One scratch slot per borrowed-param view base scheduled + // for a promotion free: flag word (offset 0) + promoted + // triple (8/16/24), per PROMO_SLOT_SIZE. + let mut promo_slots: HashMap = HashMap::new(); + for pf in &func_sidecar.promotion_frees { + promo_slots.entry(pf.base).or_insert_with(|| { + builder.create_sized_stack_slot(StackSlotData::new( + StackSlotKind::ExplicitSlot, + PROMO_SLOT_SIZE, + 3, + )) + }); + } + let mut ctx: FunctionContext<'_, M> = FunctionContext { module: &mut self.module, data_ctx: &mut self.data_ctx, @@ -964,6 +1006,10 @@ impl Codegen { loop_stack: Vec::new(), fat_locals: fat_param_locals, fat_locals_undo, + promo_slots, + promo_freed_at: vec![false; func_sidecar.promotion_frees.len()], + promo_free_by_after, + pending_promo_sweep, view_locals: view_param_locals, view_locals_undo, struct_locals: struct_param_locals, @@ -1008,6 +1054,27 @@ impl Codegen { } } + // The promotion flag must start cleared: the + // free-before-overwrite in emit_ensure_heap_for_view_base + // and the scheduled promo frees both branch on it, and a + // first-iteration garbage flag would free garbage. Iterate + // promotion_frees (deduped), not the HashMap: sidecar order + // is deterministic, HashMap iteration order is not — identical + // input must yield identical emission. + let mut zeroed: HashSet = HashSet::new(); + for pf in &func_sidecar.promotion_frees { + if !zeroed.insert(pf.base) { + continue; + } + let slot = *ctx + .promo_slots + .get(&pf.base) + .expect("every promotion-free base has a scratch slot"); + let addr = builder.ins().stack_addr(ctx.int_type, slot, 0); + let zero = builder.ins().iconst(types::I64, 0); + builder.ins().store(MemFlagsData::trusted(), zero, addr, 0); + } + // Hoist string and bytes literals while the entry block is // still the current block: one from-literal call per distinct // literal per function, dominating every use. @@ -1093,7 +1160,9 @@ impl Codegen { // sub-expression-anchored entries whose consumers have // now finished emitting IR. Self::emit_due_frees(builder, ctx, stmt_ref)?; + Self::emit_due_promo_frees(builder, ctx, stmt_ref)?; Self::sweep_due_frees(builder, ctx)?; + Self::sweep_due_promo_frees(builder, ctx)?; } } Ok(terminator) @@ -1305,12 +1374,14 @@ impl Codegen { builder.ins().store(MemFlagsData::trusted(), len, sret, 8); builder.ins().store(MemFlagsData::trusted(), cap, sret, 16); Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; Self::emit_return(builder, ctx, &[])?; } else if matches!(ctx.pool.kind(ctx.tir.return_type), TypeKind::Struct) { return Self::emit_struct_return(builder, ctx, r, operand); } else { let val = Self::eval_inst(builder, ctx, operand)?; Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; Self::emit_return(builder, ctx, &[val])?; } Ok(Terminator::Return) @@ -1322,9 +1393,11 @@ impl Codegen { if is_main { let zero = builder.ins().iconst(ctx.int_type, 0); Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; Self::emit_return(builder, ctx, &[zero])?; } else { Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; Self::emit_return(builder, ctx, &[])?; } Ok(Terminator::Return) @@ -1477,6 +1550,7 @@ impl Codegen { // statements. Without this call the Frees would // simply never be emitted. Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; let Some(loop_ctx) = ctx.loop_stack.last() else { return Err("codegen reached break outside loop".to_string()); }; @@ -1491,6 +1565,7 @@ impl Codegen { // See Break above for why the Frees must be emitted // here instead of via the post-stmt sweep. Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; let Some(loop_ctx) = ctx.loop_stack.last() else { return Err("codegen reached continue outside loop".to_string()); }; diff --git a/ryo-backend/src/codegen/structs.rs b/ryo-backend/src/codegen/structs.rs index f37be87..13a7ef3 100644 --- a/ryo-backend/src/codegen/structs.rs +++ b/ryo-backend/src/codegen/structs.rs @@ -463,6 +463,7 @@ impl Codegen { let ret_ty = ctx.tir.return_type; Self::emit_struct_copy(builder, ctx, sret, src, ret_ty)?; Self::emit_due_frees(builder, ctx, r)?; + Self::emit_due_promo_frees(builder, ctx, r)?; Self::emit_return(builder, ctx, &[])?; Ok(Terminator::Return) } diff --git a/ryo-backend/src/codegen/views.rs b/ryo-backend/src/codegen/views.rs index c1b0f76..a8b2d0d 100644 --- a/ryo-backend/src/codegen/views.rs +++ b/ryo-backend/src/codegen/views.rs @@ -5,7 +5,7 @@ //! choke point every view-creating op (slice, `ToView`) uses to turn an //! owner-typed base into stable, never-moving memory. -use cranelift::codegen::ir::{BlockArg, MemFlagsData, StackSlotData, StackSlotKind}; +use cranelift::codegen::ir::{BlockArg, FuncRef, MemFlagsData, StackSlotData, StackSlotKind}; use cranelift::prelude::*; use cranelift_module::Module; use ryo_core::tir::{TirRef, TirTag}; @@ -29,6 +29,11 @@ impl Codegen { /// - Named bindings spill → call → reload → `def_var` back into /// their `FatLocals` (the str_push write-back shape; SSA-correct /// at every later program point, including branch joins). + /// Borrowed `str`/`bytes` params are the exception: their + /// promoted triple goes into a per-base scratch slot (flag + + /// triple) instead of `FatLocals`, so the param keeps its + /// original inline triple and the caller's heap buffer is never + /// freed by the callee; the ownership pass schedules the free. /// - Anonymous temporaries spill into a scratch slot and re-cache /// the promoted triple — their scheduled Free reads `cached_repr`. pub(crate) fn emit_ensure_heap_for_view_base( @@ -96,6 +101,21 @@ impl Codegen { if Self::is_static_cap_zero(builder.func, cap) { return Ok((ptr, len)); } + // Borrowed-param base: the ownership pass scheduled a promotion + // free for this base. The promoted triple goes into the scratch + // slot — NOT back into the param's FatLocals, which must keep + // the original inline triple so reads of the param after the + // view's death stay valid, and so the caller's heap buffer is + // never freed by the callee. + let promo_slot = ctx.promo_slots.get(&r).copied(); + if promo_slot.is_some() { + // The ownership pass only schedules promotion frees for + // bases that are Vars of borrowed str/bytes params. + debug_assert!( + matches!(ctx.tir.inst(r).tag, TirTag::Var), + "promotion base must be a named param Var" + ); + } // Branch on the runtime tag: only an inline base needs the // spill/promote/reload round trip. A heap base is already // stable, so its (ptr, len) flows straight to the merge — @@ -114,20 +134,92 @@ impl Codegen { builder.append_block_param(merge_block, ctx.int_type); builder.append_block_param(merge_block, types::I64); builder.append_block_param(merge_block, types::I64); - builder.ins().brif( - is_inline, - inline_block, - &[], - merge_block, - &[ - BlockArg::Value(ptr), - BlockArg::Value(len), - BlockArg::Value(cap), - ], - ); + let heap_block = if promo_slot.is_some() { + Some(builder.create_block()) + } else { + None + }; + match heap_block { + Some(hb) => builder.ins().brif(is_inline, inline_block, &[], hb, &[]), + None => builder.ins().brif( + is_inline, + inline_block, + &[], + merge_block, + &[ + BlockArg::Value(ptr), + BlockArg::Value(len), + BlockArg::Value(cap), + ], + ), + }; // Single predecessor (the brif above) — seal immediately. builder.seal_block(inline_block); + if let Some(hb) = heap_block { + // Single predecessor (the brif above) — seal immediately. + builder.seal_block(hb); + builder.switch_to_block(hb); + // Pass-through: record flag=0 + the caller's triple so the + // scheduled free no-ops. + let slot = promo_slot.expect("heap block exists only with a promo slot"); + let addr = builder.ins().stack_addr(ctx.int_type, slot, 0); + let zero = builder.ins().iconst(types::I64, 0); + builder.ins().store(MemFlagsData::trusted(), zero, addr, 0); + builder.ins().store(MemFlagsData::trusted(), ptr, addr, 8); + builder.ins().store(MemFlagsData::trusted(), len, addr, 16); + builder.ins().store(MemFlagsData::trusted(), cap, addr, 24); + builder.ins().jump( + merge_block, + &[ + BlockArg::Value(ptr), + BlockArg::Value(len), + BlockArg::Value(cap), + ], + ); + } builder.switch_to_block(inline_block); + if let Some(slot) = promo_slot { + // Free-before-overwrite: a prior iteration's promotion + // buffer is still recorded here when the view outlives one + // loop iteration (loop-deferred death). The flag is zeroed + // at function entry and after every free, so this is exact. + let addr = builder.ins().stack_addr(ctx.int_type, slot, 0); + let old_flag = builder + .ins() + .load(types::I64, MemFlagsData::trusted(), addr, 0); + let oldfree_block = builder.create_block(); + let spill_block = builder.create_block(); + builder + .ins() + .brif(old_flag, oldfree_block, &[], spill_block, &[]); + // Single predecessor (the brif above) — seal immediately. + builder.seal_block(oldfree_block); + builder.switch_to_block(oldfree_block); + let old_ptr = builder + .ins() + .load(ctx.int_type, MemFlagsData::trusted(), addr, 8); + let old_cap = builder + .ins() + .load(types::I64, MemFlagsData::trusted(), addr, 24); + let free_callee = if is_bytes { + "ryo_bytes_free" + } else { + "ryo_str_free" + }; + let free_ref = Self::declare_runtime_fn( + ctx.module, + builder, + free_callee, + &[ctx.int_type, types::I64], + &[], + )?; + builder.ins().call(free_ref, &[old_ptr, old_cap]); + builder.ins().jump(spill_block, &[]); + // Two predecessors (the brif else-edge and the oldfree + // jump) — seal only now that both are emitted. + builder.seal_block(spill_block); + builder.switch_to_block(spill_block); + } let slot = builder.create_sized_stack_slot(StackSlotData::new( StackSlotKind::ExplicitSlot, STR_SLOT_SIZE, @@ -153,12 +245,33 @@ impl Codegen { let out_cap = builder .ins() .load(types::I64, MemFlagsData::trusted(), addr, 16); + if let Some(slot) = promo_slot { + // Record flag=1 + the promoted triple in the scratch slot: + // the ownership-pass-scheduled promo free releases this + // buffer at the view's last use. + let addr = builder.ins().stack_addr(ctx.int_type, slot, 0); + let one = builder.ins().iconst(types::I64, 1); + builder.ins().store(MemFlagsData::trusted(), one, addr, 0); + builder + .ins() + .store(MemFlagsData::trusted(), out_ptr, addr, 8); + builder + .ins() + .store(MemFlagsData::trusted(), out_len, addr, 16); + builder + .ins() + .store(MemFlagsData::trusted(), out_cap, addr, 24); + } // Write the promoted triple back into owner-side storage so // the owner's free releases the heap buffer. Only the inline // path needs this — on the heap path the binding's fat locals - // already hold the identical bits. + // already hold the identical bits. Borrowed-param bases skip + // the write-back: their promoted triple lives in the promo + // scratch slot above. let local_name = Self::local_name_of(ctx, r); - if let Some(name) = local_name { + if promo_slot.is_none() + && let Some(name) = local_name + { // Every fat binding gets FatLocals at the param/local // preamble, so a missing entry would be an invariant // violation; the silent fall-through is defensive only. @@ -173,15 +286,6 @@ impl Codegen { // `fat_locals`, never through `cached_repr`. Latent, not // live: TIR is tree-shaped today, so each Var inst is // evaluated once at its own use site. - // Known leak, unrelated to the fall-through: for a - // BORROWED param the write-back lands but no free is - // ever scheduled (the callee doesn't own its params), - // so a promoted inline argument's buffer leaks. The - // free cannot simply be added — it cannot tell a - // callee-promoted buffer apart from a caller-owned heap - // buffer and would double-free; the planned resolution - // is an ownership-pass-scheduled free of the promotion - // buffer at the view's last use. } builder.ins().jump( merge_block, @@ -215,4 +319,115 @@ impl Codegen { } Ok((m_ptr, m_len)) } + + /// Fire promotion frees anchored after `tir_ref`. Mirrors + /// `emit_due_frees`. + pub(crate) fn emit_due_promo_frees( + builder: &mut FunctionBuilder, + ctx: &mut FunctionContext<'_, M>, + tir_ref: TirRef, + ) -> Result<(), String> { + if ctx.sidecar.promotion_frees.is_empty() { + return Ok(()); + } + let Some(indices) = ctx.promo_free_by_after.get(tir_ref.index()) else { + return Ok(()); + }; + let pending: Vec = indices + .iter() + .copied() + .filter(|&idx| { + let pf = &ctx.sidecar.promotion_frees[idx]; + Self::branch_active(pf.branch, &ctx.branch_stack) && !ctx.promo_freed_at[idx] + }) + .collect(); + Self::emit_promo_frees(builder, ctx, pending) + } + + /// End-of-statement sweep for promotion frees whose anchor passed + /// without an `emit_due_promo_frees` call. Mirrors `sweep_due_frees`. + pub(crate) fn sweep_due_promo_frees( + builder: &mut FunctionBuilder, + ctx: &mut FunctionContext<'_, M>, + ) -> Result<(), String> { + if ctx.pending_promo_sweep.is_empty() { + return Ok(()); + } + let pending: Vec = ctx + .pending_promo_sweep + .iter() + .copied() + .filter(|&idx| { + let pf = &ctx.sidecar.promotion_frees[idx]; + Self::branch_active(pf.branch, &ctx.branch_stack) + && ctx.promo_slots.contains_key(&pf.base) + && Self::cached_repr(ctx, pf.after).is_some() + }) + .collect(); + Self::emit_promo_frees(builder, ctx, pending) + } + + /// Emit one flag-conditional free per pending promotion free: load + /// the flag from the base's scratch slot; only on the promoted + /// path free the recorded triple and clear the flag (so a later + /// free-before-overwrite or duplicate anchor no-ops). + fn emit_promo_frees( + builder: &mut FunctionBuilder, + ctx: &mut FunctionContext<'_, M>, + pending: Vec, + ) -> Result<(), String> { + if pending.is_empty() { + return Ok(()); + } + let mut str_free_ref: Option = None; + let mut bytes_free_ref: Option = None; + for idx in pending { + if ctx.promo_freed_at[idx] { + continue; + } + let pf = ctx.sidecar.promotion_frees[idx].clone(); + // Invariant: compile_function creates a scratch slot for + // every scheduled base — a missing entry would silently + // drop the free, so fail loudly like emit_frees does. + let slot = *ctx.promo_slots.get(&pf.base).ok_or_else(|| { + format!( + "ownership pass scheduled promotion free for base %{} but no scratch slot was created", + pf.base.index() + ) + })?; + ctx.promo_freed_at[idx] = true; + let is_bytes = matches!(ctx.pool.kind(ctx.tir.inst(pf.base).ty), TypeKind::Bytes); + let addr = builder.ins().stack_addr(ctx.int_type, slot, 0); + let flag = builder + .ins() + .load(types::I64, MemFlagsData::trusted(), addr, 0); + let free_block = builder.create_block(); + let done_block = builder.create_block(); + builder.ins().brif(flag, free_block, &[], done_block, &[]); + builder.seal_block(free_block); + builder.switch_to_block(free_block); + let ptr = builder + .ins() + .load(ctx.int_type, MemFlagsData::trusted(), addr, 8); + let cap = builder + .ins() + .load(types::I64, MemFlagsData::trusted(), addr, 24); + let free_ref = Self::free_ref_for( + builder, + ctx, + &mut str_free_ref, + &mut bytes_free_ref, + is_bytes, + )?; + builder.ins().call(free_ref, &[ptr, cap]); + let zero = builder.ins().iconst(types::I64, 0); + builder.ins().store(MemFlagsData::trusted(), zero, addr, 0); + builder.ins().jump(done_block, &[]); + builder.seal_block(done_block); + builder.switch_to_block(done_block); + } + ctx.pending_promo_sweep + .retain(|&idx| !ctx.promo_freed_at[idx]); + Ok(()) + } } diff --git a/ryo-core/src/ownership.rs b/ryo-core/src/ownership.rs index 5528179..ae03fbb 100644 --- a/ryo-core/src/ownership.rs +++ b/ryo-core/src/ownership.rs @@ -18,6 +18,20 @@ pub struct FreePoint { pub branch: Option, } +/// One scheduled promotion-buffer free. When a borrowed (non-inout) +/// `str`/`bytes` param is used as a view base, codegen promotes the +/// callee's inline copy to heap and records the result in a scratch +/// slot keyed by `base`; codegen emits a flag-conditional +/// `ryo_str_free(ptr, cap)` from that slot after the instruction at +/// `after`, gated by `branch` (same discipline as [`FreePoint`]). +#[derive(Clone, Debug)] +pub struct PromoFree { + pub after: TirRef, + pub base: TirRef, + pub span: Span, + pub branch: Option, +} + /// Per-`IfStmt` mapping from arm position to its assigned [`BranchId`]. /// Codegen uses this to push the right `BranchId` onto `branch_stack` /// as it lowers each arm, so a branch-gated `FreePoint` only fires @@ -118,6 +132,8 @@ pub struct FunctionSidecar { /// the arms it names (including a synthetic fall-through block for /// else-less ifs). pub conditional_dead_drops: Vec, + /// Promotion-buffer frees for borrowed-param view bases. + pub promotion_frees: Vec, } impl FunctionSidecar { @@ -135,6 +151,7 @@ impl FunctionSidecar { field_free_on_reassign: vec![None; arena_len], if_branches: vec![None; arena_len], conditional_dead_drops: Vec::new(), + promotion_frees: Vec::new(), } } } diff --git a/ryo-frontend/src/ownership/frees.rs b/ryo-frontend/src/ownership/frees.rs index ffd1b81..2d2fed1 100644 --- a/ryo-frontend/src/ownership/frees.rs +++ b/ryo-frontend/src/ownership/frees.rs @@ -284,6 +284,34 @@ pub(crate) fn warn_redundant_materialize( } } +/// Every `Return`/`ReturnVoid` statement in `stmts` (any depth), in +/// forward source order. The promotion-free return epilogue anchors a +/// free at each one, so an early-exit path cannot bypass a promotion +/// buffer's single normal anchor. +pub(crate) fn collect_return_stmts(tir: &Tir, stmts: &[TirRef], out: &mut Vec) { + for &r in stmts { + match tir.inst(r).tag { + TirTag::Return | TirTag::ReturnVoid => out.push(r), + TirTag::IfStmt => { + let view = tir.if_stmt_view(r); + collect_return_stmts(tir, &view.then_stmts, out); + for elif in &view.elif_branches { + collect_return_stmts(tir, &elif.body, out); + } + if let Some(else_stmts) = &view.else_stmts { + collect_return_stmts(tir, else_stmts, out); + } + } + TirTag::WhileLoop | TirTag::ForRange => { + if let Some(body) = tir.loop_body(r) { + collect_return_stmts(tir, &body, out); + } + } + _ => {} + } + } +} + /// Snapshot the owners still `Valid` at a return — values the function /// must destroy on that exit path (see `Ownership::return_epilogue`). /// The returned value itself is already `Moved` by `analyze_return`, diff --git a/ryo-frontend/src/ownership/mod.rs b/ryo-frontend/src/ownership/mod.rs index 025418e..83cee89 100644 --- a/ryo-frontend/src/ownership/mod.rs +++ b/ryo-frontend/src/ownership/mod.rs @@ -45,6 +45,7 @@ pub(crate) use walk::*; pub use ryo_core::ownership::{ BranchId, ConditionalDeadDrop, FreePoint, FunctionSidecar, IfBranchIds, OwnershipSidecar, + PromoFree, }; use ryo_core::tir::{ParamMode, Span, Tir, TirRef, TirTag}; use ryo_core::types::{InternPool, StringId, TypeId}; @@ -288,6 +289,16 @@ pub(crate) struct Ownership { /// the post-walk redundant-materialize pass to classify escapes of /// the copy and defensive-copy hazards on the view's root owner. pub owner_hazards: Vec<(Owner, TirRef)>, + + /// View-creating insts over borrowed-param bases, recorded by the + /// walk (`record_promo_candidate`). Consumed by the post-walk + /// promotion-free scheduling pass in `analyze_function`. + pub(crate) promo_candidates: Vec, + + /// The statement currently being walked; set (and restored) by + /// `analyze_stmt` so `record_promo_candidate` can anchor each + /// candidate to its enclosing statement. + pub(crate) current_stmt: Option, } impl Ownership { @@ -322,6 +333,21 @@ pub(crate) struct ReseatDrop { pub untouched_arms: Vec, } +/// A `Slice`/`ToView` instruction whose base is a borrowed `str`/`bytes` +/// parameter. Codegen promotes the callee's inline copy of the param to +/// heap for such bases; the post-walk pass schedules the free. +#[derive(Clone, Copy, Debug)] +pub(crate) struct PromoCandidate { + /// The `Slice`/`ToView` instruction. + pub(crate) view_inst: TirRef, + /// Its base operand (a `Var` of the borrowed param). + pub(crate) base: TirRef, + /// The statement being walked when the inst was visited: the + /// binding statement for bound views, the enclosing statement for + /// transient slices. + pub(crate) stmt: TirRef, +} + /// Validate move safety for every function body. Emits diagnostics /// into `sink`. Returns an [`OwnershipSidecar`] that codegen consults /// to decide where to emit `ryo_str_free` calls. The TIR itself is @@ -628,6 +654,160 @@ fn analyze_function( // codegen's inst_values won't have ptr/cap either). } + // Promotion-buffer frees for borrowed-param view bases: codegen + // promotes the callee's inline copy of the param to heap so the + // view addresses stable memory; the buffer is freed at the view's + // death. Only bases recorded by the walk qualify (borrowed, + // owner-typed, param-rooted). + let candidates = std::mem::take(&mut own.promo_candidates); + // (base, normal anchor) pairs, kept for the return epilogue below. + let mut promo_anchors: Vec<(TirRef, TirRef)> = Vec::with_capacity(candidates.len()); + for cand in candidates { + let rank = |r: TirRef| order.get(r.index()).copied().unwrap_or(0); + let bound = own.root_owner.contains_key(&Owner::Inst(cand.view_inst)); + let after = if bound { + if let Some(loop_ref) = Ownership::dense_get(&own.view_defer_loop, cand.view_inst) { + // Created outside a loop, last read inside it: dies at + // the loop's exit. The creation dominates the loop, so + // the scratch slot is initialized on every path here. + loop_ref + } else if let Some(lu) = Ownership::dense_get(&own.view_last_use, cand.view_inst) { + // P5: a reslice of this view (e.g. `w = s[0:2]; + // v = w[0:1]`) keeps the same promotion buffer alive — + // defer to the last use of any projection of this view, + // exactly as owner frees do. + let lu = defer_anchor( + lu, + &Owner::Inst(cand.view_inst), + &projections_of, + &last_use, + &order, + ); + // Conditional last use: re-anchor to the branch exit — + // but only when the view's creation dominates the + // branch, else the slot may be uninitialized on the + // skipped path (mirrors the declared-before check of + // the Owner::Param free path). + match outermost_branch_of(tir, lu) { + Some(branch_stmt) + if branch_may_not_return(tir, branch_stmt) + && rank(cand.view_inst) < rank(branch_stmt) => + { + branch_stmt + } + _ => lu, + } + } else if own + .loop_nesting + .ancestors_innermost_first(cand.stmt) + .next() + .is_some() + { + // Bound but never read, with the slice inside a loop. + // The liveness pre-pass's first-wins back-edge merge + // attributes in-loop reads of a loop-rebound view to + // the PRE-loop slice inst, leaving the in-loop slice + // with no recorded last use. Anchoring at the rebind + // statement fires every iteration, freeing the buffer + // the just-rebound view still points into. Anchoring + // at the enclosing loop's exit is equally unsound: the + // binding's slot holds the in-loop slice's final + // buffer, which a read after the loop still reaches — + // the loop-exit free releases it first (UAF). Anchor + // at the end of the function body instead, the same + // anchor the Owner::Param never-read path uses: views + // cannot escape the function, free-before-overwrite at + // the promotion site releases intermediate iterations, + // the entry-zeroed flag covers zero iterations, and + // the return-epilogue anchors below cover early exits. + match body_stmts.last().copied() { + Some(last) => last, + None => continue, + } + } else { + // Bound but never read, outside any loop: free right + // after the statement that created the slice. + cand.stmt + } + } else { + // Transient slice (e.g. `print(s[0:1])`): the projection + // dies with its enclosing statement. + cand.stmt + }; + sidecar.promotion_frees.push(PromoFree { + after, + base: cand.base, + span: tir.span(cand.view_inst), + branch: None, + }); + promo_anchors.push((cand.base, after)); + } + + // Return epilogue for promotion buffers, mirroring the owner + // return-epilogue pass below: each promo free above has exactly one + // anchor, so any path that returns before it leaks the buffer (the + // last use inside the return operand lands the anchor on a sub-inst + // the end-of-statement sweep skips on terminators; a loop-deferred + // anchor is bypassed by a `return` inside the loop). Anchor a copy + // of every candidate's free at every Return/ReturnVoid — the + // flag-conditional, flag-clearing emission makes the extra anchors + // harmless no-ops on paths that already freed (or never promoted) + // the buffer. Codegen's Return arms fire due promo frees before the + // `return_` terminator. Deduped against the candidate's normal + // anchor when that anchor IS the return statement. + let mut return_stmts: Vec = Vec::new(); + collect_return_stmts(tir, &body_stmts, &mut return_stmts); + let mut promo_epilogue_emitted: HashSet<(TirRef, TirRef)> = HashSet::new(); + for return_stmt in return_stmts { + for &(base, normal_anchor) in &promo_anchors { + if normal_anchor == return_stmt { + continue; + } + if !promo_epilogue_emitted.insert((return_stmt, base)) { + continue; + } + sidecar.promotion_frees.push(PromoFree { + after: return_stmt, + base, + span: tir.span(return_stmt), + branch: None, + }); + } + } + + // Fallthrough backstop: a body that can reach its end without an + // explicit return exits through codegen's synthesized return, which + // has no TIR statement for the return epilogue above to anchor on — + // e.g. a view whose only use is inside a returning if-arm keeps its + // in-arm anchor, and the not-taken path falls through with the + // promotion buffer still live. Anchor a copy of every candidate's + // free after the final body statement; flag-conditional emission + // keeps it a harmless no-op on paths that already freed (or never + // promoted) the buffer. + if let Some(&last) = body_stmts.last() { + let may_fall_through = match tir.inst(last).tag { + TirTag::Return | TirTag::ReturnVoid => false, + TirTag::IfStmt => if_may_fall_through(tir, last), + _ => true, + }; + if may_fall_through { + for &(base, normal_anchor) in &promo_anchors { + if normal_anchor == last { + continue; + } + if !promo_epilogue_emitted.insert((last, base)) { + continue; + } + sidecar.promotion_frees.push(PromoFree { + after: last, + base, + span: tir.span(last), + branch: None, + }); + } + } + } + // Dead-store survivors: emit W0001 and schedule a Free anchored // after the declaring instruction. Skip owners already covered by // `free_on_reassign` to avoid double-freeing the same allocation. diff --git a/ryo-frontend/src/ownership/walk.rs b/ryo-frontend/src/ownership/walk.rs index 224cd29..45e0b29 100644 --- a/ryo-frontend/src/ownership/walk.rs +++ b/ryo-frontend/src/ownership/walk.rs @@ -1,8 +1,8 @@ //! Forward statement/expression walk — split from `mod.rs`. use super::{ - BranchState, Owner, OwnerState, Ownership, ReseatDrop, analyze_for_range, analyze_while_loop, - check_field_move_out, check_field_target_projected, check_source_projected, + BranchState, Owner, OwnerState, Ownership, PromoCandidate, ReseatDrop, analyze_for_range, + analyze_while_loop, check_field_move_out, check_field_target_projected, check_source_projected, consume_struct_lit_fields, consumed_binding_name, drain_dying_views, field_path_of, format_binding, needs_tracking, owner_name_for_diag, owner_sort_key, param_idx, projection_root, prune_branch_dead_projections, push_unique, record_return_epilogue, @@ -13,7 +13,7 @@ use crate::builtins::{is_borrowed_scalar_param, view_borrow_params}; use ryo_core::diag::{Diag, DiagCode, DiagSink}; use ryo_core::ownership::{BranchId, FreePoint, FunctionSidecar, IfBranchIds}; use ryo_core::tir::{ParamMode, Span, Tir, TirData, TirRef, TirTag}; -use ryo_core::types::{InternPool, StringId}; +use ryo_core::types::{InternPool, StringId, TypeKind}; use std::collections::HashSet; pub(crate) fn analyze_stmt( @@ -24,6 +24,10 @@ pub(crate) fn analyze_stmt( sidecar: &mut FunctionSidecar, stmt: TirRef, ) { + // Anchor for `record_promo_candidate`: nested statements (if arms, + // loop bodies, loop-convergence re-walks) overwrite this, so save + // and restore the enclosing statement on the way out. + let prev_stmt = own.current_stmt.replace(stmt); let inst = *tir.inst(stmt); match inst.tag { TirTag::VarDecl => analyze_var_decl(tir, pool, own, sink, sidecar, stmt), @@ -131,6 +135,7 @@ pub(crate) fn analyze_stmt( // read and a consume within the same statement both see the view // as live. drain_dying_views(own); + own.current_stmt = prev_stmt; } /// Move-typed `VarDecl` is a consumer: the new binding takes @@ -1343,6 +1348,23 @@ pub(crate) fn visit_expr( TirTag::IfStmt => analyze_if_stmt(tir, pool, own, sink, sidecar, r), TirTag::WhileLoop => analyze_while_loop(tir, pool, own, sink, sidecar, r), TirTag::ForRange => analyze_for_range(tir, pool, own, sink, sidecar, r), + // ---- View-creating instructions over borrowed params ---- + // Same operand recursion as the catch-all (including its + // `check_use_moved` calls); additionally record the inst when + // its base is a borrowed `str`/`bytes` parameter so the + // promotion-free pass can schedule the buffer's release. + TirTag::Slice => { + recurse_operands(tir, pool, own, sink, sidecar, r); + if let TirData::Slice { base, .. } = inst.data { + record_promo_candidate(tir, pool, own, r, base); + } + } + TirTag::ToView => { + recurse_operands(tir, pool, own, sink, sidecar, r); + if let TirData::UnOp(operand) = inst.data { + record_promo_candidate(tir, pool, own, r, operand); + } + } // ---- Everything else: recurse on operands so nested // ---- producers/aliases are still observed. _ => { @@ -1351,6 +1373,50 @@ pub(crate) fn visit_expr( } } +/// Record a `Slice`/`ToView` whose base is a borrowed `str`/`bytes` +/// param. Only owner-typed bases promote (views pass through), and +/// only borrowed params leak the promotion buffer. +fn record_promo_candidate( + tir: &Tir, + pool: &InternPool, + own: &mut Ownership, + view_inst: TirRef, + base: TirRef, +) { + let base_inst = tir.inst(base); + if !matches!(pool.kind(base_inst.ty), TypeKind::Str | TypeKind::Bytes) { + return; + } + let TirData::Var(name) = base_inst.data else { + return; + }; + // The name must currently resolve to the param itself, not a + // shadowing local. + if !matches!(own.current_owner.get(&name), Some(Owner::Param(_))) { + return; + } + let Some(&idx) = own.param_index.get(&name) else { + return; + }; + if tir.params[idx].mode != ParamMode::Borrow { + return; + } + let Some(stmt) = own.current_stmt else { return }; + // Loop convergence re-walks insts; record each view inst once. + if own + .promo_candidates + .iter() + .any(|c| c.view_inst == view_inst) + { + return; + } + own.promo_candidates.push(PromoCandidate { + view_inst, + base, + stmt, + }); +} + pub(crate) fn recurse_operands( tir: &Tir, pool: &InternPool, diff --git a/ryo/tests/common/mod.rs b/ryo/tests/common/mod.rs index 3602cc4..6f4c544 100644 --- a/ryo/tests/common/mod.rs +++ b/ryo/tests/common/mod.rs @@ -556,6 +556,141 @@ fn main(): \tmut p = Person{name=int_to_str(42)} \tp = p \tprint(p.name) +", + ), + ( + // Slicing a borrowed str param whose argument is inline (SSO) + // promotes a heap buffer that must be freed. + "slice_borrowed_param_inline", + "\ +fn scan(s: str): +\tv = s[0:1] +\tprint(v) + +fn main(): +\tx: str = int_to_str(7) +\tscan(x) +\tscan(x) +", + ), + ( + // Heap argument (> 23 B): promotion is a no-op pass-through; + // the scheduled free must not touch the caller's buffer. + "slice_borrowed_param_heap", + "\ +fn scan(s: str): +\tv = s[0:2] +\tprint(v) + +fn main(): +\tx: str = int_to_str(123456789) +\ty: str = x + x + x + x +\tscan(y) +", + ), + ( + // The view's last use is inside the return operand, so the + // free's anchor lands on a sub-inst of the Return — and the + // end-of-statement sweep is skipped on terminators. Only the + // return-epilogue promo free releases the promotion buffer on + // this path. `print(int_to_str(...))` proves the slice executed + // (a panic would mask the leak under Valgrind). + "slice_borrowed_param_return_last_use", + "\ +fn scan(s: str) -> int: +\tv = s[0:2] +\treturn v.len() + +fn main(): +\tx: str = int_to_str(654321) +\tprint(int_to_str(scan(x))) +", + ), + ( + // Loop-deferred view (created before the loop, read inside it) + // with a `return` inside the loop: the loop-exit anchor is + // bypassed on the return path — only the return-epilogue promo + // free releases the promotion buffer. The in-loop `print(v)` + // proves the slice executed. + "slice_borrowed_param_return_in_loop", + "\ +fn scan(s: str) -> int: +\tv = s[0:2] +\tfor i in range(0, 4): +\t\tprint(v) +\t\treturn 1 +\treturn 0 + +fn main(): +\tx: str = int_to_str(654321) +\tscan(x) +", + ), + ( + // View declared before the loop, rebound inside it, read only + // after it: the in-loop slice gets no recorded last use (the + // liveness pre-pass attributes the post-loop read to the + // pre-loop slice), so its promo free falls to the + // bound-never-read fallback. Anchoring that fallback at the + // loop exit releases the final iteration's buffer right before + // the post-loop read — Valgrind flags the read as a + // use-after-free. The post-loop `print(v)` (prints `4`) proves + // the slice path executed. + "slice_borrowed_param_rebind_loop_read_after", + "\ +fn scan(s: str): +\tmut v = s[0:1] +\tfor i in range(0, 3): +\t\tv = s[i:i+1] +\tprint(v) + +fn main(): +\tx: str = int_to_str(654321) +\tscan(x) +", + ), + ( + // View declared before the loop and rebound inside it, with + // the read BEFORE the rebind: the in-loop slice's promotion + // buffer must survive until the loop exit — freeing it at the + // rebind statement releases the buffer the just-rebound view + // points into (the next iteration's read is a use-after-free). + // The in-loop `print(v)` (prints `65655443`) proves the slice + // path executed. + "slice_borrowed_param_rebind_loop", + "\ +fn scan(s: str): +\tmut v = s[0:2] +\tfor i in range(0, 4): +\t\tprint(v) +\t\tv = s[i:i+2] + +fn main(): +\tx: str = int_to_str(654321) +\tscan(x) +", + ), + ( + // View created before an if whose only use is inside a + // returning arm: the conditional-last-use re-anchor refuses a + // branch whose arm returns, so the normal anchor stays in-arm + // and the not-taken path falls through to the function's + // synthesized return with the promotion buffer still live. + // Only the fallthrough backstop anchor releases it on that + // path. The final `print` (prints `done`) proves the + // fallthrough path executed. + "slice_borrowed_param_last_use_in_returning_arm", + "\ +fn scan(cond: bool, s: str): +\tv = s[0:1] +\tif cond: +\t\tprint(v) +\t\treturn + +fn main(): +\tx: str = int_to_str(654321) +\tscan(false, x) +\tprint(\"done\") ", ), ]; diff --git a/ryo/tests/integration_views.rs b/ryo/tests/integration_views.rs index acb0002..c5af758 100644 --- a/ryo/tests/integration_views.rs +++ b/ryo/tests/integration_views.rs @@ -46,6 +46,111 @@ fn test_slice_of_borrowed_param_ok() { ); } +#[test] +fn test_slice_of_borrowed_param_then_read_param() { + // Reading the param after the view dies must still see the + // original (inline) value — the promoted buffer is freed, the + // param itself is not. + assert_ryo_output( + "slice_param_then_read.ryo", + "fn scan(s: str):\n\tv = s[0:1]\n\tprint(v)\n\tprint(s)\n\nfn main():\n\tx: str = int_to_str(7)\n\tscan(x)\n", + "77", + ); +} + +#[test] +fn test_slice_of_borrowed_param_in_loop() { + // Fresh view per loop iteration: each iteration's promotion buffer + // is freed at that iteration's last use. + assert_ryo_output( + "slice_param_loop.ryo", + "fn scan(s: str):\n\tfor i in range(0, 3):\n\t\tv = s[i:i+1]\n\t\tprint(v)\n\nfn main():\n\tscan(\"abc\")\n", + "abc", + ); +} + +#[test] +fn test_slice_of_borrowed_param_transient() { + assert_ryo_output( + "slice_param_transient.ryo", + "fn scan(s: str):\n\tprint(s[1:3])\n\nfn main():\n\tscan(\"abc\")\n", + "bc", + ); +} + +#[test] +fn test_reslice_of_borrowed_param_view() { + // A reslice keeps the same promotion buffer alive past the first + // view's last use — the free must defer to the reslice's last use. + assert_ryo_output( + "reslice_param_view.ryo", + "fn scan(s: str):\n\tw = s[0:3]\n\tv = w[1:3]\n\tprint(v)\n\nfn main():\n\tscan(\"abc\")\n", + "bc", + ); +} + +#[test] +fn test_slice_of_borrowed_param_rebound_in_loop() { + // View declared before the loop and rebound inside it: the in-loop + // slice's promotion buffer must stay alive across the rebind — the + // free defers to the loop exit, not the rebind statement. + assert_ryo_output( + "slice_param_rebind_loop.ryo", + "fn scan(s: str):\n\tmut v = s[0:2]\n\tfor i in range(0, 4):\n\t\tprint(v)\n\t\tv = s[i:i+2]\n\nfn main():\n\tx: str = int_to_str(654321)\n\tscan(x)\n", + "65655443", + ); +} + +#[test] +fn test_slice_of_borrowed_param_rebound_in_loop_read_after() { + // Same rebind shape, but the view is read AFTER the loop: the final + // iteration's promotion buffer must survive to that read. The + // argument is runtime-built — a static literal never promotes, so + // the free-at-loop-exit UAF would not trigger. + assert_ryo_output( + "slice_param_rebind_loop_after.ryo", + "fn scan(s: str):\n\tmut v = s[0:1]\n\tfor i in range(0, 3):\n\t\tv = s[i:i+1]\n\tprint(v)\n\nfn main():\n\tx: str = int_to_str(654321)\n\tscan(x)\n", + "4", + ); +} + +#[test] +fn test_slice_of_borrowed_param_return_last_use() { + // The view's last use is inside the return operand: only the + // return-epilogue promo free can release the promotion buffer + // (the end-of-statement sweep is skipped on terminators). + assert_ryo_output( + "slice_param_return_last_use.ryo", + "fn scan(s: str) -> int:\n\tv = s[0:2]\n\treturn v.len()\n\nfn main():\n\tx: str = int_to_str(654321)\n\tprint(int_to_str(scan(x)))\n", + "2", + ); +} + +#[test] +fn test_slice_of_borrowed_param_return_in_loop() { + // Loop-deferred view with a `return` inside the loop: the + // loop-exit anchor is bypassed on the return path — only the + // return-epilogue promo free releases the buffer. + assert_ryo_output( + "slice_param_return_in_loop.ryo", + "fn scan(s: str) -> int:\n\tv = s[0:2]\n\tfor i in range(0, 4):\n\t\tprint(v)\n\t\treturn 1\n\treturn 0\n\nfn main():\n\tx: str = int_to_str(654321)\n\tscan(x)\n", + "65", + ); +} + +#[test] +fn test_slice_of_borrowed_param_last_use_in_returning_arm() { + // The view's only use is inside a returning if-arm: the not-taken + // path falls through to the function's synthesized return, where + // only the fallthrough backstop promo free can release the + // promotion buffer (the in-arm anchor never fires). + assert_ryo_output( + "slice_param_arm_fallthrough.ryo", + "fn scan(cond: bool, s: str):\n\tv = s[0:1]\n\tif cond:\n\t\tprint(v)\n\t\treturn\n\nfn main():\n\tx: str = int_to_str(654321)\n\tscan(false, x)\n\tprint(\"done\")\n", + "done", + ); +} + #[test] fn test_slice_empty() { assert_ryo_runs( diff --git a/ryo/tests/valgrind_smoke.rs b/ryo/tests/valgrind_smoke.rs index b901e8e..c9c27d5 100644 --- a/ryo/tests/valgrind_smoke.rs +++ b/ryo/tests/valgrind_smoke.rs @@ -101,6 +101,62 @@ fn valgrind_int_to_str_then_print() { ); } +#[test] +fn valgrind_slice_borrowed_param_inline() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_inline"), + "slice_borrowed_param_inline", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_heap() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_heap"), + "slice_borrowed_param_heap", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_return_last_use() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_return_last_use"), + "slice_borrowed_param_return_last_use", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_return_in_loop() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_return_in_loop"), + "slice_borrowed_param_return_in_loop", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_rebind_loop_read_after() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_rebind_loop_read_after"), + "slice_borrowed_param_rebind_loop_read_after", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_rebind_loop() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_rebind_loop"), + "slice_borrowed_param_rebind_loop", + ); +} + +#[test] +fn valgrind_slice_borrowed_param_last_use_in_returning_arm() { + run_valgrind_smoke( + common::find_fixture("slice_borrowed_param_last_use_in_returning_arm"), + "slice_borrowed_param_last_use_in_returning_arm", + ); +} + #[test] fn valgrind_mut_reassign() { run_valgrind_smoke(common::find_fixture("mut_reassign"), "mut_reassign");