add llvm writable attribute conditionally by quiode · Pull Request #155207 · rust-lang/rust

quiode · 2026-04-12T18:54:57Z

This PR tries to address rust-lang/unsafe-code-guidelines#584 (comment). It is part of a bachelor thesis supervised by @JoJoDeveloping and @RalfJung, for more information, see: Project_Description.pdf.
If the new -Zllvm-writable flag is set, the llvm writable attribute is inserted for all mutable borrows. This can be conditionally turned off on a per-function basis using the #[rustc_no_writable] attribute. The new Undefined Behaviour introduced by this can detected by Miri, which is implemented here: rust-lang/miri#4947.

Two library functions already received the #[rustc_no_writable] attribute, as they are known to cause problems under the Tree Borrows aliasing model with implicit writes enabled.

rustbot · 2026-04-12T18:55:01Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-04-12T18:55:03Z

r? @mati865

rustbot has assigned @mati865.
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 69 candidates
Random selection from 12 candidates

quiode · 2026-04-12T18:55:08Z

r? @RalfJung

rustbot · 2026-04-12T18:55:12Z

RalfJung is not on the review rotation at the moment.
They may take a while to respond.

RalfJung

This generally looks good, thanks! I just hope this larger ArgAttribute won't cause us problems. Cc @nikic for the LLVM parts, in case you want to take a 2nd look.

I am not sure if there's anything special to look out for in the new attribute infrastructure; @jdonszelmann would be great if you could take a brief look at that part.

@rustbot author

View changes since this review

RalfJung · 2026-04-14T13:52:14Z

 // The subset of llvm::Attribute needed for arguments, packed into a bitfield.
 #[derive(Clone, Copy, Default, Hash, PartialEq, Eq, HashStable_Generic)]
- pub struct ArgAttribute(u8);
+ pub struct ArgAttribute(u16);


Ah, this is unfortunate. We should benchmark this to ensure the larger bitfield isn't a problem.

rustbot · 2026-04-14T14:03:04Z

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

JonathanBrouwer

r=me on the attribute changes, didn't check the other part of this PR

View changes since this review

JonathanBrouwer · 2026-04-14T19:47:48Z

@bors try @rust-timer queue

add llvm writable attribute conditionally

rust-bors · 2026-04-14T20:02:47Z

💔 Test for 37fa495 failed: CI. Failed job:

try - dist-x86_64-linux-quick (web logs, enhanced plaintext logs)

JonathanBrouwer · 2026-04-14T20:05:34Z

^ The PR needs to be rebased

RalfJung · 2026-04-16T15:20:10Z

That looks good! This is just a secondary benchmark that has been rather noisy recently already, nothing to worry about IMO.

@bors r+

rust-bors · 2026-04-16T15:20:14Z

📌 Commit da2bbfb has been approved by RalfJung

It is now in the queue for this repository.

rust-bors · 2026-04-17T07:35:05Z

☀️ Test successful - CI
Approved by: RalfJung
Duration: 3h 21m 21s
Pushing 1b8f2e4 to main...

github-actions · 2026-04-17T07:38:14Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 0204aca (parent) -> 1b8f2e4 (this PR)

Test differences

Show 930 test diffs

Stage 1

[codegen] tests/codegen-llvm/llvm-writable.rs: [missing] -> pass (J0)

Stage 2

[codegen] tests/codegen-llvm/llvm-writable.rs: [missing] -> pass (J1)

Additionally, 928 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1b8f2e46e14b08208a53585570edd9206374aae8 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-gnu-gcc: 48m 9s -> 1h 12m (+50.7%)
x86_64-msvc-ext1: 1h 41m -> 2h 18m (+37.1%)
dist-i686-msvc: 1h 51m -> 2h 26m (+32.0%)
dist-x86_64-msvc-alt: 2h 12m -> 2h 43m (+24.0%)
i686-msvc-2: 2h 11m -> 1h 41m (-22.6%)
dist-powerpc-linux: 1h 15m -> 1h 31m (+22.1%)
test-various: 1h 45m -> 2h 7m (+20.9%)
x86_64-gnu-llvm-21-2: 1h 35m -> 1h 17m (-19.1%)
dist-x86_64-apple: 2h 7m -> 2h 31m (+18.9%)
optional-x86_64-gnu-parallel-frontend: 2h 17m -> 2h 41m (+17.8%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-04-17T08:17:31Z

Finished benchmarking commit (1b8f2e4): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.1%, secondary -1.3%)

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

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	2.9%	[2.9%, 2.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.5%	[-5.5%, -5.5%]	1
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	1

Cycles

Results (primary 8.7%, secondary 6.0%)

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

	mean	range	count
Regressions ❌ (primary)	8.7%	[3.0%, 14.2%]	11
Regressions ❌ (secondary)	6.0%	[4.5%, 7.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	8.7%	[3.0%, 14.2%]	11

Binary size

Results (primary 0.1%, secondary 0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.1%]	15
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	18
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.1%]	15

Bootstrap: 490.004s -> 504.175s (2.89%)
Artifact size: 394.09 MiB -> 394.15 MiB (0.02%)

@JoJoDeveloping

This PR tries to address rust-lang/unsafe-code-guidelines#584 (comment). It is part of a bachelor thesis supervised by @JoJoDeveloping and @RalfJung, for more information, see: https://github.com/user-attachments/files/26537277/Project_Description.pdf. This implements the checking for implicit writes for Tree Borrows. It is disabled by default but can be enabled using the `-Zmiri-tree-borrows-implicit-writes` flag. When it is enabled, Miri inserts a write for all mutable borrows on function entry. This enables the optimization implemented here: rust-lang/rust#155207

@JoJoDeveloping

This PR tries to address rust-lang/unsafe-code-guidelines#584 (comment). It is part of a bachelor thesis supervised by @JoJoDeveloping and @RalfJung, for more information, see: https://github.com/user-attachments/files/26537277/Project_Description.pdf. This implements the checking for implicit writes for Tree Borrows. It is disabled by default but can be enabled using the `-Zmiri-tree-borrows-implicit-writes` flag. When it is enabled, Miri inserts a write for all mutable borrows on function entry. This enables the optimization implemented here: rust-lang#155207

…fns, r=RalfJung add #[rustc_no_writable] to slice::get_unchecked_mut This PR adds the `#[rustc_no_writable]` attribute introduced in rust-lang#155207 to the `slice::get_unchecked_mut` function. Two library functions already received this attribute, as they were known to cause problems with the llvm writable attribute and tree borrows. Since that PR, I ran Miri on the 30'000 most downloaded crates to see what kind of code is now UB under Tree Borrows + implicit writes, using the detection implemented in rust-lang/miri#4947. Adding this attribute to ignore checking for this function reduced the new UB introduced by more than 75%, meaning that instead of 19000 tests in 1700 crates having a difference, now only 3500 in 350 crates show a difference (measurement still running).

…lfJung add #[rustc_no_writable] to slice::get_unchecked_mut This PR adds the `#[rustc_no_writable]` attribute introduced in rust-lang/rust#155207 to the `slice::get_unchecked_mut` function. Two library functions already received this attribute, as they were known to cause problems with the llvm writable attribute and tree borrows. Since that PR, I ran Miri on the 30'000 most downloaded crates to see what kind of code is now UB under Tree Borrows + implicit writes, using the detection implemented in rust-lang#4947. Adding this attribute to ignore checking for this function reduced the new UB introduced by more than 75%, meaning that instead of 19000 tests in 1700 crates having a difference, now only 3500 in 350 crates show a difference (measurement still running).

@RalfJung

…, r=RalfJung add rustc_no_writable to mem::forget and structs it uses This builds upon rust-lang#155207 and is similar to rust-lang#157202. It adds the `#[rustc_no_writable]` attribute to `mem::forget` and the `MaybeDangling` and `ManuallyDrop`. This makes Miri with Tree Borrows and implicit writes no longer report UB for a test in `derive_more`. As the pattern itself is quite unclean, and I have not seen `mem::forget` to cause trouble before with implicit writes, I'm not sure how much sense it makes to add it to `mem::forget`. The test works unter Tree Borrows, but fails already for Stacked Borrows. Thus I would be happy for some guidance if the attribute makes sense here @RalfJung @JoJoDeveloping.

@RalfJung

Rollup merge of #159181 - quiode:mem--forget-implicit-writes, r=RalfJung add rustc_no_writable to mem::forget and structs it uses This builds upon #155207 and is similar to #157202. It adds the `#[rustc_no_writable]` attribute to `mem::forget` and the `MaybeDangling` and `ManuallyDrop`. This makes Miri with Tree Borrows and implicit writes no longer report UB for a test in `derive_more`. As the pattern itself is quite unclean, and I have not seen `mem::forget` to cause trouble before with implicit writes, I'm not sure how much sense it makes to add it to `mem::forget`. The test works unter Tree Borrows, but fails already for Stacked Borrows. Thus I would be happy for some guidance if the attribute makes sense here @RalfJung @JoJoDeveloping.

@RalfJung

add rustc_no_writable to mem::forget and structs it uses This builds upon rust-lang/rust#155207 and is similar to rust-lang/rust#157202. It adds the `#[rustc_no_writable]` attribute to `mem::forget` and the `MaybeDangling` and `ManuallyDrop`. This makes Miri with Tree Borrows and implicit writes no longer report UB for a test in `derive_more`. As the pattern itself is quite unclean, and I have not seen `mem::forget` to cause trouble before with implicit writes, I'm not sure how much sense it makes to add it to `mem::forget`. The test works unter Tree Borrows, but fails already for Stacked Borrows. Thus I would be happy for some guidance if the attribute makes sense here @RalfJung @JoJoDeveloping.

rustbot assigned mati865 Apr 12, 2026

rustbot assigned RalfJung and unassigned mati865 Apr 12, 2026

quiode force-pushed the writable branch from 245ff9c to 0c53594Compare April 12, 2026 19:29

This comment has been minimized.
Sign in to view

RalfJung reviewed Apr 14, 2026
View reviewed changes

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 Apr 14, 2026

JonathanBrouwer reviewed Apr 14, 2026
View reviewed changes

Comment threadcompiler/rustc_attr_parsing/src/attributes/rustc_internal.rs Outdated

JonathanBrouwer approved these changes Apr 14, 2026
View reviewed changes

This comment has been minimized.
Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 14, 2026

rust-borsBot pushed a commit that referenced this pull request Apr 14, 2026
Auto merge of#155207- quiode:writable, r=<try>
37fa495
add llvm writable attribute conditionally

This comment has been minimized.
Sign in to view

rust-borsBot 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 Apr 16, 2026

This comment has been minimized.
Sign in to view

rust-borsBot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 17, 2026

rust-borsBot merged commit 1b8f2e4 into rust-lang:mainApr 17, 2026
12 checks passed

rustbot added this to the 1.97.0 milestone Apr 17, 2026

quiode deleted the writable branch April 29, 2026 18:13

quiode mentioned this pull request May 31, 2026
add #[rustc_no_writable] to slice::get_unchecked_mut #157202
Merged

quiode mentioned this pull request Jul 12, 2026
add rustc_no_writable to mem::forget and structs it uses #159181
Merged

Uh oh!

Uh oh!

Conversation

quiode commented Apr 12, 2026• edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

quiode commented Apr 12, 2026

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

This comment has been minimized.

RalfJung left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RalfJungApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Apr 14, 2026

Uh oh!

Uh oh!

JonathanBrouwer left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer commented Apr 14, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-borsBot commented Apr 14, 2026

Uh oh!

JonathanBrouwer commented Apr 14, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

RalfJung commented Apr 16, 2026

Uh oh!

rust-borsBot commented Apr 16, 2026

Uh oh!

This comment has been minimized.

rust-borsBot commented Apr 17, 2026

Uh oh!

Uh oh!

github-actionsBot commented Apr 17, 2026

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Apr 17, 2026

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

quiode commented Apr 12, 2026•
edited by rustbot
Loading

RalfJung left a comment •
edited by rustbot
Loading

JonathanBrouwer left a comment •
edited by rustbot
Loading