Skip to content

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

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:fix-miri-fn-ptr-provenance
Aug 2, 2026
Merged

Fix miri UB: use transmute_copy for function pointer identity checks#8432
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:fix-miri-fn-ptr-provenance

Conversation

@youknowone

@youknowone youknowone commented Aug 2, 2026

Copy link
Copy Markdown
Member

Problem

Nightly miri now flags function pointer identity checks as UB. Both fn_ptr as usize and direct fn_ptr == fn_ptr internally call FnPtr::addr(), which attempts to dereference the function pointer's provenance. Since function items have no backing allocation in miri's model, this triggers:

error: Undefined Behavior: pointer not dereferenceable: pointer must point to
some allocation, but got 0x20cb6e[noalloc] which is a dangling pointer

Solution

Add fn_addr<T>(f: T) -> usize in crate::types that uses transmute_copy to read the function pointer's address as plain integer bytes, bypassing FnPtr::addr() entirely:

#[inline(always)]
pub(crate) fn fn_addr<T: Copy>(f: T) -> usize {
    assert!(core::mem::size_of::<T>() == core::mem::size_of::<usize>());
    unsafe { core::mem::transmute_copy::<T, usize>(&f) }
}

Replace all f as usize function pointer comparison patterns across the codebase (11 files, ~20 call sites) with fn_addr(f).

Also add -Zmiri-permissive-provenance to CI MIRIFLAGS as a safety net for any remaining integer-pointer round-trips elsewhere.

Files changed

  • types/slot.rs — add fn_addr utility, update slot comparisons
  • types/slot_defs.rsinit slot SLOTDEFINED check
  • class.rshash_not_implemented and __new__ slot checks
  • frame.rsgetattro, setattro, tp_new, tp_alloc specialization guards
  • builtins/type.rstp_new safety check, new_wrapper detection
  • builtins/object.rstp_init vs object.__init__
  • builtins/set.rs — frozenset tp_init check
  • stdlib/_thread.rsLocal custom init detection
  • vm/method.rsgetattro default check
  • vm/vm_ops.rs — binary/ternary number op slot identity
  • .github/workflows/ci.yaml — add -Zmiri-permissive-provenance

Verification

MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-permissive-provenance" cargo +nightly miri test -p rustpython-vm --lib -- miri_test
# 2 passed; 0 failed

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved function-pointer handling across object initialization, type dispatch, slot comparisons, method access, and numeric operations.
    • Preserved existing validation, dispatch ordering, and fallback behavior while improving compatibility with strict provenance checks.
  • Tests

    • Updated CI configuration to support provenance-aware runtime checks.

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
Copilot AI review requested due to automatic review settings August 2, 2026 14:51
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a shared fn_addr helper and replaces direct function-pointer-to-usize casts across VM slot checks, specialization guards, method access, thread initialization, and numeric dispatch. The Miri workflow enables permissive provenance.

Changes

Function address provenance

Layer / File(s) Summary
Shared address conversion
crates/vm/src/types/slot.rs, .github/workflows/ci.yaml
Adds fn_addr using transmute_copy and enables permissive provenance in Miri.
Slot identity checks
crates/vm/src/types/*, crates/vm/src/builtins/*, crates/vm/src/class.rs
Uses fn_addr for initializer, __new__, hash, accessor, and descriptor comparisons.
Runtime dispatch checks
crates/vm/src/frame.rs, crates/vm/src/vm/*, crates/vm/src/stdlib/_thread.rs
Uses fn_addr in specialization guards, method access, thread initialization, and numeric operator dispatch. Existing control flow remains unchanged.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: z-ca-2026

Suggested reviewers: copilot, shaharnaveh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: fixing Miri undefined behavior in function pointer identity checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses new Miri Undefined Behavior reports caused by function-pointer identity comparisons in the VM by introducing a helper that extracts a function pointer’s raw address without invoking FnPtr::addr() and then using it at various slot-comparison call sites.

Changes:

  • Add fn_addr helper (via transmute_copy) for function-pointer identity checks under Miri.
  • Replace a set of fn_ptr as usize-style slot/function identity comparisons with fn_addr(...).
  • Update the Miri CI job flags to include -Zmiri-permissive-provenance.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/vm/src/types/slot.rs Adds fn_addr utility and updates slot function comparisons to use it.
crates/vm/src/types/slot_defs.rs Uses fn_addr for init slot “slot defined” checks.
crates/vm/src/class.rs Switches slot function identity checks (e.g., __hash__, __new__ inheritance) to fn_addr.
crates/vm/src/frame.rs Updates specialization guards to compare slot functions via fn_addr.
crates/vm/src/builtins/type.rs Uses fn_addr for tp_new safety/identity checks.
crates/vm/src/builtins/object.rs Compares tp_init via fn_addr rather than integer casts.
crates/vm/src/builtins/set.rs Updates frozenset init identity check to use fn_addr.
crates/vm/src/stdlib/_thread.rs Updates Local init detection to use fn_addr.
crates/vm/src/vm/method.rs Uses fn_addr for default getattro detection.
crates/vm/src/vm/vm_ops.rs Uses fn_addr for number-slot address comparisons in binary/ternary dispatch.
.github/workflows/ci.yaml Adds -Zmiri-permissive-provenance to MIRIFLAGS for the Miri CI job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yaml
Comment on lines +651 to +654
# 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"

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@crates/vm/src/types/slot.rs`:
- Around line 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.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 83cc0a0a-2699-4345-9373-16dabd92b5c4

📥 Commits

Reviewing files that changed from the base of the PR and between 1306b71 and 46842c9.

📒 Files selected for processing (11)
  • .github/workflows/ci.yaml
  • crates/vm/src/builtins/object.rs
  • crates/vm/src/builtins/set.rs
  • crates/vm/src/builtins/type.rs
  • crates/vm/src/class.rs
  • crates/vm/src/frame.rs
  • crates/vm/src/stdlib/_thread.rs
  • crates/vm/src/types/slot.rs
  • crates/vm/src/types/slot_defs.rs
  • crates/vm/src/vm/method.rs
  • crates/vm/src/vm/vm_ops.rs

Comment on lines +2174 to +2194

/// 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) }
}

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.

@youknowone
youknowone merged commit 95f9d17 into RustPython:main Aug 2, 2026
27 checks passed
@youknowone
youknowone deleted the fix-miri-fn-ptr-provenance branch August 2, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants