Implement some more checks in `ptr_guaranteed_cmp`. #144885

zachs18 · 2025-08-04T06:35:39Z

Pointers with different residues modulo their allocations' least common alignment are never equal.
Pointers to the same static allocation are equal if and only if they have the same offset.
Pointers to different non-zero-sized static allocations are unequal if both point within their allocation, and not on opposite ends.

Tracking issue for const_raw_ptr_comparison: #53020

This should not affect is_null, the only usage of this intrinsic on stable.

rustbot · 2025-08-04T06:35:44Z

rustbot has assigned @davidtwco.
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

rustbot · 2025-08-04T06:35:45Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

theemathas · 2025-08-04T10:17:28Z

Are two different immutable statics required to have distinct addresses? In other words, is the compiler allowed to optimize two immutable statics with identical contents to be stored at the same place?

RalfJung · 2025-08-04T14:19:36Z

I don't think it is, but this also restricts people doing their own custom linking shenanigans, so we should have some stable docs to point to before having logic of this sort in const-eval. That sounds like something t-lang should sign off on.

RalfJung · 2025-08-04T14:29:05Z

If you just want to fix #144584, please reduce the PR to only affect the case where both pointers are based on the same static.

zachs18 · 2025-08-04T16:11:52Z

Are two different immutable statics required to have distinct addresses? In other words, is the compiler allowed to optimize two immutable statics with identical contents to be stored at the same place?

Yes, they are required to have distinct addresses (if non-zero-sized). No, the compiler is not allowed to coalesce two immutable statics with the same contents.

we should have some stable docs to point to before having logic of this sort in const-eval.

This is guaranteed for non-zero-sized statics, in the Reference:

https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.storage-disjointness

If the static has a size of at least 1 byte, this allocated object is disjoint from all other such static objects as well as heap allocations and stack-allocated variables. However, the storage of immutable static items can overlap with objects that do not themselves have a unique address, such as promoteds and const items.

This (non-zero-sized statics not overlapping) has been FCP'd by T-lang here (though not anything about consteval specifically)

davidtwco · 2025-08-13T17:26:57Z

r? @RalfJung

zachs18 · 2025-08-17T20:37:19Z

If you just want to fix #144584, please reduce the PR to only affect the case where both pointers are based on the same static.

I would prefer to land all of these changes, but if "Pointers to the same static allocation are equal if and only if they have the same offset" is more straightforward to land than the other two, then I can split them into a followup PR.

RalfJung

That's an impressive test, thanks a lot for writing it!

RalfJung · 2025-08-18T14:33:12Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+                        // runtime inequality of pointers to different ones) (see e.g. #73722).
+                        Some(GlobalAlloc::Function { .. } | GlobalAlloc::VTable(..)) => 2,
+                        // FIXME: Can these be duplicated (or deduplicated)?
+                        Some(GlobalAlloc::Memory(..) | GlobalAlloc::TypeId { .. }) => 2,


Currently Memory definitely can be deduplicated. TypeId is its own thing that exists mostly to prevent const-eval from comparing it so we should definitely always return 2 there.

Okay, I'll split and update the comment for TypeId here.

I'll update the comment in the different-AllocId branch to state that Memory can be deduplicated (and Vtable and Function too).

Can Memory be duplicated? That is, should two pointers to the same Memory allocation (at the same offset) compare equal, or be uncomparable?

Can Memory be duplicated? That is, should two pointers to the same Memory allocation (at the same offset) compare equal, or be uncomparable?

Until we fix #128775, yes that can happen.

tests/ui/const-ptr/guaranteed_cmp.rs

compiler/rustc_const_eval/src/const_eval/machine.rs

zachs18 · 2025-08-19T05:02:46Z

Latest push addresses some parts of review comments, moves the test to tests/ui/consts (combining and overwriting the existing test for guaranteed_cmp there), and adds alignment checks when comparing pointers with integer-valued pointers.

RalfJung · 2025-08-19T06:00:53Z

and adds alignment checks when comparing pointers with integer-valued pointers.

I would have preferred if you didn't extend the scope of the PR after I already did most of my review. Now I'll have to re-review, so back on the queue it goes.

compiler/rustc_const_eval/src/const_eval/machine.rs

zachs18 · 2025-08-19T06:12:03Z

and adds alignment checks when comparing pointers with integer-valued pointers.

I would have preferred if you didn't extend the scope of the PR after I already did most of my review. Now I'll have to re-review, so back on the queue it goes.

This is a separate commit. If you prefer I can split this into a separate PR.

cjgillot · 2025-08-20T21:01:13Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+                if a_allocid == b_allocid {
+                    match self.tcx.try_get_global_alloc(a_allocid) {
+                        None => 2,
+                        // A static cannot be duplicated, so if two pointers are into the same
+                        // static, they are equal if and only if their offsets into the static
+                        // are equal
+                        Some(GlobalAlloc::Static(_)) => (a_offset == b_offset) as u8,
+                        // Functions and vtables can be duplicated (and deduplicated), so we
+                        // cannot be sure of runtime equality of pointers to the same one, (or the
+                        // runtime inequality of pointers to different ones) (see e.g. #73722).
+                        Some(GlobalAlloc::Function { .. } | GlobalAlloc::VTable(..)) => 2,
+                        // FIXME: Revisit this once https://github.com/rust-lang/rust/issues/128775
+                        // is fixed.
+                        Some(GlobalAlloc::Memory(..)) => 2,
+                        // `GlobalAlloc::TypeId` exists mostly to prevent consteval from comparing
+                        // `TypeId`s, always return 2
+                        Some(GlobalAlloc::TypeId { .. }) => 2,
+                    }
+                } else {
+                    if let (Some(GlobalAlloc::Static(a_did)), Some(GlobalAlloc::Static(b_did))) = (
+                        self.tcx.try_get_global_alloc(a_allocid),
+                        self.tcx.try_get_global_alloc(b_allocid),
+                    ) {
+                        debug_assert_ne!(
+                            a_did, b_did,
+                            "same static item DefId had two different AllocIds? {a_allocid:?} != {b_allocid:?}, {a_did:?} == {b_did:?}"
+                        );
+
+                        if a_info.size == Size::ZERO || b_info.size == Size::ZERO {
+                            // One or both allocations is zero-sized, so we can't know if the
+                            // pointers are (in)equal.
+                            // FIXME: Can zero-sized static be "within" non-zero-sized statics?
+                            // Conservatively we say yes, since that doesn't cause them to
+                            // "overlap" any bytes, but if not, then we could delete this branch;
+                            // the other branches would already handle ZST allocations correctly.
+                            2
+                        } else if a_offset > a_info.size || b_offset > b_info.size {
+                            // One or both pointers are out of bounds of their allocation,
+                            // so conservatively say we don't know.
+                            // FIXME: we could reason about how far out of bounds the pointers are,
+                            // e.g. two pointers cannot be equal if them being equal would require
+                            // their statics to overlap.
+                            2
+                        } else if (a_offset == Size::ZERO && b_offset == b_info.size)
+                            || (a_offset == a_info.size && b_offset == Size::ZERO)
+                        {
+                            // The pointers are on opposite ends of different allocations, we
+                            // cannot know if they are equal, since the allocations may end up
+                            // adjacent at runtime.
+                            2
+                        } else {
+                            // The pointers are within (or one past the end of) different
+                            // non-zero-sized static allocations, and they are not at oppotiste
+                            // ends, so we know they are not equal because non-zero-sized statics
+                            // cannot overlap or be deduplicated, as per
+                            // https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.intro
+                            // (non-deduplication), and
+                            // https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.storage-disjointness
+                            // (non-overlapping)
+                            0
+                        }
+                    } else {
+                        // Even if one of them is a static, as per https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.storage-disjointness
+                        // immutable statics can overlap with other kinds of allocations sometimes.
+                        // FIXME: We could be more decisive for (non-zero-sized) mutable statics,
+                        // which cannot overlap with other kinds of allocations.
+                        // `GlobalAlloc::{Memory, Function, Vtable}` can at least be deduplicated with
+                        // the same kind, so comparing two of the same kind of those should return 2.
+                        // `GlobalAlloc::TypeId` exists mostly to prevent consteval from comparing
+                        // `TypeId`s, so comparing two of those should always return 2.
+                        // FIXME: Can we determine any other cases?
+                        2
+                    }
+                }


In this whole discussion, we only return !=2 if both allocations are statics. Could this be simplified a lot by first checking that allocations are statics, and only then compare alloc ids?

Yes, that would work. I think I did it this way to have the comments be more precise to which cases they cover, but the comments could definitely be combined, especially for the cases that can both be duplicated and de-duplicated.

Latest push does this.

cjgillot · 2025-08-20T21:02:11Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+                        if a_info.size == Size::ZERO || b_info.size == Size::ZERO {
+                            // One or both allocations is zero-sized, so we can't know if the
+                            // pointers are (in)equal.
+                            // FIXME: Can zero-sized static be "within" non-zero-sized statics?
+                            // Conservatively we say yes, since that doesn't cause them to
+                            // "overlap" any bytes, but if not, then we could delete this branch;
+                            // the other branches would already handle ZST allocations correctly.
+                            2
+                        } else if a_offset > a_info.size || b_offset > b_info.size {
+                            // One or both pointers are out of bounds of their allocation,
+                            // so conservatively say we don't know.
+                            // FIXME: we could reason about how far out of bounds the pointers are,
+                            // e.g. two pointers cannot be equal if them being equal would require
+                            // their statics to overlap.
+                            2
+                        } else if (a_offset == Size::ZERO && b_offset == b_info.size)
+                            || (a_offset == a_info.size && b_offset == Size::ZERO)
+                        {
+                            // The pointers are on opposite ends of different allocations, we
+                            // cannot know if they are equal, since the allocations may end up
+                            // adjacent at runtime.
+                            2
+                        } else {
+                            // The pointers are within (or one past the end of) different
+                            // non-zero-sized static allocations, and they are not at oppotiste
+                            // ends, so we know they are not equal because non-zero-sized statics
+                            // cannot overlap or be deduplicated, as per
+                            // https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.intro
+                            // (non-deduplication), and
+                            // https://doc.rust-lang.org/nightly/reference/items/static-items.html#r-items.static.storage-disjointness
+                            // (non-overlapping)
+                            0
+                        }


Could we simplify this whole discussion with as "if both offsets are strictly in bounds, then return 0, else return 2"?

If by "strictly in bounds", you are excluding the one-past-the-end address, then yes that would work, but would return 2 in some cases where we can know the answer, e.g. comparing one-past-the-end pointers to two non-zero-sized statics.

Thinking some more about this though, I think the fact that "pointers to different statics cannot be equal if them being equal would require their statics to overlap" could be used to make this section simpler, while still returning 0 for two one-past-the-end pointers; e.g:

if a_info.size == Size::ZERO || b_info.size == Size::ZERO { 2 // as above, but remove the "we could delete this branch", since the below branches assume non-zero-sized allocations } else if a_offset > b_offset { // Implement this using a match on Ord::cmp instead of chained ifs // assuming the two pointers had equal addresses, this would be the differece in address of // the base of their allocations, with `a`'s allocation at lower address. let diff = a_offset - b_offset; if a_info.size > diff { // The pointers cannot be equal, because that would imply their non-zero-sized allocations overlap 0 } else { 2 } } else if a_offset < b_offset { // assuming the two pointers had equal addresses, this would be the difference in address of // the base of their allocations, with `b`'s allocation at lower address. let diff = b_offset - a_offset; if b_info.size > diff { // The pointers cannot be equal, because that would imply their non-zero-sized allocations overlap 0 } else { 2 } } else if a_offset == b_offset { // assuming the two pointers had equal addresses, their allocations would have the same base address. // The pointers cannot be equal, because that would imply their non-zero-sized allocations overlap 0 } else { unreachable!{} }

I think this handles every case currently handled, and more cases where a (or both) pointer is out-of-bounds of its allocation, but in such a way that they still cannot be equal.

Please try to do as simple as possible. As a rule of thumb, it's better to list cases where we ensure stuff and return 0 or 1, and have the fallback to "I don't know". A "both inbound then 1", else "some special case then 1" else 2 is the best way.

This is a lot harder to think about, and only makes a difference when comparing out-of-bounds pointers... I don't think it's worth it.

Latest push only returns unequal if both pointers are strictly in-bounds of their own allocations for simplicity.

I didn't actually mind the old logic you proposed first (with the "different ends of the allocation" check), not sure why you removed that...

I guess you got mixed signaling here, I don't agree with @cjgillot.

So let's just go with the simple thing now, this constant back-and-forth is a terrible waste of review time.

RalfJung · 2025-08-22T07:41:28Z

tests/ui/consts/ptr_comparisons.rs

 // aside from 0, these pointers might end up pretty much anywhere.
-check!(!, FOO as *const _, 1); // this one could be `ne` by taking into account alignment
-check!(!, FOO as *const _, 1024);
+do_test!(&A, align_of::<T>() as *const u8, None);
+do_test!(&A, 1 as *const u8, Some(false)); // this one takes into account alignment, so we know that


This grouping no longer makes sense... you first say "can end up anywhere" but then still have a Some test.

Please also ensure that (&raw const A).wrapping_byte_add(1) might be equal to 1. (I guess technically we could exclude that since statics cannot live at address 0.... but at some point we have to stop adding more cases to this PR.^^)

Please also ensure that (&raw const A).wrapping_byte_add(1) might be equal to 1. (I guess technically we could exclude that since statics cannot live at address 0.... but at some point we have to stop adding more cases to this PR.^^)

(The change added in 2a24220 makes this known to never occur)

RalfJung · 2025-08-22T07:46:41Z

compiler/rustc_const_eval/src/const_eval/machine.rs

+            // Other ways of comparing integers and pointers can never be known for sure,
+            // except for alignment, e.g. `1 as *const _` can never be equal to an even offset
+            // in an `align(2)` allocation.
+            (Scalar::Int(int), Scalar::Ptr(ptr, _)) | (Scalar::Ptr(ptr, _), Scalar::Int(int)) => {


Another possible implementation here would be to compute ptr.wrapping_sub(int) and then compare that against null. scalar_may_be_null already handles alignment so that should cover the same cases and more, I think?

Added in 2a24220

RalfJung · 2025-08-22T07:47:50Z

tests/ui/consts/ptr_comparisons.rs

+// and `fn` pointers cannot be null, and `is_null` is stable with strong guarantees, and
+// `is_null` is implemented using `guaranteed_cmp`.
+do_test!(&A, 0 as *const u8, Some(false));
+do_test!((&raw const A).cast::<u8>().wrapping_add(1), 0 as *const u8, Some(false));


Please also add a case that adds 11 or so to ensure we hit the alignment code path, not the inbounds code path.

Added in edbafa2 (as size_of::<T>() + 1)

// This pointer is out-of-bounds, but still cannot be equal to 0 because of alignment. do_test!((&raw const A).cast::<u8>().wrapping_add(size_of::<T>() + 1), 0 as *const u8, Some(false));

RalfJung

Okay, let's go with this logic for now. :)
Just some comment nits.

View changes since this review

compiler/rustc_const_eval/src/const_eval/machine.rs

RalfJung · 2025-08-23T16:45:49Z

This looks great, thanks! Please squash the commits, then we can land this. Then write @rustbot ready after you force-pushed the squashed PR.

@rustbot author

rustbot · 2025-08-23T16:45:53Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Pointers with different residues modulo their least common allocation alignment are never equal. Pointers to the same static allocation are equal if and only if they have the same offset. Strictly in-bounds (in-bounds and not one-past-the-end) pointers to different static allocations are always unequal. A pointer cannot be equal to an integer if `ptr-int` cannot be null. Also adds more tests for `ptr_guaranteed_cmp`. Co-authored-by: Ralf Jung <[email protected]>

zachs18 · 2025-08-23T17:15:05Z

@rustbot ready

RalfJung · 2025-08-23T20:13:01Z

@bors r+

bors · 2025-08-23T20:13:03Z

📌 Commit 10fde9e has been approved by RalfJung

It is now in the queue for this repository.

…=RalfJung Implement some more checks in `ptr_guaranteed_cmp`. * Pointers with different residues modulo their allocations' least common alignment are never equal. * Pointers to the same static allocation are equal if and only if they have the same offset. * Pointers to different non-zero-sized static allocations are unequal if both point within their allocation, and not on opposite ends. Tracking issue for `const_raw_ptr_comparison`: <rust-lang#53020> This should not affect `is_null`, the only usage of this intrinsic on stable. Closes rust-lang#144584

Rollup of 6 pull requests Successful merges: - #144531 (Add lint against integer to pointer transmutes) - #144885 (Implement some more checks in `ptr_guaranteed_cmp`. ) - #145307 (Fix `LazyLock` poison panic message) - #145554 (rustc-dev-guide subtree update) - #145798 (Use unnamed lifetime spans as primary spans for `MISMATCHED_LIFETIME_SYNTAXES`) - #145799 (std/src/lib.rs: mention "search button" instead of "search bar") r? `@ghost` `@rustbot` modify labels: rollup

jhpratt · 2025-08-24T03:58:41Z

@bors r-

#145804 (comment)

All function pointers are currently treated as unaligned anyway; any change implementing function pointer alignment during consteval should add tests that it works properly on arm::t32 functions.

zachs18 · 2025-08-24T05:03:26Z

Latest commit removes the failing #[instruction_set(arm::t32)] aligned fn pointer test (and updates the comment in the test). Since #144706 all fn pointers have no alignment in consteval anyway (regardless of instruction set or compilation target), so I didn't see much point trying to fix the test to accurately detect when #[instruction_set(arm::t32)] is supported when it isn't even testing anything different than the normal aligned fn pointer test at the moment.

@rustbot ready

rustbot assigned davidtwco Aug 4, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 4, 2025

rustbot assigned RalfJung and unassigned davidtwco Aug 13, 2025

RalfJung reviewed Aug 18, 2025

View reviewed changes

RalfJung reviewed Aug 19, 2025

View reviewed changes

compiler/rustc_const_eval/src/const_eval/machine.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/const_eval/machine.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

cjgillot reviewed Aug 20, 2025

View reviewed changes

RalfJung reviewed Aug 22, 2025

View reviewed changes

RalfJung reviewed Aug 23, 2025

View reviewed changes

compiler/rustc_const_eval/src/const_eval/machine.rs Outdated Show resolved Hide resolved

compiler/rustc_const_eval/src/const_eval/machine.rs Show resolved Hide resolved

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2025

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 23, 2025

zachs18 force-pushed the ptr_guaranteed_cmp_more branch from 5e706b9 to 10fde9e Compare August 23, 2025 17:12

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 23, 2025

bors 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 Aug 23, 2025

jhpratt mentioned this pull request Aug 23, 2025

Rollup of 6 pull requests #145802

Closed

Zalathar mentioned this pull request Aug 24, 2025

Rollup of 14 pull requests #145803

Closed

Zalathar mentioned this pull request Aug 24, 2025

Rollup of 6 pull requests #145804

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2025

Remove arm::t32-specific function pointer test.

25afbbc

All function pointers are currently treated as unaligned anyway; any change implementing function pointer alignment during consteval should add tests that it works properly on arm::t32 functions.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 24, 2025

Implement some more checks in ptr_guaranteed_cmp. #144885

Are you sure you want to change the base?

Implement some more checks in ptr_guaranteed_cmp. #144885

Conversation

zachs18 commented Aug 4, 2025

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

rustbot commented Aug 4, 2025

Uh oh!

theemathas commented Aug 4, 2025

Uh oh!

RalfJung commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Aug 4, 2025

Uh oh!

zachs18 commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidtwco commented Aug 13, 2025

Uh oh!

zachs18 commented Aug 17, 2025

Uh oh!

RalfJung left a comment

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!

Uh oh!

zachs18 commented Aug 19, 2025

Uh oh!

RalfJung commented Aug 19, 2025

Uh oh!

Uh oh!

Uh oh!

zachs18 commented Aug 19, 2025

Uh oh!

This comment has been minimized.

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!

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 Aug 23, 2025 • 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!

RalfJung Aug 22, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Implement some more checks in `ptr_guaranteed_cmp`. #144885

Implement some more checks in `ptr_guaranteed_cmp`. #144885

RalfJung commented Aug 4, 2025 •

edited

Loading

zachs18 commented Aug 4, 2025 •

edited

Loading

RalfJung Aug 23, 2025 •

edited

Loading

RalfJung Aug 22, 2025 •

edited

Loading

RalfJung left a comment •

edited by rustbot

Loading