`fn_addr_eq` docs suggest incorrect reasoning

[RalfJung]

The docs for fn_addr_eq say

Despite these false positives and false negatives, this comparison can still be useful. Specifically, if

• T is the same type as U, T is a subtype of U, or U is a subtype of T, and
• ptr::fn_addr_eq(f, g) returns true,

then calling f and calling g will be equivalent.

This is not correct, I think.
Consider this example:

unsafe fn f(x: i32) -> i32 { x }
unsafe fn g(x: i32) -> i32 { std::hint::assert_unchecked(x != 0); x }

fn main() {
  let fptr = f as unsafe fn(i32) -> i32;
  let gptr = g as unsafe fn(i32) -> i32;
  if std::ptr::fn_addr_eq(fptr, gptr) {
    // SAFETY: Calling `fptr(0)` here would be allowed, and according to the `fn_addr_eq`
    // docs, inside this `if` calling `gptr` is equivalent to calling `fptr`.
    unsafe { gptr(0) };
  }
}

The compiler may now do the following:

• Realize that gptr points to g, and use that to inline the call to g.
• Optimize the then branch to std::hint::assert_unchecked(false), and then replace it by arbitrary code (e.g. a trap).
• Later during compilation, merge f and g into one function since they compile to the same assembly.

Now if we run this program we hit the trap, i.e., we have exhibited UB.
(See rust-lang/unsafe-code-guidelines#589 for a wider discussion of the problem. When discussing this we were not aware that our libs docs actually make any claims in this regard.)

Cc @rust-lang/opsem @rust-lang/libs-api

[RalfJung]

FWIW this example also shows that it would be unsound for LLVM to replace one function pointer by another even if they are equal. Which in turn implies function pointers must have provenance. Cc @nikic

[theemathas]

Another alternative for LLVM would be to disable devirtualization as soon as we "observe" a function pointer's address.

[RalfJung]

That would entirely kill devirtualization since LLVM will basically ~never be able to prove that we did not observe a function's address.

Consider that the gptr call can be arbitrarily deep inside other functions and whatnot. Our SAFETY reasoning is allowed to span across functions.

[ds84182]

During compilation LLVM makes no assumptions about function addresses since the final address is decided by the static or dynamic linker. So LLVM cannot optimize that function address equality check to true or false.

The docs for fn_addr_eq should be changed to remove the equivalence guarantee.

I don't see how LLVM replacing function pointers leads to unsoundness after the docs are changed.

[RalfJung]

 So LLVM cannot optimize that function address equality check to true or false.

We agree on that. No part of my argument involved optimizing the equality check so I am not sure why you bring that up.

I don't see how LLVM replacing function pointers leads to unsoundness after the docs are changed.

Indeed, and nobody claimed otherwise as far as I know.

[RalfJung]

The docs for fn_addr_eq should be changed to remove the equivalence guarantee.

So the docs will just say "this returns a bool, you cannot do anything useful with it"...

I don't have a better idea either. :/

[asquared31415]

The concerns that comparing function pointers is not very useful were brought up in #118833 which added a lint against comparing function pointers (because it's inherently a sketchy operation). Then fn_addr_eq was spawned off that lint, for the use case of "i want to do it anyway", but the concerns about what it means didn't come up in the ACP, implementation, or tracking issue, and were never addressed.