Change int-to-ptr transmute lowering back to inttoptr #147541

saethlin · 2025-10-09T23:00:44Z

This is a revert of #121282, but with a regression test to cover the reported miscompile in #147265 that was caused by the way the code here combines with #138759.

saethlin · 2025-10-09T23:13:15Z

This is a mitigation for a miscompilation, and we do not have a good understanding of how widespread the miscompile is. The answer could be "very". I've started a crater run to evaluate this: #138759 (comment) but it will take a significant amount of time for results to come in.

I am not sure it makes sense to wait for that crater run before making a backport decision, considering how long it will take.

scottmcm · 2025-10-09T23:15:15Z

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

        (Int(..) | Float(_), Int(..) | Float(_)) => bx.bitcast(imm, to_backend_ty),
        (Pointer(..), Pointer(..)) => bx.pointercast(imm, to_backend_ty),
-        (Int(..), Pointer(..)) => bx.ptradd(bx.const_null(bx.type_ptr()), imm),
+        (Int(..), Pointer(..)) => bx.inttoptr(imm, to_backend_ty),


Hmm, maybe we could add a BuilderMethods::inttoptr_noprovenance for this, as that would give us a place to have a -Z flag that affects what it does so we could put it back to the better version more easily later?

(Maybe skip the -Z flag in this PR, if we want something that could go to beta, but if it existed we could test both ways in the codegen test.)

Writing the actual implementation here took me about a minute, and I expect it will be similarly near-zero effort to swap it back again. Adding a helper would be more effort.

It just feels like a -Z mutable-noalias situation, where being able to test whether something is still broken without a new build is the reason to do it, not because the actual code change is hard.

A dedicated method would also allow backends which don't miscompile it to keep avoiding the provenance.

rustbot · 2025-10-10T00:17:08Z

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

rustbot · 2025-10-10T00:17:10Z

r? @lcnr

rustbot has assigned @lcnr.
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

saethlin · 2025-10-10T00:17:31Z

r? scottmcm
(you're already here)

saethlin · 2025-10-10T03:46:32Z

@bors try @rust-timer queue

Change int-to-ptr transmute lowering back to inttoptr

rust-bors · 2025-10-10T05:58:20Z

☀️ Try build successful (CI)
Build commit: 9e02838 (9e0283899906af4ae3914a14bee66862c17cec08, parent: 9725c4baacef19345e13f91b27e66e10ef5592ae)

scottmcm

Well, r=me on the code changes if we decide this is the direction we want to go. TBH personally I'd still rather disable the problematic LLVM optimization instead, if we can.

View changes since this review

scottmcm · 2025-10-10T06:05:51Z

tests/codegen-llvm/int-ptr-int-enum-miscompile.rs

+#[no_mangle]
+pub fn mk_result(a: usize) -> Result<u8, *const u8> {
+    // CHECK-LABEL: @mk_result
+    // CHECK-NOT: unreachable


suggestion: it's always good to have positive checks too, just in case the negative thing changes, so maybe

Suggested change

// CHECK-NOT: unreachable

// CHECK-NOT: unreachable

// CHECK: load i8,

// CHECK-NOT: unreachable

rust-timer · 2025-10-10T07:09:03Z

Finished benchmarking commit (9e02838): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in 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.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -2.3%, secondary 0.9%)

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

Cycles

Results (secondary -2.8%)

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

Binary size

Results (primary -0.2%, secondary -0.3%)

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.2%	[-0.5%, -0.0%]	16
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.0%]	56
All ❌✅ (primary)	-0.2%	[-0.5%, -0.0%]	16

Bootstrap: 472.692s -> 472.611s (-0.02%)
Artifact size: 388.03 MiB -> 388.05 MiB (0.00%)

saethlin · 2025-10-10T21:46:31Z

@bors r=scottmcm rollup=iffy

bors · 2025-10-10T21:46:34Z

📌 Commit 8d6aa60 has been approved by scottmcm

It is now in the queue for this repository.

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 9, 2025

saethlin mentioned this pull request Oct 9, 2025

Rust 1.90 compiler optimization (O2, O3) triggers UB with Box::from_raw() #147265

Open

scottmcm reviewed Oct 9, 2025

View reviewed changes

saethlin marked this pull request as ready for review October 10, 2025 00:17

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2025

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 10, 2025

rustbot assigned lcnr Oct 10, 2025

rustbot assigned scottmcm and unassigned lcnr Oct 10, 2025

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Oct 10, 2025

Auto merge of #147541 - saethlin:transmute-carefully, r=<try>

9e02838

Change int-to-ptr transmute lowering back to inttoptr

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 10, 2025

This comment has been minimized.

Sign in to view

scottmcm approved these changes Oct 10, 2025

View reviewed changes

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

nikic approved these changes Oct 10, 2025

View reviewed changes

Change int-to-ptr transmute lowering back to inttoptr

8d6aa60

saethlin force-pushed the transmute-carefully branch from 511ef23 to 8d6aa60 Compare October 10, 2025 13:31

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2025

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 10, 2025

saethlin added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Oct 10, 2025

Change int-to-ptr transmute lowering back to inttoptr #147541

Are you sure you want to change the base?

Change int-to-ptr transmute lowering back to inttoptr #147541

Conversation

saethlin commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saethlin commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Oct 10, 2025

Uh oh!

rustbot commented Oct 10, 2025

Uh oh!

saethlin commented Oct 10, 2025

Uh oh!

saethlin commented Oct 10, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors bot commented Oct 10, 2025

Uh oh!

This comment has been minimized.

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

Choose a reason for hiding this comment

Uh oh!

rust-timer commented Oct 10, 2025

Overall result: ❌ regressions - no action needed

Uh oh!

saethlin commented Oct 10, 2025

Uh oh!

bors commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

saethlin commented Oct 9, 2025 •

edited

Loading

saethlin commented Oct 9, 2025 •

edited

Loading

scottmcm left a comment •

edited by rustbot

Loading