Skip to content

Fix miri UB: use transmute_copy for function pointer identity checks #8432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Aug 2, 2026
Merged

Fix miri UB: use transmute_copy for function pointer identity checks #8432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Comment on lines +651 to +654

wasm:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}
Expand Down
8 changes: 6 additions & 2 deletions crates/vm/src/builtins/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions crates/vm/src/builtins/set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,15 +960,19 @@ 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
.frozenset_type
.slots
.init
.load()
.map(|init| init as usize);
.map(|init| crate::types::fn_addr(init));
cls_init == frozenset_init
};

Expand Down
6 changes: 4 additions & 2 deletions crates/vm/src/builtins/type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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(),
Expand Down
12 changes: 5 additions & 7 deletions crates/vm/src/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down
22 changes: 10 additions & 12 deletions crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/stdlib/_thread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
29 changes: 25 additions & 4 deletions crates/vm/src/types/slot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) }
}
Comment on lines +2174 to +2194

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

What Rust version stabilized std::ptr::fn_addr_eq, and is it available in core::ptr?

💡 Result:

The function core::ptr::fn_addr_eq was stabilized in Rust version 1.85.0 [1][2][3]. Yes, it is available in core::ptr [2][3][4], and it is also re-exported via std::ptr [5][6].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files matching slot/vm_ops/method.rs =="
fd -a 'slot\.rs$|vm_ops\.rs$|method\.rs$|slot_defs\.rs$' crates/vm/src | sed 's#^\./##'

echo
echo "== rust toolchain files =="
git ls-files | rg '(^|/)(rust-toolchain(\.toml)?|Cargo\.toml|Cargo\.lock)$' || true
if [ -f rust-toolchain ]; then cat -n rust-toolchain; fi
if [ -f rust-toolchain.toml ]; then cat -n rust-toolchain.toml; fi
if [ -f Cargo.toml ]; then rg -n 'edition|rust-version|rustc' Cargo.toml || true; fi

echo
echo "== fn_addr and fn_addr_eq occurrences =="
rg -n -C 2 '\bfn_addr\b|fn_addr_eq|transmute_copy' crates || true

echo
echo "== relevant slot.rs sections around fn_addr and call sites =="
python3 - <<'PY'
from pathlib import Path
p = Path("crates/vm/src/types/slot.rs")
lines = p.read_text().splitlines()
for i in [1, 940, 990, 2150, 2174]:
    start=max(1,i-5); end=min(len(lines),i+15)
    print(f"\n--- {p}:{start}-{end} ---")
    for n in range(start,end+1):
        print(f"{n:5}: {lines[n-1]}")
PY

echo
echo "== relevant vm_ops.rs section around existing fn_addr_eq =="
p="crates/vm/src/vm/vm_ops.rs"
lines=p.read_text().splitlines()
for start,end in [(330,375),(980,1045),(1060,1115)]:
    print(f"\n--- {p}:{start}-{end} ---")
    for n in range(start,end+1):
        if n <= len(lines):
            print(f"{n:5}: {lines[n-1]}")

echo
echo "== relevant slot_defs.rs copyslot_if_none section =="
p="crates/vm/src/types/slot_defs.rs"
if p.exists():
    lines=p.read_text().splitlines()
    rg -n "copyslot_if_none|fn_addr|fn_addr_eq" $p || true
    for start,end in [(1,80), (200,300), (300,400)]:
        matches=[i for i,l in enumerate(lines,1) if 'copyslot_if_none' in l or 'fn_addr' in l]
        if any(start-m <= s <= end+m for s in matches):
            print(f"\n--- {p}:{start}-{end} ---")
            for n in range(start,end+1):
                print(f"{n:5}: {lines[n-1]}")
else:
    fd -a 'slot_defs\.rs$' .

Repository: RustPython/RustPython

Length of output: 20995

Replace fn_addr with core::ptr::fn_addr_eq at the comparison sites.

core::ptr::fn_addr_eq is available in the configured Rust toolchain and gives the intended safe function-pointer identity comparison. Replace direct pair comparisons such as fn_addr(set) == fn_addr(del) with core::ptr::fn_addr_eq(set, del) or !core::ptr::fn_addr_eq(...). Keep the existing fn_addr_eq calls in crates/vm/src/vm/vm_ops.rs; where an intermediate Option<usize> is stored, derive it from fn_addr_eq instead of storing transmute_copy output.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/src/types/slot.rs` around lines 2174 - 2194, Remove the `fn_addr`
helper and replace its direct function-pointer comparison sites with
`core::ptr::fn_addr_eq`, using negation where the existing logic checks
inequality. Preserve the existing `fn_addr_eq` usage in `vm_ops.rs`; for
intermediate `Option<usize>` values, derive the value from `fn_addr_eq` rather
than storing `transmute_copy` addresses.

4 changes: 2 additions & 2 deletions crates/vm/src/types/slot_defs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/vm/method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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);
}

Expand Down
12 changes: 6 additions & 6 deletions crates/vm/src/vm/vm_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,21 @@ 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;
}

class_b
.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 {
Expand Down Expand Up @@ -302,21 +302,21 @@ 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;
}

class_b
.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 {
Expand Down
Loading