fn_addr_eq: we actually can guarantee basically nothing by RalfJung · Pull Request #162140 · rust-lang/rust

RalfJung · 2026-09-01T17:57:19Z

Fixes #160202.
See that issue for context and discussion.

rustbot · 2026-09-01T17:57:25Z

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, joboet

the8472 · 2026-09-01T18:14:03Z

Sign in to view

 ///   (From an implementation perspective, this is possible because functions may sometimes be
 ///   processed more than once by the compiler, resulting in duplicate machine code.)
 ///
-/// Despite these false positives and false negatives, this comparison can still be useful.


One potentially remaining use we were able to come up with is a set of callbacks.

If deduplicating additions to the set by "the runtime behavior should happen at least once" works for you then this should work fine.

View changes since the review

That seems incredibly niche.

Would you like to see it spelled out in the docs?

Well, the more general idea was that it might be helpful to also have a list of the few things that it can be used for, it might help users to figure out which side they fall on.

That's just the only practical thing that came to mind.

Wouldn't cases like the "different unsafe preconditions" example above cause incorrect results for this kind of use case? Equality returning true does not guarantee that they have the same behavior, so deduplicating in that scenario is not correct.

If the callbacks are all required to be safe to call, then calling fewer of them due to deduplication cannot introduce UB.

There is some sense in which equal functions have the same behavior -- they have the same asm code, after all. We might be able to give a guarantee of the sort "if neither function causes UB for the given arguments, then calling f is equivalent to calling g"?

Cc @rust-lang/opsem

Suppose there is a bunch of function pointers. They are compared and called.
All function called are identified at compile time. All calls are inlined.
Functions are never called, so their body is optimized out.
Functions are merged together. All function have the same address.

Ok, having reread that... I guess that depends on whether the comparison lets the function addresses escape like black_box would? Then the optimizer can't remove their bodies. I think even whole-program optimization wouldn't allow it since something could temporarily encrypt the bits of the pointers in some way.

Not fully arbitrarily, no. It's still the case that it must be source code that can compile to the same asm as other source code. The docs have a "guaranteed not equal" example of two functions that print different strings; that guarantee sounds legit to me.

The example perfectly demonstrates my argument from #162140 (comment). Those functions are never called. Their bodies could be optimized out as unreachable, function merged, and function pointer would compare equal.

Hm... I don't think LLVM considers this a legal transformation today, but I'm also not sure how to forbid it in the spec.

I have removed the example.

Hm... I don't think LLVM considers this a legal transformation today, but I'm also not sure how to forbid it in the spec.

LLVM won't do that transform today, but I think it would be legal? Comparison only escapes the address, not the provenance, so the function cannot be called, and optimizing it to unreachable is allowed.

clarfonthey · 2026-09-02T00:52:46Z

+/// never introduce undefined behavior because it only skips executing some of the registered
+/// callbacks.
 ///
 /// # Examples


Would also just add here: in general, while there may be niche cases that this can be legitimately useful, in pretty much all cases you're also encouraged to refactor your code anyway to avoid the pitfalls. For example, if you make your functions instead be ZSTs that implement CustomTrait and use Arc<dyn CustomTrait>, now all of a sudden you can use Arc::ptr_eq and expect it to mean what you want. In general avoiding function pointers is just more likely to always do the correct thing and not be noticeably worse in performance in ways that matter.

View changes since the review

I think you mean Arc::ptr_eq.

that said, comparing &dyn CustomTrait with std::ptr::addr_eq also has pitfalls, so I wouldn't necessarily recommend it. e.g. two different &dyn CustomTrait can compare equal with addr_eq, but have different behavior:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=9c9028b37168b6e402b51eb4996b2eef

Right, I forgot that vtables are genuinely different even though the pointers may be the same, especially for ZSTs.

That said, if you use Arc::ptr_eq, you should refer to the same reference-counted version of a dyn CustomTrait, even if the unsized pointers may compare identically. And really, my point was that there are ways of achieving infallible equality in ways that make more sense for your program that don't rely on function pointer equality.

theemathas · 2026-09-02T06:28:51Z

+/// These limitations imply that comparing function pointers (via this function or via `==`) is only
+/// useful in extremely niche circumstances. For instance, if a library maintains a set of
+/// callbacks, then it can be legimiate to deduplicate callbacks based on `fn_addr_eq`. This can
+/// never introduce undefined behavior because it only skips executing some of the registered
+/// callbacks.
 #[stable(feature = "ptr_fn_addr_eq", since = "1.85.0")]


I believe that this is not legitimate. Suppose that one of the callbacks exposes the provenance of one pointer, while the other callback doesn't, and they have the same machine code. This deduplication can cause a pointer's provenance to not be exposed, even though it would have been exposed without the deduplication. This can technically increase the amount of UB.

View changes since the review

If the library just says "I will call some of these callbacks but not all of them", that's legitimate.

Similar issues can happen without exposed provenance if one of the callbacks mutates some byte by adding provenance to it.

I'm trying to understand how that might cause problems where you run fewer copies of a callback than otherwise expected, though. Like, if the provenance is needed and isn't provided.

expose_provenance is a side-effect. If something else relies on the side-effect having happened, then they're toast.

So this amounts to the library basically saying "callbacks may spuriously just not be run for arbitrary reasons". I don't know when that is ever useful, but it's a spec one can write and deduplication with fn_addr_eq does satisfy that spec.

I have edited the text to make this more clear. I'm still not sure this example is worth actually spelling out, but t-libs seems to prefer having an extremely artificial example over not having any example.

To be clear, I'm not 100% sure that's a blocking opinion from libs, just, we really struggled to find examples and wanted to justify this function's existence somehow for a reason other than just documenting how bad function equality is. It does still feel worth having some kind of attribute like #[diagnostic::discouraged] to force people to allow a lint that explains why you almost always do not want to use this function.

Since, well, the main issue is that we know that if we don't offer this function, people are going to try and find a way to hack in its functionality anyway, e.g. casting to non-function pointers. So, we might as well offer a better way if it's already possible, but still heavily discourage it.

It does still feel worth having some kind of attribute like #[diagnostic::discouraged] to force people to allow a lint that explains why you almost always do not want to use this function.

I don't disagree, but I'm not signing up for being the one to implement that.

Maybe just keep:

These limitations imply that comparing function pointers (via this function or via ==) is only useful in extremely niche circumstances.

...and cut the rest?

scottmcm · 2026-09-02T15:33:40Z

I'm pretty torn here because this feels alot to me like the LLVM bugs we know about around ptr2int+int2ptr, which we've generally filed more under "that's a bug we hope to get fixed" not "we should change the documented behaviour".

Deduping on LLVM-IR is fine, right? So this is just about whether things are also deduping on the machine code? How often are we emitting things where the former isn't doing most of the work? (Certainly having no deduping would be really bad for lots of small functions, but things like deduping swap<u32> and swap<i32> don't need binary-level deduping.)

traviscross · 2026-09-02T15:36:53Z

Makes sense to me. Thanks @RalfJung.

(Proposing FCP as we're retracting language guarantees. Based on discussion on the call, people will likely leave some review comments about strengthening the guidance to users in the text without strengthening the guarantees.)

@rfcbot fcp merge lang

ChayimFriedman2 · 2026-10-08T17:31:10Z

I wonder if it makes sense for Miri to always, or randomly, return true for this function.

asquared31415 · 2026-10-08T17:59:34Z

Miri cannot know when returning true would be acceptable, and it does not have a concept of "machine code". Returning true when the pointers are the same value, and conservatively returning false in all other cases is the only thing that Miri can implement.

RalfJung · 2026-10-08T18:14:00Z

Every function has an integer address in Miri. There's nothing to be "conservative" about. In fact generic functions get more than one address (we pick a new one on every fn-item-to-fn-ptr cast), so Miri already sometimes returns false when you use fn_addr_eq on two pointers to "the same" function. However, Miri currently never gives the same address to two "different" functions (as in, different ty::Instance).

Miri could give all functions the same address. Or bucket all functions onto 16 different addresses, or so. Though we probably should only do that with -Zmiri-strict-provenance as otherwise we have no way to implement int2ptr casts for function pointers.

clarfonthey · 2026-10-08T20:43:55Z

I feel like giving all functions the same address should only be enabled via a -Zmiri-is-evil=yes flag.

ChayimFriedman2 · 2026-10-08T21:01:54Z

I feel like giving all functions the same address should only be enabled via a -Zmiri-is-evil=yes flag.

It's a good way to test code but the problem is that it could also hide the other path.

clarfonthey · 2026-10-08T22:24:27Z

It's a good way to test code but the problem is that it could also hide the other path.

Exactly what an evil tool would do, yes.

rust-rfcbot · 2026-10-10T16:03:00Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

RalfJung · 2026-10-10T16:07:05Z

@joboet this is ready for your review. :)

Or maybe someone else who's reading along wants to take this?

clarfonthey · 2026-10-10T18:30:12Z

I can take over if needed, although I'll let @joboet decide for now

joboet · 2026-10-12T09:30:42Z

FCP has completed and this reads fine, so
@bors r+ rollup

rust-bors · 2026-10-12T09:30:46Z

📌 Commit b623ba1 has been approved by joboet

It is now in the queue for this repository.

fn_addr_eq: we actually can guarantee basically nothing Fixes rust-lang#160202. See that issue for context and discussion.

…uwer Rollup of 8 pull requests Successful merges: - #164196 (`rust-analyzer` subtree update) - #164037 (Restrict application of optimization attributes from instance) - #164201 (Improve `--bless` output for `tests/debuginfo`) - #164202 ([rustdoc] Move the creation of `IndividualTestOptions` into the test runner closure to ensure the temporary folder is removed right after the test is done running) - #162140 (fn_addr_eq: we actually can guarantee basically nothing) - #163946 (Fix transmutability type mismatch bug) - #163978 (Copy over some methods from At to InferCtxt) - #164045 (-Zassumptions-on-binders: replace region vars with bound vars when handling alias outlives')

Rollup merge of #162140 - RalfJung:fn_addr_eq, r=joboet fn_addr_eq: we actually can guarantee basically nothing Fixes #160202. See that issue for context and discussion.

RalfJung added the I-libs-nominated Nominated for discussion during a libs team meeting. label Sep 1, 2026

rustbot assigned joboet Sep 1, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 1, 2026

RalfJung mentioned this pull request Sep 1, 2026

fn_addr_eq docs suggest incorrect reasoning #160202

Closed

RalfJung added the I-lang-nominated Nominated for discussion during a lang team meeting. label Sep 1, 2026

asquared31415 reviewed Sep 1, 2026

View reviewed changes

Comment thread library/core/src/ptr/mod.rs Outdated

traviscross added the P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang label Sep 1, 2026

the8472 reviewed Sep 1, 2026

View reviewed changes

RalfJung force-pushed the fn_addr_eq branch 4 times, most recently from 550db73 to a54d730 Compare September 1, 2026 18:26

traviscross added the T-lang-docs Relevant to the lang-docs team. label Sep 1, 2026

Urgau reviewed Sep 1, 2026

View reviewed changes

Comment thread library/core/src/ptr/mod.rs Outdated

RalfJung force-pushed the fn_addr_eq branch from a54d730 to 94e8df1 Compare September 1, 2026 19:52

clarfonthey reviewed Sep 2, 2026

View reviewed changes

RalfJung force-pushed the fn_addr_eq branch from 94e8df1 to f5b6700 Compare September 2, 2026 06:11

theemathas reviewed Sep 2, 2026

View reviewed changes

RalfJung force-pushed the fn_addr_eq branch from f5b6700 to a23b849 Compare September 2, 2026 06:32

programmerjake reviewed Sep 2, 2026

View reviewed changes

Comment thread library/core/src/ptr/mod.rs Outdated

RalfJung force-pushed the fn_addr_eq branch 2 times, most recently from c207ce5 to 72b5658 Compare September 2, 2026 13:43

fn_addr_eq: we actually can guarantee basically nothing

b623ba1

RalfJung force-pushed the fn_addr_eq branch from 72b5658 to b623ba1 Compare September 2, 2026 13:47

traviscross added the T-lang Relevant to the language team label Sep 2, 2026

This comment was marked as outdated.

Sign in to view

traviscross removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Sep 30, 2026

rustbot mentioned this pull request Oct 10, 2026

Tracking issue for release notes of #162140: fn_addr_eq: we actually can guarantee basically nothing #164102

Open

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2026

JonathanBrouwer mentioned this pull request Oct 12, 2026

Rollup of 9 pull requests #164211

Closed

This was referenced Oct 12, 2026

Rollup of 9 pull requests #164212

Closed

Rollup of 8 pull requests #164216

Merged

rust-bors Bot merged commit bd86817 into rust-lang:main Oct 12, 2026
13 checks passed

rustbot added this to the 1.101.0 milestone Oct 12, 2026

RalfJung deleted the fn_addr_eq branch October 15, 2026 08:58

theemathas mentioned this pull request Oct 23, 2026

{Rc,Arc}::eq has suspicious specialization for DSTs due to vtable merging and opsem mess. #164817

Open

Conversation

RalfJung commented Sep 1, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 1, 2026

Uh oh!

Uh oh!

the8472 Sep 1, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the8472 Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asquared31415 Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clarfonthey Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theemathas Sep 2, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottmcm commented Sep 2, 2026

Uh oh!

traviscross commented Sep 2, 2026

Uh oh!

This comment was marked as outdated.

ChayimFriedman2 commented Oct 8, 2026

Uh oh!

asquared31415 commented Oct 8, 2026

Uh oh!

RalfJung commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clarfonthey commented Oct 8, 2026

Uh oh!

ChayimFriedman2 commented Oct 8, 2026

RalfJung commented Sep 1, 2026 •

edited by rustbot

Loading

the8472 Sep 1, 2026 •

edited by rustbot

Loading

the8472 Sep 1, 2026 •

edited

Loading

asquared31415 Sep 1, 2026 •

edited

Loading

RalfJung Sep 1, 2026 •

edited

Loading

clarfonthey Sep 2, 2026 •

edited

Loading

theemathas Sep 2, 2026 •

edited by rustbot

Loading

RalfJung Sep 2, 2026 •

edited

Loading

RalfJung commented Oct 8, 2026 •

edited

Loading