Inconsistent behavior when comparing function pointers in release

@CryZe

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)

@memoryruins

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

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.

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.

@ishitatsuyuki

@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?

(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

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

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?

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

@ubsan

@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).

@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?

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.)

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?

@RalfJung

@RalfJung

What would be a problem would be if LLVM also assumes that equality is deterministic and duplicates 2 occurrences of the comparison.

@arielb1

@arielb1

What would be a problem would be if LLVM also assumes that equality is deterministic and duplicates 2 occurrences of the comparison.

That is a very good point. I don't think there is anything preventing that, and in principle that could lead to unsoundness. However, this problem applies to all comparisons that might be non-deterministic, right? So "just don't use unnamed_addr" wouldn't solve it.

@RalfJung

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

LLVM already does that, or more precisely, it adds unnamed_addr to functions that are guaranteed to not have their address taken. This is only knowable for internal functions, so it's a significant downgrade from the status quo in non-LTO builds. It also can't distinguish e.g. between only putting a function in a vtable (which is not accessible to users and only ever used for calling) and reifying a function pointer in user code. The latter could be solved by an analysis in rustc, but the former is a fundamental limitation of separate compilation.

@arielb1

@arielb1 Ah right, forgot about that... so, is it UB under LLVM IR to take the address of a function tagged unnamed_addr?

However, this problem applies to all comparisons that might be non-deterministic, right?

True, but it is not clear if there are any. I cannot give a satisfactory definition of LLVM semantics that makes ptr comparison deterministic, but I also was unable to find an example that would demonstrate that it is not deterministic.

Auto merge of #5294 - tmiasko:trait-ptr-cmp, r=Manishearth

This issue was fixed in https://reviews.llvm.org/D87123

Tagging as "needs test" -- do we have a minimal example?

Rollup merge of rust-lang#86398 - yerke:add-test-for-issue-54685, r=M…

Auto merge of rust-lang#86527 - JohnTitor:rollup-cbu78g4, r=JohnTitor

Navigation Menu

Repository navigation

Inconsistent behavior when comparing function pointers in release #54685

16 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Inconsistent behavior when comparing function pointers in release #54685

Description

Activity

repnop commented on Sep 30, 2018

ishitatsuyuki commented on Sep 30, 2018

CryZe commented on Sep 30, 2018

Havvy commented on Sep 30, 2018

eddyb commented on Sep 30, 2018

VictorKoenders commented on Sep 30, 2018

strega-nil commented on Sep 30, 2018

VictorKoenders commented on Sep 30, 2018

hanna-kruppe commented on Sep 30, 2018

strega-nil commented on Sep 30, 2018

hanna-kruppe commented on Sep 30, 2018

RalfJung commented on Oct 1, 2018

arielb1 commented on Oct 1, 2018

hanna-kruppe commented on Oct 1, 2018

RalfJung commented on Oct 1, 2018

16 remaining items

tmiasko commented on May 23, 2021

nikomatsakis commented on May 24, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions