Skip to content

refactor 'valid for read/write' definition: exclude null#152615

Open
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:null-not-valid-for-read-write
Open

refactor 'valid for read/write' definition: exclude null#152615
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:null-not-valid-for-read-write

Conversation

@RalfJung
Copy link
Member

This is an attempt to resolve #138351.

The underlying problem is that when we decided to allow reads/writes/copies of size 0 even for null pointers, we documented that by changing the definition of "valid for read/write" in the standard library to say that null pointers are valid for 0-sized reads/writes. Unfortunately, that definition is also used in other places that assume that a valid-for-read/write pointer can be converted into a reference, and of course that's UB if the pointer is null, even if the pointee is a ZST.

The proposal for fixing this is to make "valid for reads/writes" slightly weaker than it has to be, and require the pointer to be non-null, and then to add exceptions to the most basic functions (read/write/copy) to explicitly allow arbitrary pointers when the size is 0. This isn't pretty but it's the best solution that has been suggested so far I think.

Cc @rust-lang/opsem @rust-lang/libs-api

@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 14, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 14, 2026

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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: @scottmcm, libs
  • @scottmcm, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, ibraheemdev, joboet, scottmcm

@Amanieu Amanieu added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Feb 14, 2026
@Amanieu
Copy link
Member

Amanieu commented Feb 17, 2026

@rfcbot merge libs-api

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Feb 17, 2026

Team member @Amanieu 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!

See this document for info about what commands tagged team members can give me.

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Feb 17, 2026
@rust-rfcbot rust-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 Feb 17, 2026
@Amanieu
Copy link
Member

Amanieu commented Feb 17, 2026

This seemed fine to people in the libs-api meeting. Feel free to restart the FCP if you think that opsem also wants a say.

@RalfJung
Copy link
Member Author

I think the opsem rules are clear, it's "just" a question of how to best document them in the context of how they are exposed in the library.

@RalfJung RalfJung force-pushed the null-not-valid-for-read-write branch 2 times, most recently from 5bdc472 to 5600fd0 Compare February 17, 2026 21:09
@rust-rfcbot rust-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 Feb 17, 2026
@rust-rfcbot
Copy link
Collaborator

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

@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the null-not-valid-for-read-write branch from 5600fd0 to fef0baf Compare February 17, 2026 21:52
@Mark-Simulacrum
Copy link
Member

r=me with FCP done

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2026
@RalfJung RalfJung force-pushed the null-not-valid-for-read-write branch from fef0baf to 54e9a6a Compare February 25, 2026 10:44
@rustbot
Copy link
Collaborator

rustbot commented Feb 25, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@RalfJung
Copy link
Member Author

I have updated this to include #149169: ptr::replace now also documents that null is allowed if T is a ZST.

Copy link
Contributor

@CAD97 CAD97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting as a drive-by from another opsem member that this is of no impact to opsem, only how we surface it in documentation.

Furthermore, zero sized accesses are special cased at the opsem level to not care about pointer provenance at all, so the opsem fundamentally has a definition of "valid for access" which for non-zero-sized accesses separate from that for zero-sized ones. We previously attempted to document this as a single "valid for reads/writes" property in user docs, and this change essentially just admits that zero-sized access doesn't read/write and documents that preexisting special case on pointer accesses directly.

View changes since this review

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. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core::ptr::replace implementation is unsound since Rust 1.80

8 participants