New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 47090 - icmp gives inconsistent result for unnamed functions
Summary: icmp gives inconsistent result for unnamed functions
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Scalar Optimizations (show other bugs)
Version: trunk
Hardware: PC Linux
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks: release-11.0.1
  Show dependency tree
 
Reported: 2020-08-10 04:42 PDT by Ralf Jung
Modified: 2020-10-30 19:00 PDT (History)
8 users (show)

See Also:
Fixed By Commit(s): d751f86 38399ced95b


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ralf Jung 2020-08-10 04:42:50 PDT
A Rust user found a case where comparison with icmp is non-deterministic: https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=8affd2a65b02a35e16dbab4682cb8886.

What this does is it take pointers to two functions that are marked as "unnamed", casts those pointers to integers, and then compares them twice -- once after passing them through a "black box" noinline function, and once immediately. The compiler seemingly optimized the latter comparison to "false", but later makes the two function's addresses identical.

If we describe the semantics of this program at the LLVM IR level, then I think we have to either accept that "icmp" can produce different results for the same input (it is non-deterministic), or the program has to have UB.  But it's not UB to compare "unnamed function" pointers, right?  My expectation was that unnamed function can end up having the same or different addresses depending on optimizer decisions etc, but whether it is one or the other has to be consistent throughout a single execution of the program.
Comment 1 Florian Hahn 2020-08-10 05:06:21 PDT
I looked at the LLVM IR output for the link. It seems like there is no reference to the original `y` function and it uses `@_ZN10playground4main1x17h0ad53127b32611bfE` in place for both the x and y functions? Not sure if the LLVM IR shown on play.rust-lang.org is before or after LLVM optimizations.

I guess it would be helpful to see the original LLVM IR before optimizations to see if/where things go wrong in LLVM.
Comment 2 Ralf Jung 2020-08-10 05:35:21 PDT
I think it is after optimizations, but you can get a version with fewer optimizations by switching to debug mode. Then I see

  store i64 ptrtoint (void ()* @_ZN10playground4main1x17h1c7cdf3c5e1284aeE to i64), i64* %a, align 8, !dbg !343
  store i64 ptrtoint (void ()* @_ZN10playground4main1y17haac361c2b5fa2217E to i64), i64* %b, align 8, !dbg !344

Note the "x" vs "y" after "4main1".
Comment 3 Florian Hahn 2020-09-06 07:43:20 PDT
Patch available: https://reviews.llvm.org/D87123
Comment 4 Eli Friedman 2020-09-11 17:25:30 PDT
d751f86
Comment 5 Josh Stone 2020-09-16 14:54:13 PDT
Is this change feasible to backport to 11?
(It does cherry-pick cleanly, at least...)
Comment 6 Eli Friedman 2020-09-16 15:51:56 PDT
It's too late to take a minor fix like this into 11.0; the release is already running late.

For 11.0.1, maybe?  It's safe enough, but it's not a regression, and the issue hasn't shown up in any non-synthetic code as far as I know.
Comment 7 Ralf Jung 2020-09-16 23:28:44 PDT
The issue has shown up in Rust: https://github.com/rust-lang/rust/issues/73722.
Whether the user ran into this while minimizing a real problem or while just trying to figure out the semantics of function pointers in Rust I cannot tell, but as a miscompilation bug we consider this a critical bug in Rust.
Comment 8 Eli Friedman 2020-09-17 16:14:09 PDT
Then sure, we can pull it into 11.0.1.  If I don't remember, please add it as a blocker to the release-11.0.1 bug once it exists.
Comment 9 Tom Stellard 2020-10-30 19:00:00 PDT
Merged: 38399ced95b