Skip to content

Commit 95f9d17

Browse files
youknowoneyouknowone
authored
Fix miri UB: use transmute_copy for function pointer identity checks (RustPython#8432)
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<T>(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
1 parent 1306b71 commit 95f9d17

11 files changed

Lines changed: 72 additions & 42 deletions

File tree

‎.github/workflows/ci.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,10 @@ jobs:
648648
env:
649649
# miri-ignore-leaks because the type-object circular reference means that there will always be
650650
# a memory leak, at least until we have proper cyclic gc
651-
MIRIFLAGS: "-Zmiri-ignore-leaks"
651+
# miri-permissive-provenance because function pointer identity checks (slot comparisons)
652+
# cast fn pointers to usize, which strips provenance — this is the standard pattern for
653+
# fn pointer comparison in Rust and not a soundness issue
654+
MIRIFLAGS: "-Zmiri-ignore-leaks -Zmiri-permissive-provenance"
652655

653656
wasm:
654657
if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}

‎crates/vm/src/builtins/object.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,12 @@ impl Initializer for PyBaseObject {
122122
let typ = zelf.class();
123123
let object_type = &vm.ctx.types.object_type;
124124

125-
let typ_init = typ.slots.init.load().map(|f| f as usize);
126-
let object_init = object_type.slots.init.load().map(|f| f as usize);
125+
let typ_init = typ.slots.init.load().map(|f| crate::types::fn_addr(f));
126+
let object_init = object_type
127+
.slots
128+
.init
129+
.load()
130+
.map(|f| crate::types::fn_addr(f));
127131

128132
// if (type->tp_init != object_init) → first error
129133
if typ_init != object_init {

‎crates/vm/src/builtins/set.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -960,15 +960,19 @@ impl Constructor for PyFrozenSet {
960960
fn slot_new(cls: PyTypeRef, args: FuncArgs, vm: &VirtualMachine) -> PyResult {
961961
let is_exact_frozenset = cls.is(vm.ctx.types.frozenset_type);
962962
let is_frozenset_init = {
963-
let cls_init = cls.slots.init.load().map(|init| init as usize);
963+
let cls_init = cls
964+
.slots
965+
.init
966+
.load()
967+
.map(|init| crate::types::fn_addr(init));
964968
let frozenset_init = vm
965969
.ctx
966970
.types
967971
.frozenset_type
968972
.slots
969973
.init
970974
.load()
971-
.map(|init| init as usize);
975+
.map(|init| crate::types::fn_addr(init));
972976
cls_init == frozenset_init
973977
};
974978

‎crates/vm/src/builtins/type.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2829,7 +2829,8 @@ impl Callable for PyType {
28292829
// path incorrectly.
28302830
if zelf.slots.init.load().is_none()
28312831
&& !zelf.is(vm.ctx.types.type_type)
2832-
&& slot_new as usize != crate::types::new_wrapper as crate::types::NewFunc as usize
2832+
&& crate::types::fn_addr(slot_new)
2833+
!= crate::types::fn_addr(crate::types::new_wrapper as crate::types::NewFunc)
28332834
{
28342835
return slot_new(zelf.to_owned(), args, vm);
28352836
}
@@ -3066,7 +3067,8 @@ pub(crate) fn call_slot_new(
30663067
// Check if staticbase's tp_new differs from typ's tp_new
30673068
let typ_new = typ.slots.new.load();
30683069
let staticbase_new = staticbase.slots.new.load();
3069-
if typ_new.map(|f| f as usize) != staticbase_new.map(|f| f as usize) {
3070+
if typ_new.map(|f| crate::types::fn_addr(f)) != staticbase_new.map(|f| crate::types::fn_addr(f))
3071+
{
30703072
return Err(vm.new_type_error(format!(
30713073
"{}.__new__({}) is not safe, use {}.__new__()",
30723074
typ.slot_name(),

‎crates/vm/src/class.rs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
builtins::{PyBaseObject, PyType, PyTypeRef, descriptor::PyWrapper},
66
function::PyMethodDef,
77
object::Py,
8-
types::{PyTypeFlags, PyTypeSlots, SLOT_DEFS, hash_not_implemented},
8+
types::{PyTypeFlags, PyTypeSlots, SLOT_DEFS, fn_addr, hash_not_implemented},
99
vm::Context,
1010
};
1111
use rustpython_common::static_cell;
@@ -24,11 +24,9 @@ pub fn add_operators(class: &'static Py<PyType>, ctx: &Context) {
2424

2525
// Special handling for __hash__ = None
2626
if def.name == "__hash__"
27-
&& class
28-
.slots
29-
.hash
30-
.load()
31-
.is_some_and(|h| h as usize == hash_not_implemented as *const () as usize)
27+
&& class.slots.hash.load().is_some_and(|h| {
28+
fn_addr(h) == fn_addr(hash_not_implemented as crate::types::HashFunc)
29+
})
3230
{
3331
class.set_attr(ctx.names.__hash__, ctx.none.clone().into());
3432
continue;
@@ -205,7 +203,7 @@ pub trait PyClassImpl: PyClassDef {
205203
let object_new = ctx.types.object_type.slots.new.load();
206204
let is_object_itself = core::ptr::eq(class, ctx.types.object_type);
207205
let is_inherited_from_object = !is_object_itself
208-
&& object_new.is_some_and(|obj_new| slot_new as usize == obj_new as usize);
206+
&& object_new.is_some_and(|obj_new| fn_addr(slot_new) == fn_addr(obj_new));
209207

210208
if !is_inherited_from_object {
211209
let bound_new =

‎crates/vm/src/frame.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8932,11 +8932,10 @@ impl ExecutingFrame<'_> {
89328932
}
89338933

89348934
// Only specialize if getattro is the default (PyBaseObject::getattro)
8935-
let is_default_getattro = cls
8936-
.slots
8937-
.getattro
8938-
.load()
8939-
.is_some_and(|f| f as usize == PyBaseObject::getattro as *const () as usize);
8935+
let is_default_getattro = cls.slots.getattro.load().is_some_and(|f| {
8936+
crate::types::fn_addr(f)
8937+
== crate::types::fn_addr(PyBaseObject::getattro as crate::types::GetattroFunc)
8938+
});
89408939
if !is_default_getattro {
89418940
let getattribute = cls.get_attr(identifier!(_vm, __getattribute__));
89428941
if !oparg.is_method()
@@ -9953,8 +9952,8 @@ impl ExecutingFrame<'_> {
99539952
let cls_alloc = cls.slots.alloc.load();
99549953
if let (Some(cls_new_fn), Some(obj_new_fn), Some(cls_alloc_fn), Some(obj_alloc_fn)) =
99559954
(cls_new, object_new, cls_alloc, object_alloc)
9956-
&& cls_new_fn as usize == obj_new_fn as usize
9957-
&& cls_alloc_fn as usize == obj_alloc_fn as usize
9955+
&& crate::types::fn_addr(cls_new_fn) == crate::types::fn_addr(obj_new_fn)
9956+
&& crate::types::fn_addr(cls_alloc_fn) == crate::types::fn_addr(obj_alloc_fn)
99589957
{
99599958
if type_version == 0 {
99609959
unsafe {
@@ -10614,11 +10613,10 @@ impl ExecutingFrame<'_> {
1061410613
}
1061510614

1061610615
// Only specialize if setattr is the default (generic_setattr)
10617-
let is_default_setattr = cls
10618-
.slots
10619-
.setattro
10620-
.load()
10621-
.is_some_and(|f| f as usize == PyBaseObject::slot_setattro as *const () as usize);
10616+
let is_default_setattr = cls.slots.setattro.load().is_some_and(|f| {
10617+
crate::types::fn_addr(f)
10618+
== crate::types::fn_addr(PyBaseObject::slot_setattro as crate::types::SetattroFunc)
10619+
});
1062210620
if !is_default_setattr {
1062310621
unsafe {
1062410622
self.code.instructions.write_adaptive_counter(

‎crates/vm/src/stdlib/_thread.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -997,8 +997,8 @@ pub(crate) mod _thread {
997997
.slots
998998
.init
999999
.load()
1000-
.map(|init| init as usize);
1001-
(Some(cls_init as usize) != object_init).then_some(cls_init)
1000+
.map(|init| crate::types::fn_addr(init));
1001+
(Some(crate::types::fn_addr(cls_init)) != object_init).then_some(cls_init)
10021002
}
10031003

10041004
fn create_dict(&self, vm: &VirtualMachine) -> (PyDictRef, bool) {

‎crates/vm/src/types/slot.rs‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -877,8 +877,8 @@ impl PyType {
877877
.iter()
878878
.find(|cls| cls.attributes.read().contains_key(name))
879879
.is_some_and(|cls| {
880-
cls.slots.new.load().map(|f| f as usize)
881-
== Some(new_wrapper as NewFunc as usize)
880+
cls.slots.new.load().map(|f| fn_addr(f))
881+
== Some(fn_addr(new_wrapper as NewFunc))
882882
})
883883
};
884884
if needs_wrapper {
@@ -953,7 +953,7 @@ impl PyType {
953953
self.slots.setattro.store(Some(setattro_wrapper));
954954
}
955955
(NativeSlot(set), NativeSlot(del)) => {
956-
let func = if set as usize == del as usize {
956+
let func = if fn_addr(set) == fn_addr(del) {
957957
set
958958
} else {
959959
setattro_wrapper
@@ -988,7 +988,7 @@ impl PyType {
988988
self.slots.descr_set.store(Some(descr_set_wrapper));
989989
}
990990
(NativeSlot(set), NativeSlot(delete)) => {
991-
let func = if set as usize == delete as usize {
991+
let func = if fn_addr(set) == fn_addr(delete) {
992992
set
993993
} else {
994994
descr_set_wrapper
@@ -2171,3 +2171,24 @@ where
21712171
debug_assert!(prev.is_some()); // slot_iter would be set
21722172
}
21732173
}
2174+
2175+
/// Extract the raw address of a function pointer as `usize` without
2176+
/// triggering miri's "pointer not dereferenceable" UB.
2177+
///
2178+
/// The standard `fn_ptr as usize` cast goes through `FnPtr::addr()`
2179+
/// which attempts to dereference the function pointer's provenance —
2180+
/// miri considers this UB for function items. `transmute_copy` bypasses
2181+
/// that path and reads the address as plain integer bytes.
2182+
///
2183+
/// The result is suitable for identity comparison only: two function
2184+
/// pointers with the same address are the same function. The converse
2185+
/// is not always guaranteed (the compiler may merge identical function
2186+
/// bodies), but this matches CPython's slot comparison semantics.
2187+
#[inline(always)]
2188+
pub(crate) fn fn_addr<T: Copy>(f: T) -> usize {
2189+
assert!(
2190+
core::mem::size_of::<T>() == core::mem::size_of::<usize>(),
2191+
"fn_addr: T must be pointer-sized"
2192+
);
2193+
unsafe { core::mem::transmute_copy::<T, usize>(&f) }
2194+
}

‎crates/vm/src/types/slot_defs.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//!
33
//! This module provides a centralized array of all slot definitions,
44
5-
use super::{PyComparisonOp, PyTypeSlots};
5+
use super::{PyComparisonOp, PyTypeSlots, fn_addr};
66
use crate::builtins::descriptor::SlotFunc;
77

88
/// Slot operation type
@@ -609,7 +609,7 @@ impl SlotAccessor {
609609
&& let Some(base_val) = base.slots.init.load()
610610
{
611611
let slot_defined = base.base.deref().is_none_or(|bb| {
612-
bb.slots.init.load().map(|v| v as usize) != Some(base_val as usize)
612+
bb.slots.init.load().map(|v| fn_addr(v)) != Some(fn_addr(base_val))
613613
});
614614
if slot_defined {
615615
typ.slots.init.store(Some(base_val));

‎crates/vm/src/vm/method.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
builtins::{PyBaseObject, PyStr, PyStrInterned, descriptor::PyMethodDescriptor},
77
function::{IntoFuncArgs, PyMethodFlags},
88
object::{AsObject, Py, PyObject, PyObjectRef, PyResult},
9-
types::PyTypeFlags,
9+
types::{GetattroFunc, PyTypeFlags, fn_addr},
1010
};
1111

1212
#[derive(Debug)]
@@ -22,7 +22,7 @@ impl PyMethod {
2222
pub(crate) fn get(obj: PyObjectRef, name: &Py<PyStr>, vm: &VirtualMachine) -> PyResult<Self> {
2323
let cls = obj.class();
2424
let getattro = cls.slots.getattro.load().unwrap();
25-
if getattro as usize != PyBaseObject::getattro as *const () as usize {
25+
if fn_addr(getattro) != fn_addr(PyBaseObject::getattro as GetattroFunc) {
2626
return obj.get_attr(name, vm).map(Self::Attribute);
2727
}
2828

0 commit comments

Comments
 (0)