From 3364c47a4b3da385b80206cb7e4fe2e30108a18d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" <69878+youknowone@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:12:30 +0900 Subject: [PATCH] Fix miri UB: use transmute_copy for function pointer identity checks (#8432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nightly miri now flags `fn_ptr as usize` and direct `fn_ptr == fn_ptr` as UB because both paths go through `FnPtr::addr()`, which attempts to dereference a function pointer's provenance — function items have no backing allocation in miri's model. Add `fn_addr(f: T) -> usize` that uses `transmute_copy` to read the address as plain integer bytes without triggering provenance checks. Replace all `f as usize` slot comparison patterns across the codebase with `fn_addr(f)`. Also add `-Zmiri-permissive-provenance` to CI MIRIFLAGS as a safety net for any remaining integer-pointer round-trips elsewhere. Assisted-by: Claude --- .github/workflows/ci.yaml | 5 ++++- crates/vm/src/builtins/object.rs | 8 ++++++-- crates/vm/src/builtins/set.rs | 8 ++++++-- crates/vm/src/builtins/type.rs | 6 ++++-- crates/vm/src/class.rs | 12 +++++------- crates/vm/src/frame.rs | 22 ++++++++++------------ crates/vm/src/stdlib/_thread.rs | 4 ++-- crates/vm/src/types/slot.rs | 29 +++++++++++++++++++++++++---- crates/vm/src/types/slot_defs.rs | 4 ++-- crates/vm/src/vm/method.rs | 4 ++-- crates/vm/src/vm/vm_ops.rs | 12 ++++++------ 11 files changed, 72 insertions(+), 42 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7d0ab2e7c..dc7a1a3e7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -648,7 +648,10 @@ jobs: env: # miri-ignore-leaks because the type-object circular reference means that there will always be # a memory leak, at least until we have proper cyclic gc - MIRIFLAGS: "-Zmiri-ignore-leaks" + # miri-permissive-provenance because function pointer identity checks (slot comparisons) + # cast fn pointers to usize, which strips provenance — this is the standard pattern for + # fn pointer comparison in Rust and not a soundness issue + MIRIFLAGS: "-Zmiri-ignore-leaks -Zmiri-permissive-provenance" wasm: if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }} diff --git a/crates/vm/src/builtins/object.rs b/crates/vm/src/builtins/object.rs index 633eaf48a..151b753b2 100644 --- a/crates/vm/src/builtins/object.rs +++ b/crates/vm/src/builtins/object.rs @@ -122,8 +122,12 @@ impl Initializer for PyBaseObject { let typ = zelf.class(); let object_type = &vm.ctx.types.object_type; - let typ_init = typ.slots.init.load().map(|f| f as usize); - let object_init = object_type.slots.init.load().map(|f| f as usize); + let typ_init = typ.slots.init.load().map(|f| crate::types::fn_addr(f)); + let object_init = object_type + .slots + .init + .load() + .map(|f| crate::types::fn_addr(f)); // if (type->tp_init != object_init) → first error if typ_init != object_init { diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 1481bc1b3..cd724abc5 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -960,7 +960,11 @@ impl Constructor for PyFrozenSet { fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult { let is_exact_frozenset = cls.is(vm.ctx.types.frozenset_type); let is_frozenset_init = { - let cls_init = cls.slots.init.load().map(|init| init as usize); + let cls_init = cls + .slots + .init + .load() + .map(|init| crate::types::fn_addr(init)); let frozenset_init = vm .ctx .types @@ -968,7 +972,7 @@ impl Constructor for PyFrozenSet { .slots .init .load() - .map(|init| init as usize); + .map(|init| crate::types::fn_addr(init)); cls_init == frozenset_init }; diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 0ccfe155d..e2af7d21d 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -2829,7 +2829,8 @@ impl Callable for PyType { // path incorrectly. if zelf.slots.init.load().is_none() && !zelf.is(vm.ctx.types.type_type) - && slot_new as usize != crate::types::new_wrapper as crate::types::NewFunc as usize + && crate::types::fn_addr(slot_new) + != crate::types::fn_addr(crate::types::new_wrapper as crate::types::NewFunc) { return slot_new(zelf.to_owned(), args, vm); } @@ -3066,7 +3067,8 @@ pub(crate) fn call_slot_new( // Check if staticbase's tp_new differs from typ's tp_new let typ_new = typ.slots.new.load(); let staticbase_new = staticbase.slots.new.load(); - if typ_new.map(|f| f as usize) != staticbase_new.map(|f| f as usize) { + if typ_new.map(|f| crate::types::fn_addr(f)) != staticbase_new.map(|f| crate::types::fn_addr(f)) + { return Err(vm.new_type_error(format!( "{}.__new__({}) is not safe, use {}.__new__()", typ.slot_name(), diff --git a/crates/vm/src/class.rs b/crates/vm/src/class.rs index 364ae7211..d9f8b848d 100644 --- a/crates/vm/src/class.rs +++ b/crates/vm/src/class.rs @@ -5,7 +5,7 @@ use crate::{ builtins::{PyBaseObject, PyType, PyTypeRef, descriptor::PyWrapper}, function::PyMethodDef, object::Py, - types::{PyTypeFlags, PyTypeSlots, SLOT_DEFS, hash_not_implemented}, + types::{PyTypeFlags, PyTypeSlots, SLOT_DEFS, fn_addr, hash_not_implemented}, vm::Context, }; use rustpython_common::static_cell; @@ -24,11 +24,9 @@ pub fn add_operators(class: &'static Py, ctx: &Context) { // Special handling for __hash__ = None if def.name == "__hash__" - && class - .slots - .hash - .load() - .is_some_and(|h| h as usize == hash_not_implemented as *const () as usize) + && class.slots.hash.load().is_some_and(|h| { + fn_addr(h) == fn_addr(hash_not_implemented as crate::types::HashFunc) + }) { class.set_attr(ctx.names.__hash__, ctx.none.clone().into()); continue; @@ -205,7 +203,7 @@ pub trait PyClassImpl: PyClassDef { let object_new = ctx.types.object_type.slots.new.load(); let is_object_itself = core::ptr::eq(class, ctx.types.object_type); let is_inherited_from_object = !is_object_itself - && object_new.is_some_and(|obj_new| slot_new as usize == obj_new as usize); + && object_new.is_some_and(|obj_new| fn_addr(slot_new) == fn_addr(obj_new)); if !is_inherited_from_object { let bound_new = diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 5c21ad29c..a05fc7184 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -8932,11 +8932,10 @@ impl ExecutingFrame<'_> { } // Only specialize if getattro is the default (PyBaseObject::getattro) - let is_default_getattro = cls - .slots - .getattro - .load() - .is_some_and(|f| f as usize == PyBaseObject::getattro as *const () as usize); + let is_default_getattro = cls.slots.getattro.load().is_some_and(|f| { + crate::types::fn_addr(f) + == crate::types::fn_addr(PyBaseObject::getattro as crate::types::GetattroFunc) + }); if !is_default_getattro { let getattribute = cls.get_attr(identifier!(_vm, __getattribute__)); if !oparg.is_method() @@ -9953,8 +9952,8 @@ impl ExecutingFrame<'_> { let cls_alloc = cls.slots.alloc.load(); if let (Some(cls_new_fn), Some(obj_new_fn), Some(cls_alloc_fn), Some(obj_alloc_fn)) = (cls_new, object_new, cls_alloc, object_alloc) - && cls_new_fn as usize == obj_new_fn as usize - && cls_alloc_fn as usize == obj_alloc_fn as usize + && crate::types::fn_addr(cls_new_fn) == crate::types::fn_addr(obj_new_fn) + && crate::types::fn_addr(cls_alloc_fn) == crate::types::fn_addr(obj_alloc_fn) { if type_version == 0 { unsafe { @@ -10614,11 +10613,10 @@ impl ExecutingFrame<'_> { } // Only specialize if setattr is the default (generic_setattr) - let is_default_setattr = cls - .slots - .setattro - .load() - .is_some_and(|f| f as usize == PyBaseObject::slot_setattro as *const () as usize); + let is_default_setattr = cls.slots.setattro.load().is_some_and(|f| { + crate::types::fn_addr(f) + == crate::types::fn_addr(PyBaseObject::slot_setattro as crate::types::SetattroFunc) + }); if !is_default_setattr { unsafe { self.code.instructions.write_adaptive_counter( diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index e9d75dc02..64e5c596b 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -997,8 +997,8 @@ pub(crate) mod _thread { .slots .init .load() - .map(|init| init as usize); - (Some(cls_init as usize) != object_init).then_some(cls_init) + .map(|init| crate::types::fn_addr(init)); + (Some(crate::types::fn_addr(cls_init)) != object_init).then_some(cls_init) } fn create_dict(&self, vm: &VirtualMachine) -> (PyDictRef, bool) { diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index a5d7a41d3..d834406cf 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -877,8 +877,8 @@ impl PyType { .iter() .find(|cls| cls.attributes.read().contains_key(name)) .is_some_and(|cls| { - cls.slots.new.load().map(|f| f as usize) - == Some(new_wrapper as NewFunc as usize) + cls.slots.new.load().map(|f| fn_addr(f)) + == Some(fn_addr(new_wrapper as NewFunc)) }) }; if needs_wrapper { @@ -953,7 +953,7 @@ impl PyType { self.slots.setattro.store(Some(setattro_wrapper)); } (NativeSlot(set), NativeSlot(del)) => { - let func = if set as usize == del as usize { + let func = if fn_addr(set) == fn_addr(del) { set } else { setattro_wrapper @@ -988,7 +988,7 @@ impl PyType { self.slots.descr_set.store(Some(descr_set_wrapper)); } (NativeSlot(set), NativeSlot(delete)) => { - let func = if set as usize == delete as usize { + let func = if fn_addr(set) == fn_addr(delete) { set } else { descr_set_wrapper @@ -2171,3 +2171,24 @@ where debug_assert!(prev.is_some()); // slot_iter would be set } } + +/// Extract the raw address of a function pointer as `usize` without +/// triggering miri's "pointer not dereferenceable" UB. +/// +/// The standard `fn_ptr as usize` cast goes through `FnPtr::addr()` +/// which attempts to dereference the function pointer's provenance — +/// miri considers this UB for function items. `transmute_copy` bypasses +/// that path and reads the address as plain integer bytes. +/// +/// The result is suitable for identity comparison only: two function +/// pointers with the same address are the same function. The converse +/// is not always guaranteed (the compiler may merge identical function +/// bodies), but this matches CPython's slot comparison semantics. +#[inline(always)] +pub(crate) fn fn_addr(f: T) -> usize { + assert!( + core::mem::size_of::() == core::mem::size_of::(), + "fn_addr: T must be pointer-sized" + ); + unsafe { core::mem::transmute_copy::(&f) } +} diff --git a/crates/vm/src/types/slot_defs.rs b/crates/vm/src/types/slot_defs.rs index 34d62147e..69c7bb610 100644 --- a/crates/vm/src/types/slot_defs.rs +++ b/crates/vm/src/types/slot_defs.rs @@ -2,7 +2,7 @@ //! //! This module provides a centralized array of all slot definitions, -use super::{PyComparisonOp, PyTypeSlots}; +use super::{PyComparisonOp, PyTypeSlots, fn_addr}; use crate::builtins::descriptor::SlotFunc; /// Slot operation type @@ -609,7 +609,7 @@ impl SlotAccessor { && let Some(base_val) = base.slots.init.load() { let slot_defined = base.base.deref().is_none_or(|bb| { - bb.slots.init.load().map(|v| v as usize) != Some(base_val as usize) + bb.slots.init.load().map(|v| fn_addr(v)) != Some(fn_addr(base_val)) }); if slot_defined { typ.slots.init.store(Some(base_val)); diff --git a/crates/vm/src/vm/method.rs b/crates/vm/src/vm/method.rs index 9e4f71855..2beeb95ce 100644 --- a/crates/vm/src/vm/method.rs +++ b/crates/vm/src/vm/method.rs @@ -6,7 +6,7 @@ use crate::{ builtins::{PyBaseObject, PyStr, PyStrInterned, descriptor::PyMethodDescriptor}, function::{IntoFuncArgs, PyMethodFlags}, object::{AsObject, Py, PyObject, PyObjectRef, PyResult}, - types::PyTypeFlags, + types::{GetattroFunc, PyTypeFlags, fn_addr}, }; #[derive(Debug)] @@ -22,7 +22,7 @@ impl PyMethod { pub(crate) fn get(obj: PyObjectRef, name: &Py, vm: &VirtualMachine) -> PyResult { let cls = obj.class(); let getattro = cls.slots.getattro.load().unwrap(); - if getattro as usize != PyBaseObject::getattro as *const () as usize { + if fn_addr(getattro) != fn_addr(PyBaseObject::getattro as GetattroFunc) { return obj.get_attr(name, vm).map(Self::Attribute); } diff --git a/crates/vm/src/vm/vm_ops.rs b/crates/vm/src/vm/vm_ops.rs index 8cb00d4a1..692444fc7 100644 --- a/crates/vm/src/vm/vm_ops.rs +++ b/crates/vm/src/vm/vm_ops.rs @@ -180,13 +180,13 @@ impl VirtualMachine { // Number slots are inherited, direct access is O(1) let slot_a = class_a.slots.as_number.left_binary_op(op_slot); - let slot_a_addr = slot_a.map(|x| x as usize); + let slot_a_addr = slot_a.map(|x| crate::types::fn_addr(x)); let mut slot_b = None; let left_b_addr = if class_a.is(class_b) { slot_a_addr } else { let slot_bb = class_b.slots.as_number.right_binary_op(op_slot); - if slot_bb.map(|x| x as usize) != slot_a_addr { + if slot_bb.map(|x| crate::types::fn_addr(x)) != slot_a_addr { slot_b = slot_bb; } @@ -194,7 +194,7 @@ impl VirtualMachine { .slots .as_number .left_binary_op(op_slot) - .map(|x| x as usize) + .map(|x| crate::types::fn_addr(x)) }; if let Some(slot_a) = slot_a { @@ -302,13 +302,13 @@ impl VirtualMachine { // Number slots are inherited, direct access is O(1) let slot_a = class_a.slots.as_number.left_ternary_op(op_slot); - let slot_a_addr = slot_a.map(|x| x as usize); + let slot_a_addr = slot_a.map(|x| crate::types::fn_addr(x)); let mut slot_b = None; let left_b_addr = if class_a.is(class_b) { slot_a_addr } else { let slot_bb = class_b.slots.as_number.right_ternary_op(op_slot); - if slot_bb.map(|x| x as usize) != slot_a_addr { + if slot_bb.map(|x| crate::types::fn_addr(x)) != slot_a_addr { slot_b = slot_bb; } @@ -316,7 +316,7 @@ impl VirtualMachine { .slots .as_number .left_ternary_op(op_slot) - .map(|x| x as usize) + .map(|x| crate::types::fn_addr(x)) }; if let Some(slot_a) = slot_a {