Implement opcode tracing and fix per-code instrumentation

youknowone · youknowone · commit 1498d7f5042b · 2026-02-28T23:04:50.000+09:00
- Add TraceEvent::Opcode for sys.settrace f_trace_opcodes support
- Fire 'opcode' event per instruction when f_trace_opcodes is set,
  gated by trace_opcodes lock in _trace_event_inner
- Add events_for_code() to compute per-code-object event mask
  (global events | code-local events) instead of broadcasting all
  local events to every code object, which caused stack corruption
  when INSTRUCTION was applied to unrelated running frames
- Use events_for_code() in update_events_mask and Resume handlers
- Remove test_stepinstr expectedFailure marker in test_bdb
diff --git a/Lib/test/test_bdb.py b/Lib/test/test_bdb.py
@@ -614,7 +614,6 @@ def test_step_next_on_last_statement(self):
                 with TracerRun(self) as tracer:
                     tracer.runcall(tfunc_main)
 
-    @unittest.expectedFailure  # TODO: RUSTPYTHON; AssertionError: All paired tuples have not been processed, the last one was number 1 [('next',), ('quit',)]
     def test_stepinstr(self):
         self.expect_set = [
             ('line',   2, 'tfunc_main'),  ('stepinstr', ),
diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs
@@ -596,6 +596,21 @@ impl ExecutingFrame<'_> {
                 self.state.prev_line = loc.line.get() as u32;
             }
 
+            // Fire 'opcode' trace event for sys.settrace when f_trace_opcodes
+            // is set. Skip RESUME and ExtendedArg (matching CPython's exclusion
+            // of these in _Py_call_instrumentation_instruction).
+            if vm.use_tracing.get()
+                && !vm.is_none(&self.object.trace.lock())
+                && !matches!(
+                    op,
+                    Instruction::Resume { .. }
+                        | Instruction::InstrumentedResume
+                        | Instruction::ExtendedArg
+                )
+            {
+                vm.trace_event(crate::protocol::TraceEvent::Opcode, None)?;
+            }
+
             let result = self.execute_instruction(op, arg, &mut do_extend_arg, vm);
             match result {
                 Ok(None) => {}
@@ -2192,7 +2207,10 @@ impl ExecutingFrame<'_> {
                     .instrumentation_version
                     .load(atomic::Ordering::Acquire);
                 if code_ver != global_ver {
-                    let events = vm.state.monitoring_events.load();
+                    let events = {
+                        let state = vm.state.monitoring.lock();
+                        state.events_for_code(self.code.get_id())
+                    };
                     monitoring::instrument_code(self.code, events);
                     self.code
                         .instrumentation_version
@@ -2569,7 +2587,10 @@ impl ExecutingFrame<'_> {
                     .instrumentation_version
                     .load(atomic::Ordering::Acquire);
                 if code_ver != global_ver {
-                    let events = vm.state.monitoring_events.load();
+                    let events = {
+                        let state = vm.state.monitoring.lock();
+                        state.events_for_code(self.code.get_id())
+                    };
                     monitoring::instrument_code(self.code, events);
                     self.code
                         .instrumentation_version
diff --git a/crates/vm/src/protocol/callable.rs b/crates/vm/src/protocol/callable.rs
@@ -76,6 +76,7 @@ pub(crate) enum TraceEvent {
     Return,
     Exception,
     Line,
+    Opcode,
     CCall,
     CReturn,
     CException,
@@ -91,6 +92,11 @@ impl TraceEvent {
             Self::Call | Self::Return | Self::CCall | Self::CReturn | Self::CException
         )
     }
+
+    /// Whether this event is dispatched only when f_trace_opcodes is set.
+    pub(crate) fn is_opcode_event(&self) -> bool {
+        matches!(self, Self::Opcode)
+    }
 }
 
 impl core::fmt::Display for TraceEvent {
@@ -101,6 +107,7 @@ impl core::fmt::Display for TraceEvent {
             Return => write!(f, "return"),
             Exception => write!(f, "exception"),
             Line => write!(f, "line"),
+            Opcode => write!(f, "opcode"),
             CCall => write!(f, "c_call"),
             CReturn => write!(f, "c_return"),
             CException => write!(f, "c_exception"),
@@ -142,11 +149,17 @@ impl VirtualMachine {
         }
 
         let is_profile_event = event.is_profile_event();
+        let is_opcode_event = event.is_opcode_event();
 
         let Some(frame_ref) = self.current_frame() else {
             return Ok(None);
         };
 
+        // Opcode events are only dispatched when f_trace_opcodes is set.
+        if is_opcode_event && !*frame_ref.trace_opcodes.lock() {
+            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())];
diff --git a/crates/vm/src/stdlib/sys/monitoring.rs b/crates/vm/src/stdlib/sys/monitoring.rs
@@ -119,6 +119,20 @@ impl MonitoringState {
         let local = self.local_events.values().fold(0, |acc, &e| acc | e);
         global | local
     }
+
+    /// Compute the events that apply to a specific code object:
+    /// global events OR'd with local events registered for that code.
+    /// This prevents events like INSTRUCTION that are local to one code
+    /// from being applied to unrelated code objects.
+    pub fn events_for_code(&self, code_id: usize) -> u32 {
+        let global = self.global_events.iter().fold(0, |acc, &e| acc | e);
+        let local = self
+            .local_events
+            .iter()
+            .filter(|((_, cid), _)| *cid == code_id)
+            .fold(0, |acc, (_, &e)| acc | e);
+        global | local
+    }
 }
 
 /// Global atomic mask: OR of all tools' events. Checked in the hot path
@@ -464,13 +478,17 @@ fn update_events_mask(vm: &VirtualMachine, state: &MonitoringState) {
         + 1;
     // Eagerly re-instrument all frames on the current thread's stack so that
     // code objects already past their RESUME pick up the new event set.
+    // Each code object gets only the events that apply to it (global + its
+    // own local events), preventing e.g. INSTRUCTION from being applied to
+    // unrelated code objects.
     for fp in vm.frames.borrow().iter() {
         // SAFETY: frames in the Vec are alive while their FrameRef is on the call stack.
         let frame = unsafe { fp.as_ref() };
         let code = &frame.code;
         let code_ver = code.instrumentation_version.load(Ordering::Acquire);
         if code_ver != new_ver {
-            instrument_code(code, events);
+            let code_events = state.events_for_code(code.get_id());
+            instrument_code(code, code_events);
             code.instrumentation_version
                 .store(new_ver, Ordering::Release);
         }
diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs
@@ -1164,15 +1164,12 @@ impl VirtualMachine {
         // PY_UNWIND fires PyTrace_RETURN with arg=None — so we fire for
         // both Ok and Err, matching `call_trace_protected` behavior.
         if self.use_tracing.get()
-            && (!self.is_none(&frame.trace.lock())
-                || !self.is_none(&self.profile_func.borrow()))
+            && (!self.is_none(&frame.trace.lock()) || !self.is_none(&self.profile_func.borrow()))
         {
             let ret_result = self.trace_event(TraceEvent::Return, None);
             // call_trace_protected: if trace function raises, its error
             // replaces the original exception.
-            if let Err(e) = ret_result {
-                return Err(e);
-            }
+            ret_result?;
         }
 
         result
