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

add llvm writable attribute conditionally

RalfJung · 2026-04-16T12:11:52Z

This looks great, thanks! Please squash the commits.

Let's hope the larger ArgAttribute does not cause a perf issue -- I kicked of a benchmark run to check this.

quiode · 2026-04-16T12:29:35Z

@bors squash msg="add llvm writable attribute conditionally"

rust-bors · 2026-04-16T12:29:55Z

🔨 11 commits were squashed into da2bbfb.

RalfJung · 2026-04-16T12:44:47Z

Nice, I've not seen this in action before. :)

rust-bors · 2026-04-16T14:21:43Z

☀️ Try build successful (CI)
Build commit: 84b05e7 (84b05e789a7194a627e1ca3cda715fe05e5ca3f6, parent: e8e4541ff19649d95afab52fdde2c2eaa6829965)

rust-timer · 2026-04-16T15:02:03Z

Finished benchmarking commit (84b05e7): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

@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.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 3.1%, secondary 1.7%)

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

	mean	range	count
Regressions ❌ (primary)	3.1%	[2.6%, 3.7%]	2
Regressions ❌ (secondary)	4.5%	[2.8%, 6.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.0%	[-4.0%, -4.0%]	1
All ❌✅ (primary)	3.1%	[2.6%, 3.7%]	2

Cycles

Results (primary 2.3%, secondary 1.4%)

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

	mean	range	count
Regressions ❌ (primary)	2.3%	[2.2%, 2.4%]	2
Regressions ❌ (secondary)	2.7%	[1.8%, 4.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	2.3%	[2.2%, 2.4%]	2

Binary size

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

Bootstrap: 490.756s -> 493.416s (0.54%)
Artifact size: 394.16 MiB -> 394.14 MiB (-0.00%)

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%)

rustbot assigned mati865 Apr 12, 2026

rustbot assigned RalfJung and unassigned mati865 Apr 12, 2026

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