mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 18:59:04 +08:00
Fix miri UB: use transmute_copy for function pointer identity checks (#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
This commit is contained in:
1 parent
0df8c8428b
commit
3364c47a4b
11 files changed
+72
-42
No files matched your search
@@ -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') }}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
};
|
||||
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<PyType>, 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 =
|
||||
|
||||
+10
-12
@@ -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(
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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<T: Copy>(f: T) -> usize {
|
||||
assert!(
|
||||
core::mem::size_of::<T>() == core::mem::size_of::<usize>(),
|
||||
"fn_addr: T must be pointer-sized"
|
||||
);
|
||||
unsafe { core::mem::transmute_copy::<T, usize>(&f) }
|
||||
}
|
||||
@@ -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));
|
||||
|
||||
@@ -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<PyStr>, vm: &VirtualMachine) -> PyResult<Self> {
|
||||
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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user