Skip to content
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.

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

Function pointer address compares are misleading #2882

Closed
MasterAwesome opened this issue May 9, 2023 · 6 comments
Closed

Function pointer address compares are misleading #2882

MasterAwesome opened this issue May 9, 2023 · 6 comments

Comments

@MasterAwesome
Copy link

Consider the case where I want to take any type T and convert it into some Opaque struct as so, comparing just the function pointers would give me a meaningful way to determine that this Opaque was created by the opaquify function. To ensure that generics that alias because of similar implementations (as shown below) don't cause undefined behavior the type_name field is used.

use std::ffi::c_void;

#[derive(Debug)]
#[repr(C)]
struct Opaque {
    fptr: extern "C" fn(),
    type_name: &'static str,
    inner: *mut c_void,
}

fn opaquify<T>(val: T) -> Opaque {
    dbg!(std::any::type_name::<T>());
    dbg!(ffi::<T> as *const c_void);
    Opaque {
        fptr: ffi::<T>,
        type_name: std::any::type_name::<T>(),
        inner: Box::into_raw(Box::new(val)).cast(),
    }
}

fn unopaquify<T>(val: &Opaque) -> Option<&T> {
    dbg!(val);
    if val.fptr == ffi::<T> && val.type_name == std::any::type_name::<T>() {
        unsafe { Some(val.inner.cast::<T>().as_ref().unwrap_unchecked()) }
    } else {
        None
    }
}

extern "C" fn ffi<T>() {
    /* ... */
}

pub fn main() {
    let t1 = opaquify(1_u32);
    let t2 = opaquify(1_usize);

    assert!(unopaquify::<usize>(&t1).is_none());
    assert!(unopaquify::<u32>(&t2).is_none());


    assert_eq!(unopaquify::<u32>(&t1), Some(&1));
    assert_eq!(unopaquify::<usize>(&t2), Some(&1));
}

This above program executes as expected on debug and release however something weird happens when I introduce miri into the mix. Below is the comparisons between outputs on different modes.

Debug builds

[src/main.rs:12] std::any::type_name::<T>() = "u32"
[src/main.rs:13] ffi::<T> as *const c_void = 0x000055be50049d20
[src/main.rs:12] std::any::type_name::<T>() = "usize"
[src/main.rs:13] ffi::<T> as *const c_void = 0x000055be50049d10
[src/main.rs:22] val = Opaque {
    fptr: 0x000055be50049d20,
    type_name: "u32",
    inner: 0x000055be50421ba0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055be50049d10,
    type_name: "usize",
    inner: 0x000055be50421bc0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055be50049d20,
    type_name: "u32",
    inner: 0x000055be50421ba0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055be50049d10,
    type_name: "usize",
    inner: 0x000055be50421bc0,
}

Here we can see that generic fptrs don't alias and behaves as expected.

Release mode

[src/main.rs:12] std::any::type_name::<T>() = "u32"
[src/main.rs:13] ffi::<T> as *const c_void = 0x000055613651b6c0
[src/main.rs:12] std::any::type_name::<T>() = "usize"
[src/main.rs:13] ffi::<T> as *const c_void = 0x000055613651b6c0
[src/main.rs:22] val = Opaque {
    fptr: 0x000055613651b6c0,
    type_name: "u32",
    inner: 0x0000556137583ba0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055613651b6c0,
    type_name: "usize",
    inner: 0x0000556137583bc0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055613651b6c0,
    type_name: "u32",
    inner: 0x0000556137583ba0,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000055613651b6c0,
    type_name: "usize",
    inner: 0x0000556137583bc0,
}

Here functions do alias because of presumably an LLVM pass that merges functions but because of the type check unopaquify knows when it's safe to convert.

Miri

Run with: MIRIFLAGS="-Zmiri-ignore-leaks" cargo miri run

[src/main.rs:12] std::any::type_name::<T>() = "u32"
[src/main.rs:13] ffi::<T> as *const c_void = 0x000000000002f170
[src/main.rs:12] std::any::type_name::<T>() = "usize"
[src/main.rs:13] ffi::<T> as *const c_void = 0x0000000000046572
[src/main.rs:22] val = Opaque {
    fptr: 0x000000000003d3a3,
    type_name: "u32",
    inner: 0x000000000003d508,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x00000000000548f9,
    type_name: "usize",
    inner: 0x0000000000054a90,
}
[src/main.rs:22] val = Opaque {
    fptr: 0x000000000003d3a3,
    type_name: "u32",
    inner: 0x000000000003d508,
}
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `None`,
 right: `Some(1)`', src/main.rs:41:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The dbg logs from opaquify suggests that fptr do NOT alias and should follow the same branches as debug build. However, the first unopaquify call has an fptr which has the value completely different than any of the ones returned. I'm not sure what 0x000000000003d3a3 or 0x00000000000548f9 point to but based on the initial logs it's neither ffi::<usize> nor ffi::<u32>. And hence the tests fail.

Is this an expected false positive from miri and can be safely ignored, or is this actual UB that I'm failing to understand?

https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=c7009ed9436d42f9d78d59d8ef95abd5

@MasterAwesome MasterAwesome changed the title Function pointer address compares are misleading on miri Function pointer address compares are misleading with miri May 9, 2023
@MasterAwesome MasterAwesome changed the title Function pointer address compares are misleading with miri Function pointer address compares are misleading May 9, 2023
@oli-obk
Copy link
Contributor

oli-obk commented May 9, 2023

In miri, every time you create a function pointer for a function, you get a new address. The same function pointer will be equal to itself, but never equal to any other function pointer. This is pessimistic, as there are situations where rustc + llvm can actually behave this way (although usually not as extreme). rust-lang/rust#70861 is related, but I think you can get the same behaviour with comparisons instead of matches.

@RalfJung
Copy link
Member

RalfJung commented May 9, 2023

Basically Rust makes no guarantees at all when it comes to fn ptr equality: the 'same' function can have multiple addresses (e.g. if it is called from multiple codegen units / crates, leading to multiple monomorphizations) and different functions can have the same address.

should follow the same branches as debug build

Note that debug and release builds are just 2 of the many many possible ways a Rust program can execute, depending on compiler versions, crate structure, etc. If you go into unspecified territory, you can't just consider the 2 cases of debug and release builds.

@RalfJung
Copy link
Member

RalfJung commented May 9, 2023

In miri, every time you create a function pointer for a function, you get a new address.

Specifically, the ffi::<T> in your code are coercion sites: here a fn item gets coerced to a fn ptr. For type-generic functions, each such coercion generates a fresh address, simulating the case where each such coercion occurs in a different crate that receives its own monomorphized copy of this function.

In opaquify, there are two such coercions, meaning the address printed and the address stored in the returned opaque handle are different. In unopaquify there is a third coercion site, so that will give yet another address.

Arguably the first two coercions will always be in the same crate + codegen unit since they occur in the same function, so one could consider guaranteeing that they will yield the same address (but Rust currently documents no such guarantee). However crucially your entire construction relies on the coercion site in unopaquify also giving the same address, and that can be violated in practice if the calls to opaquify and unopaquify occur in different crates.

@MasterAwesome
Copy link
Author

MasterAwesome commented May 9, 2023

Arguably the first two coercions will always be in the same crate + codegen unit since they occur in the same function, so one could consider guaranteeing that they will yield the same address (but Rust currently documents no such guarantee). However crucially your entire construction relies on the coercion site in unopaquify also giving the same address, and that can be violated in practice if the calls to opaquify and unopaquify occur in different crates.

Ah I see, thank God I checked miri :)

Maybe then this would be a follow up design question, how do I model such a thing to work across crates? There might be a case where I might not be able to look at the type_name or inner without ensuring the function pointer matches however if every time I coerce a fn item to a fn pointer it's not guaranteed to be constant, is there any way around it?

@RalfJung
Copy link
Member

RalfJung commented May 9, 2023

I'm not sure what the established best practices are for type erasure. I'd suggest asking around on Zulip. :)

@MasterAwesome
Copy link
Author

Thanks @RalfJung and @oli-obk I will continue this discussion on Zulip!

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

No branches or pull requests

3 participants