Avoid hoisting uninitialized reads in EarlyOtherwiseBranch by darkraider01 · Pull Request #164864 · rust-lang/rust

darkraider01 · 2026-10-24T16:07:20Z

Fixes #164765.
EarlyOtherwiseBranch can hoist a read onto a path where its operand is uninitialized, causing a miscompilation.
When the parent’s otherwise branch is reachable, this restricts scalar hoisting to unchanged function arguments and their struct/tuple fields. It reuses VisitPlacesWith to reject writes, borrows, moves, and storage resets, without adding runtime branches.
The tradeoff is conservative: some safe computed locals and borrowed or modified arguments lose this optimization. Supporting those cases would require a broader initialization analysis. (any suggestions or pointers here?)

rustbot · 2026-10-24T16:07:24Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-10-24T16:07:27Z

r? @nnethercote

rustbot has assigned @nnethercote.
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 17 candidates

nnethercote · 2026-10-26T11:40:58Z

Let's pick a reviewer who knows more about MIR opts:

r? @dianqk

dianqk

IMO, I'm wondering whether we should ignore the pattern that the otherwise branch is reachable. I don't see how it would be useful.

View changes since this review

darkraider01 · 2026-10-28T13:37:03Z

IMO, I'm wondering whether we should ignore the pattern that the otherwise branch is reachable. I don't see how it would be useful.

View changes since this review

The reason for keeping reachable-otherwise cases was to preserve scalar match optimizations. In the earlier codegen checks, one three-way example grew from 20 to 55 bytes with the pass disabled, though that’s only a small example.

Skipping these cases would definitely make the fix simpler. Do i keep this PR to that restriction and handle the optimization seperately ?

dianqk · 2026-10-29T13:30:50Z

IMO, I'm wondering whether we should ignore the pattern that the otherwise branch is reachable. I don't see how it would be useful.
View changes since this review

The reason for keeping reachable-otherwise cases was to preserve scalar match optimizations. In the earlier codegen checks, one three-way example grew from 20 to 55 bytes with the pass disabled, though that’s only a small example.

Skipping these cases would definitely make the fix simpler. Do i keep this PR to that restriction and handle the optimization seperately ?

I don't see any missing cases in the PR, so we can handle the optimization there. I 'll take another look later.

dianqk · 2026-10-29T13:30:59Z

@bors try @rust-timer queue

…5, r=<try> Avoid hoisting uninitialized reads in EarlyOtherwiseBranch

rust-bors · 2026-10-29T14:30:00Z

☀️ Try build successful (CI)
Build commit: 22a54bb (22a54bb2c73f7cfa474ea45ac1926d62a947be13)
Base parent: ce37c4f (ce37c4ff837c5c5dc45cddf9e1e3704e42ca67ea)

rust-timer · 2026-10-29T16:06:24Z

Finished benchmarking commit (22a54bb): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Max RSS (memory usage)

Results (primary -0.9%, secondary -0.7%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-3.4%, -0.4%]	13
Improvements ✅ (secondary)	-0.7%	[-1.1%, -0.4%]	2
All ❌✅ (primary)	-0.9%	[-3.4%, -0.4%]	13

Cycles

Results (primary 0.5%, secondary 0.7%)

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

	mean	range	count
Regressions ❌ (primary)	1.0%	[0.4%, 2.8%]	13
Regressions ❌ (secondary)	1.9%	[0.5%, 2.5%]	4
Improvements ✅ (primary)	-0.8%	[-1.1%, -0.5%]	5
Improvements ✅ (secondary)	-1.9%	[-3.2%, -0.5%]	2
All ❌✅ (primary)	0.5%	[-1.1%, 2.8%]	18

Binary size

Results (primary 0.2%)

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

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.9%, 0.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[-0.0%, 0.9%]	4

Bootstrap: 490.291s -> 489.287s (-0.20%)
Artifact size: 406.63 MiB -> 406.62 MiB (-0.00%)

dianqk

LGTM. Small nits.

View changes since this review

rustbot · 2026-11-12T13:12:05Z

⚠️ Warning ⚠️

This PR is based on an upstream commit that is older than 28 days.

It's recommended to update your branch according to the rustc-dev-guide.

dianqk · 2026-11-12T15:01:51Z

@bors r+
You can squash the latest four commits into one if you would like. Two commits look better. This is not required.

rust-bors · 2026-11-12T15:02:01Z

📌 Commit a89dd90 has been approved by dianqk

It is now in the queue for this repository.

…ranch-164765, r=dianqk Avoid hoisting uninitialized reads in EarlyOtherwiseBranch Fixes rust-lang#164765. EarlyOtherwiseBranch can hoist a read onto a path where its operand is uninitialized, causing a miscompilation. When the parent’s otherwise branch is reachable, this restricts scalar hoisting to unchanged function arguments and their struct/tuple fields. It reuses VisitPlacesWith to reject writes, borrows, moves, and storage resets, without adding runtime branches. The tradeoff is conservative: some safe computed locals and borrowed or modified arguments lose this optimization. Supporting those cases would require a broader initialization analysis. (any suggestions or pointers here?)

…uwer Rollup of 7 pull requests Successful merges: - #161922 (Tie extern "llvm-intrinsics" and llvm.* symbol names together) - #164864 (Avoid hoisting uninitialized reads in EarlyOtherwiseBranch) - #165854 (Explicitly drop job server helper thread) - #164077 (replace `TypingEnv::post_analysis()` with `mir::Body::typing_env()`) - #165305 (hexagon: pass small aggregates on the stack when registers run out) - #165417 (Mention width limit panic in fmt doc) - #165860 (Fix inverted wasm cfg in core benchmarks)

darkraider01 · 2026-11-12T15:32:07Z

@bors r+ You can squash the latest four commits into one if you would like. Two commits look better. This is not required.

ah yea right

rust-bors · 2026-11-12T15:35:59Z

⚠️ A new commit 43dc29bb3fab2a69f47900f562f4b1315dda8bcf was pushed.

This pull request was unapproved.

This PR was contained in a rollup (#165873), which was closed.

dianqk · 2026-11-13T04:14:20Z

@bors r+

rust-bors · 2026-11-13T04:14:24Z

📌 Commit 43dc29b has been approved by dianqk

It is now in the queue for this repository.

…ranch-164765, r=dianqk Avoid hoisting uninitialized reads in EarlyOtherwiseBranch Fixes rust-lang#164765. EarlyOtherwiseBranch can hoist a read onto a path where its operand is uninitialized, causing a miscompilation. When the parent’s otherwise branch is reachable, this restricts scalar hoisting to unchanged function arguments and their struct/tuple fields. It reuses VisitPlacesWith to reject writes, borrows, moves, and storage resets, without adding runtime branches. The tradeoff is conservative: some safe computed locals and borrowed or modified arguments lose this optimization. Supporting those cases would require a broader initialization analysis. (any suggestions or pointers here?)

…uwer Rollup of 18 pull requests Successful merges: - #164828 (When suggesting path of renamed crate, use the renamed name) - #164864 (Avoid hoisting uninitialized reads in EarlyOtherwiseBranch) - #165068 (Change color usage for highlights) - #165221 (Canonicalize more comparison operators) - #165841 (Tell Miri about the alignment handling in the Windows System allocator) - #165869 (Cleanup: Remove `path_matches` and `value_str` from `AttributeExt`) - #165880 (Remove dead code for returning `BackendRepr::Memory` with `PassMode::Direct`) - #164108 (Add unstable `-Zllvm-tpde` flag to `rustc_codegen_llvm`) - #165565 (view-types: lower view types from HIR) - #165622 (small cleanups in rustc_codegen_llvm) - #165745 (Don't ICE when a guard condition is a place of type `!`) - #165771 (tests: cover MSVC ThinLTO exported symbol preservation) - #165810 (reflection: reflect attributes from adts, fields, and variants) - #165823 (Tweak crate error wording and turn them into fatal errors) - #165849 (More opt-dist improvements) - #165855 (Update `sysinfo` version to `0.40.3`) - #165872 (Some attribute related cleanups) - #165905 (CI: Use a consistent locale when sorting docker build inputs)

…uwer Rollup of 17 pull requests Successful merges: - #164828 (When suggesting path of renamed crate, use the renamed name) - #164864 (Avoid hoisting uninitialized reads in EarlyOtherwiseBranch) - #165068 (Change color usage for highlights) - #165221 (Canonicalize more comparison operators) - #165841 (Tell Miri about the alignment handling in the Windows System allocator) - #165869 (Cleanup: Remove `path_matches` and `value_str` from `AttributeExt`) - #165880 (Remove dead code for returning `BackendRepr::Memory` with `PassMode::Direct`) - #164108 (Add unstable `-Zllvm-tpde` flag to `rustc_codegen_llvm`) - #165565 (view-types: lower view types from HIR) - #165622 (small cleanups in rustc_codegen_llvm) - #165771 (tests: cover MSVC ThinLTO exported symbol preservation) - #165810 (reflection: reflect attributes from adts, fields, and variants) - #165823 (Tweak crate error wording and turn them into fatal errors) - #165849 (More opt-dist improvements) - #165855 (Update `sysinfo` version to `0.40.3`) - #165872 (Some attribute related cleanups) - #165905 (CI: Use a consistent locale when sorting docker build inputs)

Rollup merge of #164864 - darkraider01:fix-early-otherwise-branch-164765, r=dianqk Avoid hoisting uninitialized reads in EarlyOtherwiseBranch Fixes #164765. EarlyOtherwiseBranch can hoist a read onto a path where its operand is uninitialized, causing a miscompilation. When the parent’s otherwise branch is reachable, this restricts scalar hoisting to unchanged function arguments and their struct/tuple fields. It reuses VisitPlacesWith to reject writes, borrows, moves, and storage resets, without adding runtime branches. The tradeoff is conservative: some safe computed locals and borrowed or modified arguments lose this optimization. Supporting those cases would require a broader initialization analysis. (any suggestions or pointers here?)

Reproduce EarlyOtherwiseBranch reading an uninitialized union field

1998fb6

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 24, 2026

rustbot assigned nnethercote Oct 24, 2026

rustbot assigned dianqk and unassigned nnethercote Oct 26, 2026

dianqk reviewed Oct 28, 2026

View reviewed changes

Comment thread compiler/rustc_mir_transform/src/early_otherwise_branch.rs Resolved

Comment thread compiler/rustc_mir_transform/src/early_otherwise_branch.rs Resolved

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 Oct 29, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 29, 2026

Auto merge of #164864 - darkraider01:fix-early-otherwise-branch-16476…

22a54bb

…5, r=<try> Avoid hoisting uninitialized reads in EarlyOtherwiseBranch

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2026

darkraider01 requested a review from dianqk November 12, 2026 08:50

dianqk approved these changes Nov 12, 2026

View reviewed changes

Comment thread tests/mir-opt/early_otherwise_branch.rs Resolved

Comment thread tests/mir-opt/early_otherwise_branch.rs Resolved

rust-bors Bot 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 Nov 12, 2026

JonathanBrouwer mentioned this pull request Nov 12, 2026

Rollup of 7 pull requests #165873

Closed

Prevent early otherwise branch from hoisting uninitialized reads

43dc29b

darkraider01 force-pushed the fix-early-otherwise-branch-164765 branch from a89dd90 to 43dc29b Compare November 12, 2026 15:35

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2026

rust-bors Bot 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. labels Nov 13, 2026

JonathanBrouwer mentioned this pull request Nov 13, 2026

Rollup of 18 pull requests #165908

Closed

JonathanBrouwer mentioned this pull request Nov 13, 2026

Rollup of 17 pull requests #165918

Merged

rust-bors Bot merged commit 9c41224 into rust-lang:main Nov 13, 2026
14 checks passed

rustbot added this to the 1.102.0 milestone Nov 13, 2026

darkraider01 mentioned this pull request Nov 13, 2026

EarlyOtherwiseBranch can cause matches to read more bytes of the scrutinee #164135

Open

tmiasko mentioned this pull request Nov 13, 2026

EarlyOtherwiseBranch introduces access to a dead local variable #159618

Closed

Conversation

darkraider01 commented Oct 24, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 24, 2026

Uh oh!

rustbot commented Oct 24, 2026

Uh oh!

nnethercote commented Oct 26, 2026

Uh oh!

dianqk 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!

darkraider01 commented Oct 28, 2026

Uh oh!

dianqk commented Oct 29, 2026

Uh oh!

dianqk commented Oct 29, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Oct 29, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Oct 29, 2026

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

dianqk 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!

rustbot commented Nov 12, 2026

Uh oh!

dianqk commented Nov 12, 2026

Uh oh!

rust-bors Bot commented Nov 12, 2026

Uh oh!

darkraider01 commented Nov 12, 2026

Uh oh!

rust-bors Bot commented Nov 12, 2026

Uh oh!

dianqk commented Nov 13, 2026

Uh oh!

rust-bors Bot commented Nov 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

darkraider01 commented Oct 24, 2026 •

edited by rustbot

Loading

dianqk left a comment •

edited by rustbot

Loading

dianqk left a comment •

edited by rustbot

Loading