Skip to content

Commit e02c21e

Browse files
committed
fix(effect): embed Resume<T> scratch in state machine, drop per-perform malloc
cd3184a fixed the j3 stack-lifetime bug by switching the Resume<T> struct from `Alloca` to `Intrinsic::Malloc(32)`. Correct but unbounded: every perform-site invocation allocated 32 bytes with no free path. Long-running programs with hot perform sites would accumulate the allocations forever. Reserve a 4-slot region (= 32 bytes, the exact Resume<T> size) at the end of the state machine. `lower_async_function` now allocates `num_slots + 4` and returns `resume_scratch_slot` (the base slot index) in `LowerResult`. `upgrade_resume_struct_at_perform_sites` takes that slot, computes `r_ptr = sm_ptr + (slot * 8)`, and writes the four Resume<T> fields there instead of malloc'ing. The scratch's lifetime matches the state machine's — already heap- allocated by `generate_sync_entry`'s malloc, never freed today. So no new free path is needed, and the Resume<T> outlives any out-of-line capture (j3 still passes — the stashed pointer points into the SM, valid as long as the SM is alive). Memory cost drops from O(perform- site invocations) to O(state-machine instantiations). Known limitation (documented inline): all perform sites in the same function share this one scratch region. Nested or interleaved performs with async stashing — host holds k from perform A while perform B's handler runs — would alias the slot and corrupt the stashed pointer. Current tests all have a single perform site per function; extending to N per-perform scratches (one slot region per site) is a future refinement when multi-perform-with-stash programs land. Workspace `cargo test --tests` = 1439/0; effect runtime tests all 10/10; clippy + fmt clean.
1 parent cd3184a commit e02c21e

3 files changed

Lines changed: 64 additions & 36 deletions

File tree

crates/passes/krio_adapter/src/abi_emit.rs

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,7 @@ pub fn upgrade_resume_struct_at_perform_sites(
11011101
handler_resolution: &std::collections::HashMap<(HirId, InternedString), HirId>,
11021102
poll_fn_id: HirId,
11031103
frame_ptr: HirId,
1104+
resume_scratch_slot: u32,
11041105
) {
11051106
// First pass: collect (block_id, inst_idx, metadata) without
11061107
// mutating to avoid borrow conflicts.
@@ -1169,49 +1170,47 @@ pub fn upgrade_resume_struct_at_perform_sites(
11691170
// Mint values + build the new instruction sequence.
11701171
let mut new_insts: Vec<HirInstruction> = Vec::new();
11711172

1172-
// r_ptr = malloc(32) — 32-byte heap slot.
1173+
// r_ptr = sm_ptr + resume_scratch_slot * 8.
11731174
//
1174-
// Resume<T> structs need to outlive the perform site's stack
1175-
// frame in the async out-of-line case (phase J.3): the handler
1176-
// stashes `k` (= this pointer) somewhere reachable from outside
1177-
// poll_fn, and the host later calls `__zyntax_effect_resume(k, v)`
1178-
// out-of-line. If `r_ptr` is an Alloca (stack slot), the pointer
1179-
// is dangling the moment poll_fn returns — every subsequent
1180-
// dereference reads whatever the runtime stack scribbled over
1181-
// the old slot.
1175+
// The Resume<T> struct (4 i64 fields = 32 bytes) lives in a
1176+
// fixed 4-slot region reserved at the end of the state
1177+
// machine by `orchestrator::lower_async_function`. Embedding
1178+
// it in the SM rather than malloc'ing per perform-site
1179+
// invocation eliminates the per-perform 32-byte leak the
1180+
// previous `Intrinsic::Malloc(32)` introduced — the scratch's
1181+
// lifetime matches the SM's (already heap-allocated, never
1182+
// freed today), so no new free path is needed.
11821183
//
1183-
// On macOS / Linux x86_64 the stack region below the old rsp
1184-
// happens to survive long enough for synthetic test harness
1185-
// paths to read the stale-but-intact Resume struct. On
1186-
// x86_64-pc-windows-msvc the intervening Rust frames between
1187-
// poll_fn's return and the out-of-line invocation overwrite
1188-
// the old slot — typically clearing `next_state` to 0, which
1189-
// routes the dispatcher's Switch to `default = resume_entries[0]`
1190-
// (the perform site) instead of the resume entry, producing
1191-
// wrong results for `phase_j3_async_out_of_line_resume`.
1184+
// Resume<T> structs must outlive the perform site's stack
1185+
// frame in the async out-of-line case (phase J.3): the
1186+
// handler stashes `k` (= this pointer) somewhere reachable
1187+
// from outside poll_fn, and the host later calls
1188+
// `__zyntax_effect_resume(k, v)` out-of-line. An Alloca
1189+
// (stack slot) would dangle the moment poll_fn returns; a
1190+
// malloc'd buffer works but leaks. The SM-embedded slot
1191+
// gives us the outlive semantics with no extra allocation.
11921192
//
1193-
// Heap-allocate to make every perform site's Resume<T> live as
1194-
// long as someone has a pointer to it. The 32 bytes leak on
1195-
// every perform (no free hook yet) — bounded by the program's
1196-
// total perform-site execution count; a future Tier 3 refinement
1197-
// can refcount or arena-collect.
1193+
// Limitation: all perform sites in this function share this
1194+
// one scratch region. Nested or interleaved performs with
1195+
// async stashing (host holds k from perform A while perform
1196+
// B's handler runs) would alias the slot and corrupt the
1197+
// stashed pointer. Current tests all have a single perform
1198+
// site per function; extending to N per-perform scratches is
1199+
// a future refinement when multi-perform-with-stash programs
1200+
// land.
1201+
let scratch_offset_bytes = (resume_scratch_slot as i64) * 8;
1202+
let scratch_offset_id = mint_const_i64(&mut function.values, scratch_offset_bytes);
11981203
let r_ptr_id = mint_value(
11991204
&mut function.values,
12001205
HirType::Ptr(Box::new(HirType::U8)),
12011206
HirValueKind::Instruction,
12021207
);
1203-
let r_size_id = mint_value(
1204-
&mut function.values,
1205-
HirType::I64,
1206-
HirValueKind::Constant(HirConstant::I64(32)),
1207-
);
1208-
new_insts.push(HirInstruction::Call {
1209-
result: Some(r_ptr_id),
1210-
callee: HirCallable::Intrinsic(Intrinsic::Malloc),
1211-
args: vec![r_size_id],
1212-
type_args: vec![],
1213-
const_args: vec![],
1214-
is_tail: false,
1208+
new_insts.push(HirInstruction::Binary {
1209+
result: r_ptr_id,
1210+
op: BinaryOp::Add,
1211+
ty: HirType::I64,
1212+
left: frame_ptr,
1213+
right: scratch_offset_id,
12151214
});
12161215

12171216
// r_poll_fn_ptr = CreateClosure(poll_fn_id) (self-reference).

crates/passes/krio_adapter/src/orchestrator.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,24 @@ pub struct LowerResult {
9696
/// Used by the entry function to size the malloc. Computed as
9797
/// `max(captures_slot, state_slot, param_slots) + 1`.
9898
pub num_slots: u32,
99+
/// Base slot index of the 4-slot region reserved at the END of the
100+
/// state machine for the Resume<T> struct scratch space. The
101+
/// algebraic-effects perform-site lowering writes its 32-byte
102+
/// Resume<T> (poll_fn_ptr / state_machine_ptr / result_slot_offset
103+
/// / next_state) into `sm_ptr + resume_scratch_slot * 8` rather
104+
/// than calling malloc(32) per perform-site invocation. The struct
105+
/// lives as long as the state machine does — no per-perform heap
106+
/// allocation, no leak.
107+
///
108+
/// Limitation: all perform sites in the same function share this
109+
/// one scratch region. Nested or interleaved performs with async
110+
/// stashing (host holds k from perform A while perform B's
111+
/// handler runs) would alias the slot and corrupt the stashed
112+
/// pointer. Current tests all have a single perform site per
113+
/// function; extending to N per-perform scratches (one per site)
114+
/// is a future refinement when multi-perform-with-stash programs
115+
/// land.
116+
pub resume_scratch_slot: u32,
99117
}
100118

101119
/// Errors the orchestrator may surface.
@@ -245,7 +263,16 @@ pub fn lower_async_function(
245263
state_slot,
246264
after_awaits_slot,
247265
);
248-
let num_slots: u32 = after_performs_slot;
266+
// Reserve 4 slots at the end for the Resume<T> struct scratch
267+
// space (4 i64 fields: poll_fn_ptr, state_machine_ptr,
268+
// result_slot_offset, next_state). Embedding it in the state
269+
// machine instead of malloc'ing per perform-site invocation
270+
// eliminates the bounded-but-unbounded leak the previous
271+
// `Intrinsic::Malloc(32)` introduced — the scratch's lifetime
272+
// matches the SM's (already heap-allocated, never freed today),
273+
// so no new free path is needed.
274+
let resume_scratch_slot: u32 = after_performs_slot;
275+
let num_slots: u32 = after_performs_slot + 4;
249276

250277
// F.2 follow-up: the await lowering creates new resume blocks
251278
// that may now be predecessors of phi blocks (e.g. loop headers
@@ -270,6 +297,7 @@ pub fn lower_async_function(
270297
param_slots,
271298
state_slot,
272299
num_slots,
300+
resume_scratch_slot,
273301
})
274302
}
275303

crates/zyntax_embed/src/runtime.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,7 @@ fn apply_krio_effect_lowering(module: &mut zyntax_compiler::HirModule) -> Runtim
959959
&handler_resolution,
960960
new_poll_id,
961961
post_reshape_frame_ptr,
962+
lower_result.resume_scratch_slot,
962963
);
963964

964965
// Phase 3: SYNC entry wrapper (returns T, not *Promise<T>).

0 commit comments

Comments
 (0)