Skip to content

Don't allow dereferencing uninitialized references inside PlaceMention - #164812

Open
Jules-Bertholet wants to merge 3 commits into
rust-lang:mainfrom
Jules-Bertholet:placemention-deref
Open

Don't allow dereferencing uninitialized references inside PlaceMention#164812
Jules-Bertholet wants to merge 3 commits into
rust-lang:mainfrom
Jules-Bertholet:placemention-deref

Conversation

@Jules-Bertholet

@Jules-Bertholet Jules-Bertholet commented Oct 23, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

View all comments

Fixes #164705, fixes #164738

Needs a crater run.

@rustbot label A-borrow-checker

@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. labels Oct 23, 2026
@rustbot

rustbot commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

r? @folkertdev

rustbot has assigned @folkertdev.
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: compiler
  • compiler expanded to 77 candidates
  • Random selection from 19 candidates

@rustbot rustbot added the A-borrow-checker Area: The borrow checker label Oct 23, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@folkertdev

folkertdev commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

r? @dianne maybe?

@rustbot rustbot assigned dianne and unassigned folkertdev Oct 23, 2026
maxdexh
maxdexh reviewed Oct 23, 2026
View reviewed changes
Comment thread tests/ui/borrowck/placemention-deref.rs Resolved
@theemathas theemathas added needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Oct 25, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
dianne
dianne reviewed Oct 26, 2026
View reviewed changes

@dianne dianne left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

one suggestion regarding iteration, but otherwise it looks good to me! I don't have that much borrowck experience though, and I expect T-types will want to be on the fcp, so r? types

View changes since this review

Comment thread compiler/rustc_borrowck/src/lib.rs Resolved
@rustbot rustbot added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Oct 26, 2026
@rustbot rustbot assigned spastorino and unassigned dianne Oct 26, 2026
@Jules-Bertholet @dianne
Avoid extra allocations
58d3745 
Co-authored-by: dianne <diannes.gm@gmail.com>
@theemathas

theemathas commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

I'm going to queue up a crater run, on the assumption that this is done being reviewed, and only needs an FCP and a crater run before merging.

@bors try

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Oct 26, 2026
@bors
Auto merge of #164812 - Jules-Bertholet:placemention-deref, r=<try>
f1c52bf 
Don't allow dereferencing dangling refs inside `PlaceMention`
@rust-bors

rust-bors Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f1c52bf (f1c52bf13c990fc425c542cc90a0396a87db24ba)
Base parent: 846e51f (846e51f66b1e7b49ba9e3988c58ccfe5817ce0a9)

@theemathas

theemathas commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

@craterbot check

@craterbot

craterbot commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

👌 Experiment pr-164812 created and queued.
🤖 Automatically detected try build f1c52bf
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 26, 2026
dianne
dianne reviewed Oct 26, 2026
View reviewed changes
Comment thread tests/ui/borrowck/placemention-deref.rs Resolved
@rustbot

rustbot commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

@Amanieu

Amanieu commented Oct 27, 2026

Copy link
Copy Markdown
Member

I would also like to see a crater run for a stricter version of this that also requires cases where there is no dereference to be initialized. This is necessary for the new MIR semantics for move elimination, which only allocates a local when it is initialized (it doesn't have an address before that point).

let a: u32;
_ = a;

In this example, a is not initialized and therefore doesn't have an address, but place evaluation must produce a pointer.

@Jules-Bertholet Jules-Bertholet mentioned this pull request Oct 27, 2026
Draft
@Jules-Bertholet

Jules-Bertholet commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@Amanieu here you go: #165046

@theemathas Can you cancel this PR's crater run in favor of that one? We can then crater this PR only on the regressions from that run

@theemathas

theemathas commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

@craterbot abort

@craterbot

craterbot commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

🗑️ Experiment pr-164812 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Oct 27, 2026
@Jules-Bertholet

Jules-Bertholet commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

@theemathas could you please crater this with the regressions from #165046 (comment)?

@theemathas

theemathas commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-165046/retry-regressed-list.txt

@craterbot

craterbot commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

👌 Experiment pr-164812 created and queued.
🤖 Automatically detected try build f1c52bf
⚠️ Try build based on commit 58d3745, but latest commit is 2766312. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2026
@theemathas

theemathas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Since the batch being ran on crater is small:

@craterbot p=1

@craterbot

craterbot commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator

📝 Configuration of the pr-164812 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

craterbot commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator

🚧 Experiment pr-164812 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

craterbot commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator

🎉 Experiment pr-164812 is completed!
📊 0 regressed and 0 fixed (5954 total)
📊 466 spurious results on the retry-regressed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-164812/retry-regressed-list.txt ↩

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 8, 2026
@Jules-Bertholet

Jules-Bertholet commented Nov 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I guess this is ready to go…

@rustbot label needs-fcp I-types-nominated

@rustbot rustbot added the I-types-nominated Nominated for discussion during a types team meeting. label Nov 8, 2026
@rustbot

rustbot commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator

🤖 A dedicated #t-types/nominated topic has been opened for humans to discuss this issue 🤖

@theemathas theemathas removed the needs-crater This change needs a crater run to check for possible breakage in the ecosystem. label Nov 8, 2026
@RalfJung

RalfJung commented Nov 8, 2026

Copy link
Copy Markdown
Member

This PR currently has basically no description, which is unsuitable for a nominated PR. Please explain what exactly is being rejected here, and what is still allowed compared to #165046 (comment).

@Jules-Bertholet Jules-Bertholet changed the title Don't allow dereferencing dangling refs inside PlaceMention Don't allow dereferencing dangling references inside PlaceMention Nov 8, 2026
@Jules-Bertholet Jules-Bertholet changed the title Don't allow dereferencing dangling references inside PlaceMention Don't allow dereferencing uninitialized references inside PlaceMention Nov 8, 2026
@Jules-Bertholet

Jules-Bertholet commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

This PR rejects dereferences of fully uninitialized or moved-out places, even after let _ =. For example, let r: &u32; let _ = *r; is rejected. (Today, it triggers UB.)

Dereferencing a partially moved Box is still OK.

#165046 (comment) added some restrictions that would be necessary for rust-lang/rfcs#3943. It's not relevant to the immediate soundness fix.

@RalfJung

RalfJung commented Nov 8, 2026 via email

Copy link
Copy Markdown
Member

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dianne dianne dianne left review comments
@maxdexh maxdexh maxdexh left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@spastorino spastorino

Labels

A-borrow-checker Area: The borrow checker I-types-nominated Nominated for discussion during a types team meeting. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. 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-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone