Skip to content

Guarantee behavior of transmuting Option::<T>::None subject to NPO #137323

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

Merged
merged 1 commit into from
May 24, 2025

Conversation

joshlf
Copy link
Contributor

@joshlf joshlf commented Feb 20, 2025

In #115333, we added a guarantee that transmuting from [0u8; N] to Option<P> is sound where P is a pointer type subject to the null pointer optimization (NPO). It would be useful to be able to guarantee the inverse - that a None::<P> value can be transmutes to an array and that will yield [0u8; N].

Closes #117591

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2025

r? @tgross35

rustbot has assigned @tgross35.
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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 20, 2025
@jieyouxu
Copy link
Member

r? libs-api (since this is making a behavior guaranteed)

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 20, 2025
@rustbot rustbot assigned dtolnay and unassigned tgross35 Feb 20, 2025
@RalfJung
Copy link
Member

Cc @rust-lang/opsem @rust-lang/lang

This will need a t-lang FCP. @joshlf would be good to write a summary for t-lang, knowing that they will lack all the context we have here. :)

@RalfJung RalfJung added T-lang Relevant to the language team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. labels Feb 20, 2025
@RalfJung
Copy link
Member

Looking at the PR itself, the change LGTM.

@dtolnay dtolnay assigned RalfJung and unassigned dtolnay Feb 20, 2025
@dtolnay dtolnay removed T-libs-api Relevant to the library API 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 Feb 20, 2025
@RalfJung RalfJung added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. I-lang-nominated Nominated for discussion during a lang team meeting. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2025
@joshlf
Copy link
Contributor Author

joshlf commented Mar 5, 2025

This will need a t-lang FCP. @joshlf would be good to write a summary for t-lang, knowing that they will lack all the context we have here. :)

Currently, zerocopy has the ability to validate at runtime whether a [u8; N] contains a valid Option<&T>. In particular, the NPO guarantees that, if the [u8; N] contains all 0u8 bytes, then it constitutes a valid Option<&T>. We do the same for other types subject to NPO: see e.g. this set of impls.

This works via our TryFromBytes trait. Under the hood, #[derive(TryFromBytes)] synthesizes a predicate over &[u8] - in particular, the TryFromBytes::is_bit_valid method takes a &[u8] (it's actually a different type, but it has the same bit validity and alignment as &[u8]) and determines whether it contains a valid Self.

Eventually, we'd like to not only support going from &[u8] to &T, but from &T to &U for a broader range of pairs of types. We can already support &T -> &U if we can soundly convert &T -> &[u8], but some cases don't satisfy that requirement. For example, consider:

#[repr(C)]
struct T {
    a: u8,
    b: u16,
}

#[repr(C)]
struct U {
    a: bool,
    b: u16,
}

Since T has inter-field padding, it's unsound to convert a &T to a &[u8]. We'd still like to be able to support the &T -> &U case, though. This requires changing TryFromBytes::is_bit_valid to no longer take a &[u8] argument, and to instead take something that we call AsInitialized. An AsInitialized<T> has the invariant that it has initialized bytes wherever T has initialized bytes, but it may be uninitializated in other byte positions. This will permit us to go from &T to &AsInitialized<U>, and have TryFromBytes::is_bit_valid take this &AsInitialized<U> as an argument.

That gets us to this PR: In order to make this change, for each type that currently implements TryFromBytes, we need a guarantee about which bytes are always initialized. Currently, if P is a type subject to NPO, we have no guarantee that an instance of Option<P> has all its bytes initialized. While getting this guarantee in full is blocked on ptr-to-int semantics, this PR is intended as a first step that will at least handle the None case.

@traviscross traviscross changed the title Guarantee behavior of transmuting Option::<T>::None subject to NPO Guarantee behavior of transmuting Option::<T>::None subject to NPO May 7, 2025
@joshtriplett
Copy link
Member

This seems consistent with how we already support using Option<&T> rather than *const T in FFI calls, in both directions. I'm surprised we don't already guarantee this; people's code would be broken if we did otherwise.

@joshtriplett
Copy link
Member

Per the above, we already have to guarantee this and there'd be widespread breakage if we ever failed to uphold it. So, let's write it down.

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented May 7, 2025

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 7, 2025
@traviscross
Copy link
Contributor

@rfcbot reviewed

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels May 7, 2025
@scottmcm
Copy link
Member

scottmcm commented May 7, 2025

Under the restriction (that's in the docs) of the very specific list of NPO types, 100% agreed.

@rfcbot reviewed

(Just wanted to double-check that we weren't accidentally guaranteeing anything for Option<char> or Option<CustomEnumOneOrTwo> or similar.)

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 7, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented May 7, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@traviscross
Copy link
Contributor

We talked about this in the lang call today, were happy to see it go forward, and it's now in FCP.

Let's cc @rust-lang/spec, to think about the interplay between the Reference and the library documentation when making language guarantees like this.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 17, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented May 17, 2025

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.

This will be merged soon.

@RalfJung
Copy link
Member

Ah, I missed the fact that I'm the reviewer here. ;)
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 24, 2025

📌 Commit 0fdeab9 has been approved by RalfJung

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels May 24, 2025
bors added a commit that referenced this pull request May 24, 2025
Rollup of 6 pull requests

Successful merges:

 - #137323 (Guarantee behavior of transmuting `Option::<T>::None` subject to NPO)
 - #139254 (std: sys: net: uefi: Implement TCP4 connect)
 - #141432 (refactor `CanonicalVarKind`)
 - #141480 (document some -Z flags as living in the rustc-dev-guide)
 - #141486 (rustdoc book: add argument explanation for `html_playground_url`)
 - #141496 (Enable `[issue-links]` and `[no-mentions]` in triagebot)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 334d7bd into rust-lang:master May 24, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 24, 2025
rust-timer added a commit that referenced this pull request May 24, 2025
Rollup merge of #137323 - joshlf:transmute-npo, r=RalfJung

Guarantee behavior of transmuting `Option::<T>::None` subject to NPO

In #115333, we added a guarantee that transmuting from `[0u8; N]` to `Option<P>` is sound where `P` is a pointer type subject to the null pointer optimization (NPO). It would be useful to be able to guarantee the inverse - that a `None::<P>` value can be transmutes to an array and that will yield `[0u8; N]`.

Closes #117591
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request May 26, 2025
Guarantee behavior of transmuting `Option::<T>::None` subject to NPO

In rust-lang#115333, we added a guarantee that transmuting from `[0u8; N]` to `Option<P>` is sound where `P` is a pointer type subject to the null pointer optimization (NPO). It would be useful to be able to guarantee the inverse - that a `None::<P>` value can be transmutes to an array and that will yield `[0u8; N]`.

Closes rust-lang#117591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Guarantee that it is sound to observe the bytes of None::<P> where P is a pointer type subject to NPO