Skip to content

Conversation

rcvalle
Copy link
Member

@rcvalle rcvalle commented Aug 13, 2025

Fix #142284 by ensuring that #![no_builtins] crates can still emit bitcode when proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto) is used.

@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

r? @jieyouxu

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

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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 Aug 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 13, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@rcvalle
Copy link
Member Author

rcvalle commented Aug 13, 2025

This PR continues the discussion on #142323 about fixing #142284 with @maurer fix.

@rcvalle rcvalle added A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation A-sanitizers Area: Sanitizers for correctness and code quality labels Aug 13, 2025
@rcvalle
Copy link
Member Author

rcvalle commented Aug 13, 2025

r? dianqk

@rustbot rustbot assigned dianqk and unassigned jieyouxu Aug 13, 2025
@rcvalle
Copy link
Member Author

rcvalle commented Aug 13, 2025

I know there are some concerns with #118609, but it feels to me that this is the right fix (instead of avoiding indirect branches in the compiler_builtins crate). Optionally, we could enable it only when CFI is enabled for now and test it for some time for regressions.

@rcvalle rcvalle force-pushed the rust-cfi-fix-142284 branch from bb54ba9 to e3090b9 Compare August 13, 2025 18:43
@rustbot

This comment has been minimized.

@dianqk
Copy link
Member

dianqk commented Aug 17, 2025

As I mentioned at #142323 (comment), compiler_builtins doesn't participate in LTO. A example is:

# Cargo.toml
[profile.release]
lto = "fat"
codegen-units = 1

The output of RUSTC_LOG=rustc_codegen_llvm::back::lto RUSTFLAGS="--print=link-args" cargo +stage1 build --release --verbose -Zbuild-std is:

 INFO rustc_codegen_llvm::back::lto going for a fat lto
 INFO rustc_codegen_llvm::back::lto using "help.2d1a57ff1e402e78-cgu.0" as a base module
 INFO rustc_codegen_llvm::back::lto linking "addr2line-203643a25ae02a3b.addr2line.2fecf1c782c0419e-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "adler2-435b34f2723b9349.adler2.5df6e73bc23c9e9f-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "akgnnwb0o5wvr816kqr6rs0zh"
 INFO rustc_codegen_llvm::back::lto linking "alloc-5c9f657af48586ca.alloc.5820817416cb6e47-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "cfg_if-6a12be656f49f93d.cfg_if.3b09abd4d92c8bae-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "core-b177f723136f3556.core.823e6f9e4a114466-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "gimli-2d45c3d42fd51760.gimli.1ed3a06188407bf6-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "hashbrown-1fd960b5f06b8d80.hashbrown.23ad63d7e9e57be3-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "libc-a24c69218d75087e.libc.e551838495a52468-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "memchr-7888c46bca7dad36.memchr.46bd0df994389a33-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "miniz_oxide-3724888294b8a70c.miniz_oxide.43628447bf809289-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "object-d31066ec128d3f17.object.1f32eb2f555bed60-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "panic_unwind-4bae356bb465b997.panic_unwind.d91fe3411b3e54dc-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "rustc_demangle-d773373ca3cf9715.rustc_demangle.c5e32cb73aaf322f-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "rustc_std_workspace_alloc-1b4f5d0d4833b3d2.rustc_std_workspace_alloc.81bced7e86f59962-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "rustc_std_workspace_core-564695c546b95ef2.rustc_std_workspace_core.9a9d63330e2c57e1-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "std-3a81b2fb00659a54.std.95d71bf3291e81c9-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "std_detect-60aefcce94c834b9.std_detect.7972f7f32da3ce54-cgu.0.rcgu.o"
 INFO rustc_codegen_llvm::back::lto linking "unwind-c94884f8b6cde95c.unwind.8063d473e65bf815-cgu.0.rcgu.o"

There should be a log like INFO rustc_codegen_llvm::lto linking "compiler_builtins-hash.compiler_builtins.hash-cgu.0.rcgu.o".

@rcvalle
Copy link
Member Author

rcvalle commented Aug 26, 2025

Sorry, I should've described it better. The ignored_for_lto function in compiler/rustc_codegen_ssa/src/back/link.rs already handles the special case for #![no_builtins] crates by excluding them from LTO participation. The bitcode emission decision should be separate from the LTO participation decision.

This fix ensures that #![no_builtins] crates can still emit bitcode when proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto) is used, which is necessary for CFI to work properly. The fix doesn't change the behavior of how #![no_builtins] crates are handled in LTO--it just allows them to emit bitcode when needed for CFI (or proper [i.e., non-rustc] LTO [i.e., -Clinker-plugin-lto] is used]).

Fix rust-lang#142284 by ensuring that `#![no_builtins]` crates can still emit bitcode
when proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto) is used.
@rcvalle rcvalle force-pushed the rust-cfi-fix-142284 branch from e3090b9 to cf8753e Compare August 26, 2025 23:33
@rustbot

This comment was marked as spam.

@dianqk
Copy link
Member

dianqk commented Aug 28, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 28, 2025

📌 Commit cf8753e has been approved by dianqk

It is now in the queue for this repository.

@bors bors 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 Aug 28, 2025
bors added a commit that referenced this pull request Aug 28, 2025
Rollup of 6 pull requests

Successful merges:

 - #142472 (Add new `doc(attribute = "...")` attribute)
 - #145368 (CFI: Make `lto` and `linker-plugin-lto` work the same for `compiler_builtins`)
 - #145853 (Improve error messages around invalid literals in attribute arguments)
 - #145920 (bootstrap: Explicitly mark the end of a failed test's captured output)
 - #145937 (add doc-hidden to exports in attribute prelude)
 - #145965 (Move exporting of profiler and sanitizer symbols to the LLVM backend)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9e4a283 into rust-lang:master Aug 28, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 28, 2025
rust-timer added a commit that referenced this pull request Aug 28, 2025
Rollup merge of #145368 - rcvalle:rust-cfi-fix-142284, r=dianqk

CFI: Make `lto` and `linker-plugin-lto` work the same for `compiler_builtins`

Fix #142284 by ensuring that `#![no_builtins]` crates can still emit bitcode when proper (i.e., non-rustc) LTO (i.e., -Clinker-plugin-lto) is used.
@rcvalle rcvalle deleted the rust-cfi-fix-142284 branch August 29, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-control-flow-integrity Area: Control Flow Integrity (CFI) security mitigation A-sanitizers Area: Sanitizers for correctness and code quality PG-exploit-mitigations Project group: Exploit mitigations 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.

rustc-LLVM ERROR of ControlFlowIntegrit
6 participants