Invalid equality check result for usizes obtained from function pointers

[Riateche]

Code:

fn main() {
    fn x() {}
    fn y() {}

    let a = x as usize;
    let b = y as usize;
    
    assert_eq!(a, b);
}

Output in release mode:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `94740677858160`,
 right: `94740677858160`', src/main.rs:8:5

Playground

There is a similar issue about comparing function pointers, but incorrect comparison of usize may be more dangerous. For example, if unsafe code checks the equality between usize to uphold its invariant, passing values obtained this way may break the invariant, even though the unsafe code itself didn't work with function pointers.

[RalfJung]

In some sense there's no bug here, this is expected: comparing function pointers should behave the same with or without the usize cast.

What is more surprising is that they print the same. This indicates that comparison is non-deterministic... which is rather problematic indeed, and could break plenty of LLVM optimizations.

[RalfJung]

Indeed this can be turned into a demonstration of non-deterministic comparison:

#[inline(never)]
fn nop(x: usize) -> usize { x }

fn main() {
    fn x() {}
    fn y() {}

    let a = x as usize;
    let b = y as usize;
    
    assert_eq!(nop(a), nop(b));
    assert_eq!(a, b);
}

The second assertion fails, the first passes (in release mode on 1.45.2). That's no good. In principle this could lead to a soundness bug, though I am not sure if that can be actually done in practice.

Cc @rust-lang/wg-unsafe-code-guidelines @rust-lang/lang

[RalfJung]

This is an LLVM bug: https://bugs.llvm.org/show_bug.cgi?id=47090

[RalfJung]

But MIR optimizations also have to be aware that folding equality tests on functions is subtle (Cc @rust-lang/wg-mir-opt). And @oli-obk recently had to prevent me from making a similar mistake for pointer equality tests in CTFE (Cc @rust-lang/wg-const-eval).

[RalfJung]

The LLVM bug has been fixed. @Mark-Simulacrum (pinging you in lieu of there being a way to ping the LLVM WG), do you usually backport such fixes or wait for the next release?

[Mark-Simulacrum]

cc @cuviper @nikic @nagisa (pinging LLVM WG)

I suspect we'll want to backport the patch ourselves, and try to nominate it for LLVM 11 (since that's not yet released, and we're currently on a release candidate). https://reviews.llvm.org/D87123 is the thing to backport I think.

[apiraino]

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

[cuviper]

Discussion on the LLVM bug is that it's too late for 11.0.0, but could be added to 11.0.1.

If someone wants to see the fix sooner in rust's fork, I'm happy to review PRs:
https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html#bugfix-updates