Skip to content

Emit retags in codegen to support BorrowSanitizer (part 4)#158100

Open
icmccorm wants to merge 2 commits into
rust-lang:mainfrom
BorrowSanitizer:codegen-emit-retag-4
Open

Emit retags in codegen to support BorrowSanitizer (part 4)#158100
icmccorm wants to merge 2 commits into
rust-lang:mainfrom
BorrowSanitizer:codegen-emit-retag-4

Conversation

@icmccorm

@icmccorm icmccorm commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: #154760
Zulip Thread

This is one of several PRs that add experimental support for emitting retags as function calls in codegen.

This PR emits our retag intrinsics based on the structure of a RetagPlan. The feature works end-to-end now! But, it has two limitations:

  • We do not track interior mutable or pinned ranges precisely. This will come in the next PR, which should be the last one that we need to complete this feature (excluding documentation and compile tests).

  • We recurse into repr(simd) types when building a RetagPlan, but we do not emit retags for their fields. We would need BuilderMethods::insert_element, which isn't available. I'm not sure what the best workaround would be for this.

Related: #157825, #156210, #156208

Cc: @RalfJung
r? @saethlin

@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 Jun 18, 2026
@rust-log-analyzer

This comment has been minimized.

Comment thread compiler/rustc_codegen_ssa/src/mir/retag.rs Outdated
Comment on lines +354 to +356
// If the variant contains another variant, then the current block
// will be different than the one that we created above. We want this
// block to jump to the terminator block.

@saethlin saethlin Jun 27, 2026

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.

I don't think this is quite right. For example:

#![crate_type = "lib"]

pub enum Inner {
    A(&'static u8),
    B(&'static u16),
}

pub enum Outer {
    P(&'static u32),
    Q(Inner),
}

#[inline(never)]
pub fn demo(_x: Outer) {}

produces a block for the Outer::Q case that recurses into the Inner, but that block has no predecessors.

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch! This should be fixed now with 19ba1b0.

@saethlin

saethlin commented Jul 1, 2026

Copy link
Copy Markdown
Member

@bors squash

@rust-bors

rust-bors Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔑 The Allow edits by maintainers option is not enabled on this PR. It is required for squashing to work.

@saethlin

saethlin commented Jul 1, 2026

Copy link
Copy Markdown
Member

Oof. @icmccorm Can you combine the commits into perhaps two commits, or check the box that lets maintainers push changes?

Then r=me

@icmccorm icmccorm force-pushed the codegen-emit-retag-4 branch from 19ba1b0 to c268765 Compare July 2, 2026 14:35
@icmccorm

icmccorm commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@saethlin I squashed this into two commits.

Since our fork is under an org, GitHub doesn't support letting maintainers make changes, unless I add each individual maintainer to the org. I can add you, if you like. Otherwise, I can just do this from a personal fork in the future.

@saethlin

saethlin commented Jul 2, 2026

Copy link
Copy Markdown
Member

Since our fork is under an org, GitHub doesn't support letting maintainers make changes, unless I add each individual maintainer to the org. I can add you, if you like. Otherwise, I can just do this from a personal fork in the future.

That's very unfortunate. Don't bother changing your workflow around this limitation, I'll just be ready for it in the future :p

@saethlin

saethlin commented Jul 2, 2026

Copy link
Copy Markdown
Member

@bors r+

@rust-bors

rust-bors Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

📌 Commit c268765 has been approved by saethlin

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@rust-bors 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 Jul 2, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 2, 2026
…-4, r=saethlin

Emit retags in codegen to support BorrowSanitizer (part 4)

Tracking issue: rust-lang#154760
[Zulip Thread](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Staging.20for.20emitting.20retags.20in.20codegen/with/593004012)

This is one of several PRs that add experimental support for emitting retags as function calls in codegen.

This PR emits our retag intrinsics based on the structure of a `RetagPlan`. The feature works end-to-end now! But, it has two limitations:

* We do not track interior mutable or pinned ranges precisely. This will come in the next PR, which should be the last one that we need to complete this feature (excluding documentation and compile tests).

* We recurse into `repr(simd)` types when building a `RetagPlan`, but we do not emit retags for their fields. We would need `BuilderMethods::insert_element`, which isn't available. I'm not sure what the best workaround would be for this.

Related: rust-lang#157825, rust-lang#156210, rust-lang#156208

Cc: @RalfJung
r? @saethlin
rust-bors Bot pushed a commit that referenced this pull request Jul 3, 2026
…uwer

Rollup of 22 pull requests

Successful merges:

 - #155429 (Support `u128`/`i128` c-variadic arguments)
 - #158100 (Emit retags in codegen to support BorrowSanitizer (part 4))
 - #158494 (Improve E0277 diagnostics for conditionally implemented traits)
 - #158606 (use ProjectionPredicate instead of AliasRelate)
 - #158627 (Simplify option-iterator flattening in the compiler)
 - #158658 (Update LLVM submodule)
 - #158665 (Revert "Remove redundant dyn-compatibility check.")
 - #158021 (Remove old MinGW workaround)
 - #158473 (Add `riscv32imfc-unknown-none-elf` bare-metal target)
 - #158549 (process::exec: using appropriate exit code on vxworks.)
 - #158585 (Improve diagnostic for too many super keywords)
 - #158637 (hir_ty_lowering: avoid self type lookup for inherent aliases)
 - #158651 (ptr doc: reduce use of unsafe block to where needed)
 - #158669 (Remove `src/tools/test-float-parse/Cargo.lock`)
 - #158674 (library: Polish transmute's `split_at_stdlib` example)
 - #158677 (Add extra splat tests)
 - #158680 (Avoid ICE for `NonZero<char>` in improper_ctypes)
 - #158681 (Remove unnecessary `Hash` derives from MIR types)
 - #158682 (Avoid delayed bug for disabled on_type_error arguments)
 - #158684 (Add missing generic test coverage for ```#[splat]```)
 - #158687 (Streamline `MacEager`)
 - #158688 (Cleanup attribute docs and add links to other mentioned attributes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants