Skip to content

const-eval: full support for pointer fragments #144081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 17, 2025

This fixes rust-lang/const-eval#72 and makes swap_nonoverlapping fully work in const-eval by enhancing per-byte provenance tracking with tracking of which of the bytes of the pointer this one is. Later, if we see all the same bytes in the exact same order, we can treat it like a whole pointer again without ever risking a leak of the data bytes (that encode the offset into the allocation). This lifts the limitation that was discussed quite a bit in #137280.

For a concrete piece of code that used to fail and now works properly consider this example doing a byte-for-byte memcpy in const without using intrinsics:

use std::{mem::{self, MaybeUninit}, ptr};

type Byte = MaybeUninit<u8>;

const unsafe fn memcpy(dst: *mut Byte, src: *const Byte, n: usize) {
    let mut i = 0;
    while i < n {
        *dst.add(i) = *src.add(i);
        i += 1;
    }
}

const _MEMCPY: () = unsafe {
    let ptr = &42;
    let mut ptr2 = ptr::null::<i32>();
    // Copy from ptr to ptr2.
    memcpy(&mut ptr2 as *mut _ as *mut _, &ptr as *const _ as *const _, mem::size_of::<&i32>());
    assert!(*ptr2 == 42);
};

What makes this code tricky is that pointers are "opaque blobs" in const-eval, we cannot just let people look at the individual bytes since we don't know what those bytes look like -- that depends on the absolute address the pointed-to object will be placed at. The code above "breaks apart" a pointer into individual bytes, and then puts them back together in the same order elsewhere. This PR implements the logic to properly track how those individual bytes relate to the original pointer, and to recognize when they are in the right order again.

We still reject constants where the final value contains a not-fully-put-together pointer: I have no idea how one could construct an LLVM global where one byte is defined as "the 3rd byte of a pointer to that other global over there" -- and even if LLVM supports this somehow, we can leave implementing that to a future PR. It seems unlikely to me anyone would even want this, but who knows.^^

This also changes the behavior of Miri, by tracking the order of bytes with provenance and only considering a pointer to have valid provenance if all bytes are in the original order again. This is related to rust-lang/unsafe-code-guidelines#558. It means one cannot implement XOR linked lists with strict provenance any more, which is however only of theoretical interest. Practically I am curious if anyone will show up with any code that Miri now complains about - that would be interesting data. Cc @rust-lang/opsem

@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2025

r? @davidtwco

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 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 17, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2025

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung RalfJung added T-lang Relevant to the language team needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Jul 17, 2025
@RalfJung
Copy link
Member Author

This will need t-lang FCP since it is an insta-stable extension of what const can do, but I first want to make CI pass.

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned davidtwco Jul 17, 2025
@RalfJung RalfJung added T-opsem Relevant to the opsem team and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 17, 2025
@RalfJung
Copy link
Member Author

RalfJung commented Jul 17, 2025

Let's see if there are any perf issues here.
@rust-timer queue
@bors2 try

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Jul 17, 2025
const-eval: full support for pointer fragments

This fixes rust-lang/const-eval#72 and makes `swap_nonoverlapping` fully work in const-eval by enhancing per-byte provenance tracking with tracking of *which* of the bytes of the pointer this one is. Later, if we see all the same bytes in the exact same order, we can treat it like a whole pointer again without ever risking a leak of the data bytes (that encode the offset into the allocation). This lifts the limitation that was discussed quite a bit in #137280.

However, we still reject constants where the final value contains such a partial pointer: I have no idea how one could construct an LLVM global where one byte is defined as "the 3rd byte of a pointer to that other global over there" -- and even if LLVM supports this somehow, we can leave implementing that to a future PR. This one here is useful by allowing such partial pointers as intermediate values; in particular, this means one can do a byte-for-byte copy of a pointer within const-eval and that will work properly.

This also changes the behavior of Miri, by tracking the order of bytes with provenance and only considering a pointer to have valid provenance if all bytes are in the original order again. This is related to rust-lang/unsafe-code-guidelines#558. It means one cannot implement XOR linked lists with strict provenance any more, which is however only of theoretical interest. Practically I am curious if anyone will show up with any code that Miri now complains about - that would be interesting data.

TODO: update Miri test suite.
@rust-bors
Copy link

rust-bors bot commented Jul 17, 2025

⌛ Trying commit e97b4fe with merge 09988fb

To cancel the try build, run the command @bors2 try cancel.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 17, 2025
@rust-timer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the const-ptr-fragments branch from e97b4fe to 224d236 Compare July 17, 2025 19:50
@RalfJung RalfJung added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jul 17, 2025
@RalfJung RalfJung force-pushed the const-ptr-fragments branch 2 times, most recently from 429218f to 4ff7ee7 Compare July 17, 2025 20:09
@rust-bors
Copy link

rust-bors bot commented Jul 17, 2025

☀️ Try build successful (CI)
Build commit: 09988fb (09988fb389684ce1e69a7ca9c7238fbe228aa65e, parent: bf5e6cc7a7a7eb03e3ed9b875d76530eddd47d5f)

@rust-timer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (09988fb): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.2%, 0.3%] 9
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -3.7%, secondary -3.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.7% [-3.7%, -3.7%] 1
Improvements ✅
(secondary)
-3.4% [-4.0%, -2.5%] 4
All ❌✅ (primary) -3.7% [-3.7%, -3.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 464.659s -> 464.054s (-0.13%)
Artifact size: 374.78 MiB -> 374.88 MiB (0.02%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 17, 2025
@RalfJung
Copy link
Member Author

Odd that it would affect coercions but not ctfe-stress... but this is on the edge if being noise anyway.

@RalfJung RalfJung force-pushed the const-ptr-fragments branch from 4ff7ee7 to 1c307a9 Compare July 18, 2025 06:12
@lcnr
Copy link
Contributor

lcnr commented Jul 18, 2025

Odd that it would affect coercions but not ctfe-stress... but this is on the edge if being noise anyway.

the coercions test isn't really a stress test for coercions, it's a stress test for computing the least upper bound of 60000 &'static str 😁 https://github.com/rust-lang/rustc-perf/blob/2120e3b7b8996e96858b88edefea371679a3d415/collector/compile-benchmarks/coercions/src/main.rs#L39

So this test doesn't do much during CTFE, but pretty much every single eval step moves a pointer

@RalfJung
Copy link
Member Author

Ah lol.^^ Yeah pointer ops become a tiny bit more expensive since we can't just skip checking the per-byte provenance any more. But if it takes a benchmark like that to result in a barely measurable slowdown, I think we're good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-nominated Nominated for discussion during a lang team meeting. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. perf-regression Performance regression. 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. T-lang Relevant to the language team T-opsem Relevant to the opsem team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full support for pointer fragments
7 participants