Address review: bounds checks, UB fix, version overflow, error handling

- Add bounds checks to read_cache_u16/u32/u64 - Fix quicken() aliasing UB by using &mut directly - Add JumpBackwardJit/JumpBackwardNoJit to deoptimize() - Guard can_specialize_call with NEWLOCALS flag check - Use compare_exchange_weak for version tag to prevent wraparound - Propagate dict lookup errors in LoadAttrMethodWithValues - Apply adaptive backoff on version tag assignment failure - Remove duplicate imports in frame.rs
RustPython · youknowone · Mar 2, 2026 · Mar 1, 2026 · Mar 1, 2026 · Mar 1, 2026
commit 79ca9c11249699fc364bd586b94aeb7531351dcc
diff --git a/crates/compiler-core/src/bytecode.rs b/crates/compiler-core/src/bytecode.rs
@@ -466,8 +466,12 @@ impl CodeUnits {
     }
 
     /// Read a u16 value from a CACHE code unit at `index`.
+    ///
+    /// # Panics
+    /// Panics if `index` is out of bounds.
     pub fn read_cache_u16(&self, index: usize) -> u16 {
         let units = unsafe { &*self.0.get() };
+        assert!(index < units.len(), "read_cache_u16: index out of bounds");
         let ptr = units.as_ptr().wrapping_add(index) as *const u8;
         unsafe { core::ptr::read_unaligned(ptr as *const u16) }
     }
@@ -484,6 +488,9 @@ impl CodeUnits {
     }
 
     /// Read a u32 value from two consecutive CACHE code units starting at `index`.
+    ///
+    /// # Panics
+    /// Panics if `index + 1` is out of bounds.
     pub fn read_cache_u32(&self, index: usize) -> u32 {
         let lo = self.read_cache_u16(index) as u32;
         let hi = self.read_cache_u16(index + 1) as u32;
@@ -502,6 +509,9 @@ impl CodeUnits {
     }
 
     /// Read a u64 value from four consecutive CACHE code units starting at `index`.
+    ///
+    /// # Panics
+    /// Panics if `index + 3` is out of bounds.
     pub fn read_cache_u64(&self, index: usize) -> u64 {
         let lo = self.read_cache_u32(index) as u64;
         let hi = self.read_cache_u32(index + 2) as u64;
@@ -553,7 +563,7 @@ impl CodeUnits {
     /// Called lazily at RESUME (first execution of a code object).
     /// Uses the `arg` byte of the first CACHE entry, preserving `op = Instruction::Cache`.
     pub fn quicken(&self) {
-        let units = unsafe { &*self.0.get() };
+        let units = unsafe { &mut *self.0.get() };
         let len = units.len();
         let mut i = 0;
         while i < len {
@@ -565,9 +575,7 @@ impl CodeUnits {
                 if !op.is_instrumented() {
                     let cache_base = i + 1;
                     if cache_base < len {
-                        unsafe {
-                            self.write_adaptive_counter(cache_base, ADAPTIVE_WARMUP_VALUE);
-                        }
+                        units[cache_base].arg = OpArgByte::from(ADAPTIVE_WARMUP_VALUE);
                     }
                 }
                 i += 1 + caches;

diff --git a/crates/compiler-core/src/bytecode/instruction.rs b/crates/compiler-core/src/bytecode/instruction.rs
@@ -620,6 +620,10 @@ impl Instruction {
             }
             // RESUME specializations
             Self::ResumeCheck => Self::Resume { arg: Arg::marker() },
+            // JUMP_BACKWARD specializations
+            Self::JumpBackwardJit | Self::JumpBackwardNoJit => Self::JumpBackward {
+                target: Arg::marker(),
+            },
             // Instrumented opcodes map back to their base
             _ => match self.to_base() {
                 Some(base) => base,

diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs
@@ -611,12 +611,14 @@ impl Py<PyFunction> {
     pub(crate) fn can_specialize_call(&self, effective_nargs: u32) -> bool {
         let code = self.code.lock();
         let flags = code.flags;
-        !flags.intersects(
-            bytecode::CodeFlags::VARARGS
-                | bytecode::CodeFlags::VARKEYWORDS
-                | bytecode::CodeFlags::GENERATOR
-                | bytecode::CodeFlags::COROUTINE,
-        ) && code.kwonlyarg_count == 0
+        flags.contains(bytecode::CodeFlags::NEWLOCALS)
+            && !flags.intersects(
+                bytecode::CodeFlags::VARARGS
+                    | bytecode::CodeFlags::VARKEYWORDS
+                    | bytecode::CodeFlags::GENERATOR
+                    | bytecode::CodeFlags::COROUTINE,
+            )
+            && code.kwonlyarg_count == 0
             && code.arg_count == effective_nargs
     }
 

diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs
@@ -201,12 +201,19 @@ fn is_subtype_with_mro(a_mro: &[PyTypeRef], a: &Py<PyType>, b: &Py<PyType>) -> b
 impl PyType {
     /// Assign a fresh version tag. Returns 0 on overflow (all caches invalidated).
     pub fn assign_version_tag(&self) -> u32 {
-        let v = NEXT_TYPE_VERSION.fetch_add(1, Ordering::Relaxed);
-        if v == 0 {
-            return 0;
+        loop {
+            let current = NEXT_TYPE_VERSION.load(Ordering::Relaxed);
+            let Some(next) = current.checked_add(1) else {
+                return 0; // Overflow: version space exhausted
+            };
+            if NEXT_TYPE_VERSION
+                .compare_exchange_weak(current, next, Ordering::Relaxed, Ordering::Relaxed)
+                .is_ok()
+            {
+                self.tp_version_tag.store(current, Ordering::Release);
+                return current;
+            }
         }
-        self.tp_version_tag.store(v, Ordering::Release);
-        v
     }
-    /// Assign a fresh version tag. Returns 0 on overflow (all caches invalidated).
-    pub fn assign_version_tag(&self) -> u32 {
-        let v = NEXT_TYPE_VERSION.fetch_add(1, Ordering::Relaxed);
-        if v == 0 {
-            return 0;
-        }
-        self.tp_version_tag.store(v, Ordering::Release);
-        v
-    }
+    /// Assign a fresh version tag. Returns 0 on overflow (all caches invalidated).
+    pub fn assign_version_tag(&self) -> u32 {
+        let v = NEXT_TYPE_VERSION.fetch_add(1, Ordering::Relaxed);
+        if v == 0 || v == u32::MAX {
+            NEXT_TYPE_VERSION.store(0, Ordering::Relaxed);
+            self.tp_version_tag.store(0, Ordering::Release);
+            return 0;
+        }
+        self.tp_version_tag.store(v, Ordering::Release);
+        v
+    }
-    /// Assign a fresh version tag. Returns 0 on overflow (all caches invalidated).
-    pub fn assign_version_tag(&self) -> u32 {
-        let v = NEXT_TYPE_VERSION.fetch_add(1, Ordering::Relaxed);
-        if v == 0 {
-            return 0;
-        }
-        self.tp_version_tag.store(v, Ordering::Release);
-        v
-    }
+    /// Assign a fresh version tag. Returns 0 on overflow (all caches invalidated).
+    pub fn assign_version_tag(&self) -> u32 {
+        let v = NEXT_TYPE_VERSION.fetch_add(1, Ordering::Relaxed);
+        if v == 0 || v == u32::MAX {
+            NEXT_TYPE_VERSION.store(0, Ordering::Relaxed);
+            self.tp_version_tag.store(0, Ordering::Release);
+            return 0;
+        }
+        self.tp_version_tag.store(v, Ordering::Release);
+        v
+    }
 
     /// Invalidate this type's version tag and cascade to all subclasses.

diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs
@@ -8,10 +8,8 @@ use crate::{
         PyFloat, PyGenerator, PyInt, PyInterpolation, PyList, PySet, PySlice, PyStr, PyStrInterned,
         PyTemplate, PyTraceback, PyType, PyUtf8Str,
         asyncgenerator::PyAsyncGenWrappedValue,
-        float::PyFloat,
         frame::stack_analysis,
         function::{PyCell, PyCellRef, PyFunction},
-        int::PyInt,
         range::PyRangeIterator,
         tuple::{PyTuple, PyTupleRef},
     },
@@ -2722,7 +2720,23 @@ impl ExecutingFrame<'_> {
                 if type_version != 0 && owner.class().tp_version_tag.load(Acquire) == type_version {
                     // Check instance dict doesn't shadow the method
                     let shadowed = if let Some(dict) = owner.dict() {
-                        dict.get_item_opt(attr_name, vm).ok().flatten().is_some()
+                        match dict.get_item_opt(attr_name, vm) {
+                            Ok(Some(_)) => true,
+                            Ok(None) => false,
+                            Err(_) => {
+                                // Dict lookup error → deoptimize to safe path
+                                unsafe {
+                                    self.code.instructions.replace_op(
+                                        instr_idx,
+                                        Instruction::LoadAttr { idx: Arg::marker() },
+                                    );
+                                    self.code
+                                        .instructions
+                                        .write_adaptive_counter(cache_base, ADAPTIVE_BACKOFF_VALUE);
+                                }
+                                return self.load_attr_slow(vm, oparg);
+                            }
+                        }
                     } else {
                         false
                     };
@@ -4471,7 +4485,12 @@ impl ExecutingFrame<'_> {
             type_version = cls.assign_version_tag();
         }
         if type_version == 0 {
-            // Version counter overflow
+            // Version counter overflow — backoff to avoid re-attempting every execution
+            unsafe {
+                self.code
+                    .instructions
+                    .write_adaptive_counter(cache_base, ADAPTIVE_BACKOFF_VALUE);
+            }
             return;
         }
 

diff --git a/crates/vm/src/stdlib/sys/monitoring.rs b/crates/vm/src/stdlib/sys/monitoring.rs
@@ -276,13 +276,13 @@ pub fn instrument_code(code: &PyCode, events: u32) {
         let mut i = 0;
         while i < len {
             let op = code.code.instructions[i].op;
-            let de_opt = op.deoptimize();
-            if u8::from(de_opt) != u8::from(op) {
+            let base_op = op.deoptimize();
+            if u8::from(base_op) != u8::from(op) {
                 unsafe {
-                    code.code.instructions.replace_op(i, de_opt);
+                    code.code.instructions.replace_op(i, base_op);
                 }
             }
-            let caches = de_opt.cache_entries();
+            let caches = base_op.cache_entries();
             // Zero all CACHE entries (the op+arg bytes may have been overwritten
             // by specialization with arbitrary data like pointers).
             for c in 1..=caches {