CFI: Fix LTO for #![no_builtins] crates with CFI

[rcvalle]

Fixes LTO for #![no_builtins] crates with CFI enabled by using rustc's EmitObj::Bitcode path (and emitting LLVM bitcode in the .o for linker-based LTO).

In addition to adding a specific regression test for #142284, it also adds a test that verifies that the examples in rust-cfi-examples build with -Zbuild-std to prevent other future regressions.

[rustbot]

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 20 candidates

[bjorn3]

The reason we don't use LTO for compiler-builtins is that doing so breaks the weak linkage that compiler-builtins uses. We use a legacy interface for implementing LTO that treats weak and strong symbols identically. We tried to make compiler-builtins participate in LTO before in #113923, but this resulted in the regression #118609.

In any case just emitting bitcode is not enough. You also need to updated ignored_for_lto:

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Lines 1406 to 1422 in 0469a92

	/// Returns a boolean indicating whether the specified crate should be ignored
	/// during LTO.
	///
	/// Crates ignored during LTO are not lumped together in the "massive object
	/// file" that we create and are linked in their normal rlib states. See
	/// comments below for what crates do not participate in LTO.
	///
	/// It's unusual for a crate to not participate in LTO. Typically only
	/// compiler-specific and unstable crates have a reason to not participate in
	/// LTO.
	pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool {
	// If our target enables builtin function lowering in LLVM then the
	// crates providing these functions don't participate in LTO (e.g.
	// no_builtins or compiler builtins crates).
	!sess.target.no_builtins
	&& (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum))
	}

View changes since the review

[rcvalle]

This change only makes it participate on LTO when using both -Clinker-plugin-lto (not rustc LTO) and -Zsanitizer=cfi, which expects a clang or ld.lld linker already and thus doesn't cause the regressions listed on #146133, with the regression tests added by @dianqk now passing. However, I see that the regressions you mention are different ones. Are there any regression tests for them I can verify?

[bjorn3]

Cargo uses -Clinker-plugin-lto when compiling rlib crates even when not doing linker plugin LTO to skip unnecessary codegen for them.

[rcvalle]

I'm a bit confused about what is the issue you're referring to. ignored_for_lto is for rustc LTO (not proper LTO). The #142284 issue is when using both -Clinker-plugin-lto (proper LTO) and -Zsanitizer=cfi, where LTO is deferred to the linker; the fix needed was to ensure those crates emit LLVM bitcode so proper LTO can run the LowerTypeTests pass and lower llvm.type.test intrinsics and related metadata. (Note the issue #142284 doesn't happen with rust LTO: #142284 (comment) because these are properly lowered already.) Would you mind providing more details? Maybe a regression test or a reproducer?

[bjorn3]

I was thinking wrong in my earlier comments. I however did just remember that you tried to land pretty much the same PR previously as #145368, which then had to be reverted in #146133 due to regressions. Has anything changed since then to fix those regressions even when compiling compiler-builtins with LTO enabled?

[jieyouxu]

r? bjorn3 (since you're already reviewing this, and know way more about this than I do)