Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions pyre/pyre-interpreter/src/baseobjspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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__" => {
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions pyre/pyre-interpreter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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) }
{
Expand All @@ -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();
}
Expand Down
3 changes: 3 additions & 0 deletions pyre/pyre-interpreter/src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
} {
Expand Down
10 changes: 6 additions & 4 deletions pyre/pyre-interpreter/src/module/sys/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
Expand Down
53 changes: 48 additions & 5 deletions pyre/pyre-interpreter/src/pytraceback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the escape mark in the IR traceback prepend

When the full-body walker takes record_prepend_application_traceback for a nonconstant exception with a materialized frame, pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs:805-809 bypasses this function and lowers get_traceback() to only a GETFIELD_GC_R; it emits neither the concrete escape mark nor the residual marking call added for the attribute fold. Consequently, optimized JIT exception paths can still grow a traceback chain without marking the previous head's frame escaped, retaining the interpreter/JIT divergence this change is intended to remove. Pair that inline slot read with the same mark used by the opaque path.

AGENTS.md reference: AGENTS.md:L194-L196

Useful? React with 👍 / 👎.

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);
}
Expand Down
13 changes: 13 additions & 0 deletions pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1419,6 +1419,19 @@ pub(crate) fn try_walker_specialize_load_attr<Sym: WalkSym>(
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 {
Expand Down
16 changes: 7 additions & 9 deletions pyre/pyre-object/src/interp_exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
Loading