From 754fc85fb8fde62aba882e39922a4f566d4d9819 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Sat, 28 Feb 2026 10:32:15 +0900 Subject: [PATCH] Fix trace_event to return trace function result - Return trace function's return value from trace_event() to support per-frame f_trace assignment - Match CPython's trace_trampoline: set f_trace from call event return value, clear on error - Fire return event only when frame is traced or profiled - Remove expectedFailure from passing bdb/settrace tests --- Lib/test/test_bdb.py | 4 --- Lib/test/test_sys_settrace.py | 14 --------- crates/vm/src/protocol/callable.rs | 46 ++++++++++++++++++++++++------ crates/vm/src/vm/mod.rs | 39 +++++++++++++++++-------- 4 files changed, 65 insertions(+), 38 deletions(-) diff --git a/Lib/test/test_bdb.py b/Lib/test/test_bdb.py index a3abd275b..f1077d91f 100644 --- a/Lib/test/test_bdb.py +++ b/Lib/test/test_bdb.py @@ -762,7 +762,6 @@ class StateTestCase(BaseTestCase): bdb = Bdb(skip=['anything*']) self.assertIs(bdb.is_skipped_module(None), False) - @unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs def test_down(self): # Check that set_down() raises BdbError at the newest frame. self.expect_set = [ @@ -784,7 +783,6 @@ class StateTestCase(BaseTestCase): class BreakpointTestCase(BaseTestCase): """Test the breakpoint set method.""" - @unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs def test_bp_on_non_existent_module(self): self.expect_set = [ ('line', 2, 'tfunc_import'), ('break', ('/non/existent/module.py', 1)) @@ -792,7 +790,6 @@ class BreakpointTestCase(BaseTestCase): with TracerRun(self) as tracer: self.assertRaises(BdbError, tracer.runcall, tfunc_import) - @unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs def test_bp_after_last_statement(self): code = """ def main(): @@ -969,7 +966,6 @@ class BreakpointTestCase(BaseTestCase): with TracerRun(self) as tracer: tracer.runcall(tfunc_import) - @unittest.expectedFailure # TODO: RUSTPYTHON; Error in atexit._run_exitfuncs def test_clear_at_no_bp(self): self.expect_set = [ ('line', 2, 'tfunc_import'), ('clear', (__file__, 1)) diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index 65193d4f8..a98b4d227 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -1218,8 +1218,6 @@ class RaisingTraceFuncTestCase(unittest.TestCase): def test_exception(self): self.run_test_for_event('exception') - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_trash_stack(self): def f(): for i in range(5): @@ -1785,15 +1783,11 @@ class JumpTestCase(unittest.TestCase): # The second set of 'jump' tests are for things that are not allowed: - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(2, 3, [1], (ValueError, 'after')) def test_no_jump_too_far_forwards(output): output.append(1) output.append(2) - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(2, -2, [1], (ValueError, 'before')) def test_no_jump_too_far_backwards(output): output.append(1) @@ -1840,8 +1834,6 @@ class JumpTestCase(unittest.TestCase): output.append(4) raise e - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(1, 3, [], (ValueError, 'into')) def test_no_jump_forwards_into_for_block(output): output.append(1) @@ -1857,8 +1849,6 @@ class JumpTestCase(unittest.TestCase): output.append(3) pass - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(3, 2, [2, 2], (ValueError, 'into')) def test_no_jump_backwards_into_for_block(output): for i in 1, 2: @@ -2020,8 +2010,6 @@ class JumpTestCase(unittest.TestCase): raise output.append(8) - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(3, 6, [2], (ValueError, "into an 'except'")) def test_no_jump_into_qualified_except_block_from_try_block(output): try: @@ -2087,8 +2075,6 @@ class JumpTestCase(unittest.TestCase): return output.append(7) - # TODO: RUSTPYTHON - @unittest.expectedFailure @jump_test(7, 4, [1, 6], (ValueError, 'into')) def test_no_jump_into_for_block_before_else(output): output.append(1) diff --git a/crates/vm/src/protocol/callable.rs b/crates/vm/src/protocol/callable.rs index 9a621dee4..316ed36dd 100644 --- a/crates/vm/src/protocol/callable.rs +++ b/crates/vm/src/protocol/callable.rs @@ -96,37 +96,67 @@ impl core::fmt::Display for TraceEvent { impl VirtualMachine { /// Call registered trace function. + /// + /// Returns the trace function's return value: + /// - `Some(obj)` if the trace function returned a non-None value + /// - `None` if it returned Python None or no trace function was active + /// + /// In CPython's trace protocol: + /// - For 'call' events: the return value determines the per-frame `f_trace` + /// - For 'line'/'return' events: the return value can update `f_trace` #[inline] - pub(crate) fn trace_event(&self, event: TraceEvent, arg: Option) -> PyResult<()> { + pub(crate) fn trace_event( + &self, + event: TraceEvent, + arg: Option, + ) -> PyResult> { if self.use_tracing.get() { self._trace_event_inner(event, arg) } else { - Ok(()) + Ok(None) } } - fn _trace_event_inner(&self, event: TraceEvent, arg: Option) -> PyResult<()> { + fn _trace_event_inner( + &self, + event: TraceEvent, + arg: Option, + ) -> PyResult> { let trace_func = self.trace_func.borrow().to_owned(); let profile_func = self.profile_func.borrow().to_owned(); if self.is_none(&trace_func) && self.is_none(&profile_func) { - return Ok(()); + return Ok(None); } let Some(frame_ref) = self.current_frame() else { - return Ok(()); + return Ok(None); }; let frame: PyObjectRef = frame_ref.into(); let event = self.ctx.new_str(event.to_string()).into(); let args = vec![frame, event, arg.unwrap_or_else(|| self.ctx.none())]; + let mut trace_result = None; + // temporarily disable tracing, during the call to the // tracing function itself. if !self.is_none(&trace_func) { self.use_tracing.set(false); let res = trace_func.call(args.clone(), self); self.use_tracing.set(true); - if res.is_err() { - *self.trace_func.borrow_mut() = self.ctx.none(); + match res { + Ok(result) => { + if !self.is_none(&result) { + trace_result = Some(result); + } + } + Err(e) => { + // trace_trampoline behavior: clear per-frame f_trace + // and propagate the error. + if let Some(frame_ref) = self.current_frame() { + *frame_ref.trace.lock() = self.ctx.none(); + } + return Err(e); + } } } @@ -138,6 +168,6 @@ impl VirtualMachine { *self.profile_func.borrow_mut() = self.ctx.none(); } } - Ok(()) + Ok(trace_result) } } diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index ead411702..3ee421e04 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -1084,19 +1084,25 @@ impl VirtualMachine { // Fire 'call' trace event after pushing frame // (current_frame() now returns the callee's frame) + // + // trace_dispatch protocol (matching CPython's trace_trampoline): + // - For 'call' events, the global trace function is called. + // If it returns non-None, set f_trace to that value (trace this frame). + // If it returns None, leave f_trace unset (skip tracing this frame). + // - For 'return' events, fire if this frame has f_trace set OR if + // a profile function is active (profiling is independent of f_trace). match self.trace_event(TraceEvent::Call, None) { - Ok(()) => { - // Set per-frame trace function so line events fire for this frame. - // Frames entered before sys.settrace() keep trace=None and skip line events. - if self.use_tracing.get() { - let trace_func = self.trace_func.borrow().clone(); - if !self.is_none(&trace_func) { - *frame.trace.lock() = trace_func; - } + Ok(trace_result) => { + if let Some(local_trace) = trace_result { + *frame.trace.lock() = local_trace; } let result = f(frame.clone()); - // Fire 'return' trace event on success - if result.is_ok() { + // Fire 'return' event if frame is being traced or profiled + if result.is_ok() + && self.use_tracing.get() + && (!self.is_none(&frame.trace.lock()) + || !self.is_none(&self.profile_func.borrow())) + { let _ = self.trace_event(TraceEvent::Return, None); } result @@ -1155,9 +1161,18 @@ impl VirtualMachine { use crate::protocol::TraceEvent; match self.trace_event(TraceEvent::Call, None) { - Ok(()) => { + Ok(trace_result) => { + // Update per-frame trace if trace function returned a new local trace + if let Some(local_trace) = trace_result { + *frame.trace.lock() = local_trace; + } let result = f(frame); - if result.is_ok() { + // Fire 'return' event if frame is being traced or profiled + if result.is_ok() + && self.use_tracing.get() + && (!self.is_none(&frame.trace.lock()) + || !self.is_none(&self.profile_func.borrow())) + { let _ = self.trace_event(TraceEvent::Return, None); } result