Inconsistent behavior when comparing function pointers in release

[repnop]

Playground link: https://play.rust-lang.org/?gist=62d362e2bf72001bd3f3c4a3ed0c842c&version=stable&mode=release&edition=2015

Switching to debug causes it to print both addresses twice (expected behavior), but in release both functions are optimized into one, so x and y are given the same address. @CryZe took a look at the assembly, and LLVM removed the first comparison (and evaluated it to a constant false), but not the second, which then evaluates to true. This is very reminiscent of undefined behavior in C and C++.

Thanks to @memoryruins for sending the original link which allowed me to discover this.

fn foo(i: i32) -> i32 {
    1
}
fn bar(i: i32) -> i32 {
    1
}

fn main() {
    let x: fn(i32) -> i32 = foo;
    let y: fn(i32) -> i32 = bar;
    
    if x == y {
        println!("true")
    } else {
        println!("{:?}, {:?}", x, y);
    }

    if x == y {
        println!("true")
    } else {
        println!("{:?}, {:?}", x, y);
    }
}

Debug output:

0x562a7b399600, 0x562a7b399620
0x562a7b399600, 0x562a7b399620

Release output:

0x560d1e3a1310, 0x560d1e3a1310
true

stable rustc 1.29.1 (b801ae6 2018-09-20)

[repnop]

Also worth noting (pointed out by @memoryruins) that MIRI throws an error in the comparison saying it dereferenced a function pointer.

[ishitatsuyuki]

We run mergefunc pass on release builds. Anyway, can you explain why this is a problem? Rust does not make any guarantee on pointer values.

[CryZe]

It seems like LLVM starts comparing the pointer of bar to bar instead of comparing bar to foo. This very much looks like LLVM is messing up in some way.

Unless LLVM is allowed to break function pointers entirely.

Actually that may be what mergefunc is doing.

[Havvy]

@ishitatsuyuki The problem is that they shouldn't produce an x == y expression that is false in one statement and then true in the next. They should either always be equal or never be equal, but this code snippet shows that this is not always true.

@rep-nop Can you include the source code from the playground and the compilation results (under release and debug) into your opening comment?

[eddyb]

(came here from #54695)

According to @rkruppe, this could be caused by constant-folding &a == &b to false in LLVM (when a and b are different globals).
IMO this optimization shouldn't apply to functions and/or any unnamed_addr globals.

cc @rust-lang/compiler

[VictorKoenders]

Insanity Is Doing the Same Thing Over and Over Again and Expecting Different Results

[strega-nil]

This is definitely a bug in LLVM, afaict - this expression should never be true in C++. It may be that they never set unnamed_addr on functions?

[VictorKoenders]

• Still happens with no_mangle
• Still happens with pub
• Still happens with extern "C"

[hanna-kruppe]

@ubsan Yeah Clang doesn't add unnamed_addr anywhere as aggressively as rustc, and unnamed_addr is the one that allows completely merging the functions (which is required for the comparison to return true).

[strega-nil]

@rkruppe seems like someone added an optimization that assumes that functions are always distinct, even when unnamed_addr. We might want to open a bug on LLVM?

[hanna-kruppe]

I don't see why this would be an LLVM bug. The observed behavior seems perfectly in line with the meaning of unnamed_addr: the address is not significant, so address comparisons can evaluate either way. If a frontend does not want that, it shouldn't add unnamed_addr to functions whose address is observed. (Not saying that's what rustc should do, if it came down to it I would argue for the current behavior, just describing what would probably be a reasonable position of LLVM developers.)

[RalfJung]

Oh this is beautiful. ;) Finally we have a real-life example of pointer comparison being non-deterministic. It's not at all how I expected that example to look, though...

Non-determinism does not mean that we have UB, so I do not think there is a huge problem here. But non-deterministic comparison is certainly surprising. It means, for example, that the PartialEq implementation for function pointers does not satisfy the PartialEq contract, which is something we should keep in mind.

I suppose we could also have an analysis to see which functions have their address taken, and not set unnamed_addr for them?