From ecde196c8442ee7c61bfff7c82cd624306238b0b Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sun, 26 Jul 2026 04:18:56 +0900 Subject: [PATCH] exc: mark the traceback's frame escaped on app-level reads Upstream issues `tb.frame.mark_as_escaped()` from two getters, `descr_gettraceback` and `OperationError.get_traceback`, while direct `_application_traceback` reads do not. pyre had it from neither. Add `pytraceback::mark_traceback_escaped` -- the frame type is not visible from pyre-object, so it cannot sit beside the slot reader -- and call it from the seven sites that mirror one of the two getters: the `__traceback__` getattr arm, the attribute fold, `record_application_traceback`, the tracer hand-off in `exception_trace`, `sys.exc_info`, and both `write_unraisable` reads. The printing, chain-trimming and metadata-compare readers mirror direct slot reads and are left alone. The fold lowers the read to a raw slot load, so the specializer pairs it with a `CallN` to an `extern "C"` entry carrying `cannot_raise_effect_info`, and applies the mark concretely on the walk. This restores the upstream contract; it does not change any observed value. The `f_lineno` divergence it was written for survives, and a control proves why: `tb.tb_frame`'s getter has always marked the frame, and marking from inside the handler -- at either site -- has no effect, while marking before the `try` does. Also correct the `PyTraceback.frame` note that forbids dereferencing the pointer: `pytraceback_object_custom_trace` forwards that edge and frames are GC-owned, so `tb_frame` is live for the traceback's lifetime. check.py dynasm 322/322 + cranelift 322/322; lib tests 400/326/294. Assisted-by: Claude --- pyre/pyre-interpreter/src/baseobjspace.rs | 11 ++-- pyre/pyre-interpreter/src/error.rs | 6 +++ pyre/pyre-interpreter/src/eval.rs | 3 ++ pyre/pyre-interpreter/src/module/sys/vm.rs | 10 ++-- pyre/pyre-interpreter/src/pytraceback.rs | 53 +++++++++++++++++-- .../src/jitcode_dispatch/specialize.rs | 13 +++++ pyre/pyre-object/src/interp_exceptions.rs | 16 +++--- 7 files changed, 89 insertions(+), 23 deletions(-) diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index e2af832658a..46e9b316694 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -6494,9 +6494,11 @@ fn object_getattr_miss(obj: PyObjectRef, name: &str, call_getattr: bool) -> PyRe // returns the `w_traceback` slot stamped by // `descr_settraceback` and the `raise` machinery's // `record_application_traceback`; `None` when none has - // been set. + // been set. The traceback reaches app level here, so its + // frame is marked escaped. let stored = unsafe { pyre_object::interp_exceptions::w_exception_get_traceback(obj) }; + unsafe { crate::pytraceback::mark_traceback_escaped(stored) }; return Ok(if stored.is_null() { w_none() } else { stored }); } "__cause__" => { @@ -9610,10 +9612,9 @@ pub unsafe fn exception_attr_slot_fold( ExceptionAttrSlot::Filename2 } // `__traceback__` is a `W_BaseException` slot on every exception kind. - // `descr_gettraceback` also calls `tb.frame.mark_as_escaped()`, which - // `w_exception_get_traceback` omits while the `ExecutionContext::leave` - // vref force it feeds is unported; the fold tracks that getter, so both - // sides gain the mark together. + // `descr_gettraceback` also calls `tb.frame.mark_as_escaped()`; the + // fold folds only the slot read, so the specializer pairs it with a + // residual call that issues the mark. "__traceback__" => ExceptionAttrSlot::Traceback, // `name` shares the `w_exc_name` slot across the kinds whose getattr // arm reads it; other kinds keep the regular attribute fall-through. diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index 31356f1c953..b895e970c0e 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -1110,11 +1110,14 @@ impl PyError { if w_type.is_null() { w_type = pyre_object::w_none(); } + // `error.py:271` — `w_tb = self.get_w_traceback(space)`, the slot + // read with its escape mark. let mut w_tb = if !w_value.is_null() && unsafe { pyre_object::is_exception(w_value) } { unsafe { pyre_object::interp_exceptions::w_exception_get_traceback(w_value) } } else { pyre_object::PY_NULL }; + unsafe { crate::pytraceback::mark_traceback_escaped(w_tb) }; if w_tb.is_null() { w_tb = pyre_object::w_none(); } @@ -1162,6 +1165,8 @@ impl PyError { if w_type.is_null() { w_type = pyre_object::w_none(); } + // `error.py:312` — same `get_w_traceback` read + // for the exception the hook itself raised. w_tb = if !hook_value.is_null() && unsafe { pyre_object::is_exception(hook_value) } { @@ -1173,6 +1178,7 @@ impl PyError { } else { pyre_object::PY_NULL }; + unsafe { crate::pytraceback::mark_traceback_escaped(w_tb) }; if w_tb.is_null() { w_tb = pyre_object::w_none(); } diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 01e951b4dea..f5e1118a3dd 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -1432,7 +1432,10 @@ pub fn handle_exception(frame: &mut PyFrame, err: &mut PyError, next_instr: &mut // null `w_type` makes `normalize_exception` take `w_inst = w_type` // (null) and raise "exceptions must derive from BaseException". let operr_obj = err.to_exc_object(); + // `executioncontext.py:362` hands the tracer + // `operr.get_w_traceback(space)` — the slot read with its mark. let w_tb = unsafe { pyre_object::interp_exceptions::w_exception_get_traceback(operr_obj) }; + unsafe { crate::pytraceback::mark_traceback_escaped(w_tb) }; if let Err(trace_err) = unsafe { (*ec).exception_trace(frame as *mut PyFrame, operr_obj, pyre_object::PY_NULL, w_tb) } { diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index 4ee74eae575..79e172aa097 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -555,11 +555,13 @@ pub fn exc_info_direct() -> PyObjectRef { exc_type }; // The third tuple slot mirrors - // `space.exception_gettraceback(operror)` - // (`error.py:140-145`). Pyre stores the chain on the - // typed `w_traceback` slot of `W_BaseException` - // (`interp_exceptions.rs:303`); surface it directly here. + // `vm.py:147-153 exc_info_with_tb`'s + // `operror.get_w_traceback(space)`, i.e. the slot read plus + // the escape mark it wraps. Pyre stores the chain on the + // typed `w_traceback` slot of `W_BaseException`; surface it + // directly here. let tb = pyre_object::interp_exceptions::w_exception_get_traceback(exc); + crate::pytraceback::mark_traceback_escaped(tb); let w_tb = if tb.is_null() { w_none() } else { tb }; w_tuple_new(vec![exc_type, exc, w_tb]) } diff --git a/pyre/pyre-interpreter/src/pytraceback.rs b/pyre/pyre-interpreter/src/pytraceback.rs index 6688b5ce41b..8a337aea54a 100644 --- a/pyre/pyre-interpreter/src/pytraceback.rs +++ b/pyre/pyre-interpreter/src/pytraceback.rs @@ -39,11 +39,11 @@ pub struct PyTraceback { pub ob_header: PyObject, /// `pytraceback.py:29 self.frame = frame` — opaque `*mut PyFrame`. /// Not a `PyObjectRef` because `PyFrame` has no `PyObject` header. - /// **Dangling-safe note**: by the time a traceback escapes the - /// raising frame (the common case — `except` block in a caller), - /// the original `PyFrame` allocation has been freed. Readers - /// MUST NOT dereference `frame`; metadata that survives the - /// frame's lifetime is snapshotted into the `w_code` slot below. + /// Frames are GC-owned non-moving blocks and + /// `pytraceback_object_custom_trace` forwards this edge, so the + /// pointer stays live for the traceback's whole lifetime and + /// `tb_frame` is safe to dereference. The `w_code` snapshot below + /// still exists for readers that run without the GC hook wired. pub frame: *mut crate::pyframe::PyFrame, /// `pytraceback.py:30 self.lasti = lasti` — bytecode index at /// which the exception was raised (in instruction units). @@ -263,6 +263,46 @@ pub unsafe fn w_pytraceback_get_lineno(tb: PyObjectRef) -> i64 { } } +/// The side effect shared by `error.py:359-370 OperationError.get_traceback` +/// and `interp_exceptions.py:195-200 descr_gettraceback`: a traceback that +/// becomes reachable by app-level code marks its frame escaped, so +/// `ExecutionContext::leave` forces that frame's vref and the frame stays +/// inspectable after the JIT frame is gone. +/// +/// This has no effect once several frames are recorded on the chain — those +/// are already marked by `leave` running with `got_exception` set. What it +/// covers is the single-frame chain: a `raise` caught in the frame that +/// raised it, where no callee ever left, whose traceback then outlives a +/// normal return. Without the mark such a frame keeps the vable shadow of +/// `last_instr` the JIT never wrote back, and `f_lineno` reads the `def` +/// line instead of the raising line. +/// +/// # Safety +/// `w_traceback` must be `PY_NULL` or point to a valid object. +pub unsafe fn mark_traceback_escaped(w_traceback: PyObjectRef) { + unsafe { + if w_traceback.is_null() || !is_pytraceback(w_traceback) { + return; + } + let frame = w_pytraceback_get_frame(w_traceback); + if !frame.is_null() { + (*frame).mark_as_escaped(); + } + } +} + +/// `extern "C"` entry for the residual call the `__traceback__` attribute +/// fold emits. Compiled code folds that read to a raw slot load, so the +/// getter's escape mark has to be issued separately; the fold pairs the +/// load with a call here. +/// +/// Cannot raise, allocates nothing, and writes only the frame's status +/// byte, which no field descriptor exposes to the trace — so the call +/// carries `cannot_raise_effect_info` and invalidates no heap cache entry. +pub extern "C" fn jit_mark_traceback_escaped(w_traceback: i64) { + unsafe { mark_traceback_escaped(w_traceback as usize as PyObjectRef) }; +} + /// `pytraceback.py:104-109 record_application_traceback` parity: /// /// ```python @@ -340,7 +380,10 @@ pub unsafe fn record_application_traceback( crate::pyframe::offset2lineno(&*code_obj, last_instruction as isize) as i64 } }; + // `tb = operror.get_traceback()` — the read that grows the chain + // marks the previous head's frame, matching `get_traceback`. let prev_tb = pyre_object::interp_exceptions::w_exception_get_traceback(w_exc_object); + mark_traceback_escaped(prev_tb); let new_tb = w_pytraceback_new(frame, last_instruction, prev_tb, lineno, w_code); pyre_object::interp_exceptions::w_exception_set_traceback(w_exc_object, new_tb); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 211dd1092a6..68562dac7c0 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -1419,6 +1419,19 @@ pub(crate) fn try_walker_specialize_load_attr( raw_value, majit_ir::Value::Ref(majit_ir::GcRef(stored as usize)), ); + if slot == pyre_interpreter::baseobjspace::ExceptionAttrSlot::Traceback { + // The fold replaces `descr_gettraceback`, whose read marks the + // traceback's frame escaped so `ExecutionContext::leave` forces + // its vref. The walk is the authoritative execution path, so + // mark now as well as on every compiled re-execution. + unsafe { pyre_interpreter::pytraceback::mark_traceback_escaped(stored) }; + ctx.trace_ctx.call_void_typed_with_effect( + pyre_interpreter::pytraceback::jit_mark_traceback_escaped as *const (), + &[raw_value], + &[majit_ir::Type::Ref], + majit_metainterp::cannot_raise_effect_info(), + ); + } let value = if slot == pyre_interpreter::baseobjspace::ExceptionAttrSlot::Args { let list = unsafe { &*(stored as *const pyre_object::listobject::W_ListObject) }; if list.strategy != pyre_object::listobject::ListStrategy::Object { diff --git a/pyre/pyre-object/src/interp_exceptions.rs b/pyre/pyre-object/src/interp_exceptions.rs index 7854bf4f915..09ae8e24ad7 100644 --- a/pyre/pyre-object/src/interp_exceptions.rs +++ b/pyre/pyre-object/src/interp_exceptions.rs @@ -810,15 +810,13 @@ pub unsafe fn w_exception_set_context(obj: PyObjectRef, value: PyObjectRef) { } } -/// `interp_exceptions.py:196-201 descr_gettraceback` parity, minus the -/// `tb.frame.mark_as_escaped()` side effect. `PyFrame::mark_as_escaped` -/// exists, but the flag's only consumer is `ExecutionContext::leave`, whose -/// `frame_vref()` force stays a no-op until `jit.virtual_ref` is ported — -/// and a frame a traceback can reach has already left, with `got_exception` -/// set, so `leave` marked its `f_back` either way. The mark belongs with -/// that force, together with the `__traceback__` arm of -/// `baseobjspace::exception_attr_slot_fold`, which folds this read to a raw -/// slot load in traced code. +/// The raw `self.w_traceback` slot read. `descr_gettraceback` +/// (`interp_exceptions.py:196-201`) and `OperationError.get_traceback` +/// (`error.py:359-370`) are this read plus `tb.frame.mark_as_escaped()`; +/// that mark lives in `pytraceback::mark_traceback_escaped`, since the +/// frame type is not visible from this crate. Callers mirroring either +/// getter pair the two; callers mirroring a direct `_application_traceback` +/// read (printing, chain trimming) use this alone. /// /// # Safety /// `obj` must point to a valid `W_BaseException`.