allow -Ldependency search paths for panic runtimes

[Bryanskiy]

View all comments

Part of build-std=always RFC.

This PR supports -Ldependency= search paths for panic runtimes. This will allow Cargo to treat panic runtimes as regular transitive dependencies, rather than special-casing them.

To do so:

1. During panic runtime injection we remove the early‑return only_rlib check. As a result the panic runtime is injected as a conditional dependency when building std and is subsequently included in std's metadata. When building a crate that uses std, the panic runtime is resolved through resolve_crate_deps in the same way as any other transitive dependency.
   
   For a prebuilt std (unlike build-std), the -Cpanic= option passed to std may differ from that passed to its dependents, so we might still need to inject another panic runtime as a direct dependency. However, this is not important in the context of prebuilt std.
2. NoPanicStrategy is checked only for panic_abort. We can compile panic_unwind with -Cpanic=abort for some targets. This was previously overlooked.

Summary of the history and discussions in this PR

1. In the first iteration of this PR PathKind for injected dependencies was simply changed from -Lcrate= to -Ldependency, but this raised a concern. Bjorn3 suggested to include both panic runtimes as a dependency of std so we don't have to inject it as a dependency for any user of std. The correct one should then be activated later in dependency_format.rs.
2. In the second iteration Bjorn3's suggestion was implemented. The problem was that for build-std=always, we might want to avoid building panic_unwind when not necessary. From the compiler perspective we need to
   
   2.1. Either always resolve panic_unwind and ignore any resulting error. (might lead to bugs)
   2.2. Or conditionally resolve panic_unwind. For build-std we might rely on -Cpanic= option for this in accordance with panic strategies but for prebuilt std this will fail for some targets.
   
   It was decided to relax the implementation so we can inject panic runtime as direct dependency for prebuilt std.
3. Third iteration: we are here

cc @adamgemmell

[rustbot]

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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 74 candidates
• Random selection from 18 candidates

[Bryanskiy]

@rustbot blocked on #159717

[bjorn3]

This would risk accidentally getting a crates.io crate replacing compiler-builtins or a panic runtime from the sysroot even when it isn't a direct dependency, right? Currently -Ldependency is only used for crates that are indirect dependencies and thus we already know the crate hash for to prevent such accidental inclusion.

[petrochenkov]

One possible alternative is using something like force (ExternEntry::force) + no-link (CrateDepKind::MacrosOnly) for dependencies like panic runtime. Then the panic runtime will be present in the dependency tree, its crate hash will be recorded, and it will be naturally found in the dependency directories.

inject_panic_runtime and friends will just need to promote such dependencies from "no-link" to "link".

[bjorn3]

Wouldn't --extern panic_abort=... --extern panic_unwind=... already be enough? I would expect the panic runtime injection code to already look at --extern.

[petrochenkov]

--extern panic_abort=... --extern panic_unwind=... exist when building std, but they are not not actually used by std so the crates are not loaded and the options are lost. (#160007 (comment) would make them loaded and not lost, just not linked).

When cargo builds some final binary depending on libstd, panic runtime crates are already indirect dependencies.
So cargo doesn't pass them with --extern, and as Kirill explained to me, cargo doesn't want to pass them with --extern at this stage, because it would contradict the cargo's internal logic and design.
Cargo wants library\std\Cargo.toml to be the only place where these dependencies are specified.

[bjorn3]

--extern panic_abort=... --extern panic_unwind=... exist when building std

I mean when building user crates.

but they are not not actually used by std so the crates are not loaded and the options are lost.

At panic_unwind is already listed as conditional dependency of std, see rustc +nightly -Zls=root $(rustc +nightly --print target-libdir)/libstd-*.rmeta:

[...]
18 panic_unwind-aff6afe86ac8b0f1 hash 362c7180b46c57babeedc50f4e9b00d0 host_hash None kind Conditional private linkage Some(RequireStatic)

panic_abort isn't though.

[Bryanskiy]

I mean when building user crates.

From my understanding, we want to avoid special cases for compiler_builtins/panic runtimes in Cargo as much as possible. But @adamgemmell might know the motivation better.

[adamgemmell]

That's indeed the reason, we'd like Cargo to only have to be aware of stable crates (plus test)

This would risk accidentally getting a crates.io crate replacing compiler-builtins or a panic runtime from the sysroot even when it isn't a direct dependency, right?

rustc does report an error when it finds more than one candidate, including if one of these candidates is in the sysroot. So for this to happen, rustc would need to want to inject a crate that cargo hasn't built as part of build-std (and doesn't exist in the sysroot), which I don't think is possible right now.

If it is possible I think you'd have to pass a panic strategy via RUSTFLAGS rather than the profile, and we do have this line in the RFC to fall back on:

In line with Cargo’s stance on not parsing the RUSTFLAGS environment variable, it will not be checked for compilation flags that would require additional crates to be built for compilation to succeed.

[bjorn3]

rustc does report an error when it finds more than one candidate, including if one of these candidates is in the sysroot.

Sysroot crates should have lower priority than user crates. Otherwise it wouldn't be possible to use libc from crates.io.

Would it work if we make rustc add a conditional dependency on both panic_unwind and panic_abort when compiling libstd rather than only on panic_unwind?

[adamgemmell]

Sorry, I'm just referring to injected dependencies, for which Kirill found that rustc threw an error when he added -Ldependency args to their search paths. That's why this patch now depends on the one for -Zimplicit-sysroot-deps.

Always building both is an option, but the Cargo team expressed a preference for not building panic_unwind if it can be avoided. Hence we don't pass the panic-unwind feature if the profile panic is set to abort. We have a future possibility to get the target's default panic strategy to avoid building it when the panic strategy is set to "unwind" (aka target default in the profile).

Sorry, I'm clearly not awake yet!

[Bryanskiy]

Sysroot crates should have lower priority than user crates. Otherwise it wouldn't be possible to use libc from crates.io.

There is no such thing as priority from the crate locator perspective:

rust/compiler/rustc_metadata/src/locator.rs

Lines 502 to 505 in dfbea5b

	match libraries.len() {
	0 => Ok(None),
	1 => Ok(Some(libraries.into_iter().next().unwrap().1)),
	_ => {

The right version of libc is chosen based on svh, I think.

Would it work if we make rustc add a conditional dependency on both panic_unwind and panic_abort when compiling libstd rather than only on panic_unwind?

When compiling libstd as rlib, we don't inject the panic runtime at all. So the panic runtime is not present in std metadata as a dependency and therefore is not loaded like regular transitive dependencies.

[Bryanskiy]

Always building both is an option

It's not about building a panic runtime. It's about rustc representation of dependencies in the crate graph. Here is what conditional dependency mean:

rust/compiler/rustc_session/src/cstore.rs

Lines 42 to 44 in ad0c9dc

	/// A dependency that is injected into the crate graph but which only
	/// sometimes needs to actually be linked in, e.g., the injected panic runtime.
	Conditional,

[bjorn3]

The right version of libc is chosen based on svh, I think.

For indirect dependencies yes. For direct dependencies the --extern libc=/path/to/rlib passed by cargo has a higher priority than the version in the sysroot. If they had identical priority, any attempt to use libc from crates.io would result in rustc emitting an error that it doesn't know if it should pick the one passed by cargo or the one in the sysroot.

When compiling libstd as rlib, we don't inject the panic runtime at all. So the panic runtime is not present in std metadata as a dependency and therefore is not loaded like regular transitive dependencies.

panic_unwind is absolutely present as conditional dependency in the libstd crate metadata:

rustc +nightly -Zls=root $(rustc +nightly --print target-libdir)/libstd-*.rmeta | rg "Dependencies|panic_unwind"
=External Dependencies=
18 panic_unwind-aff6afe86ac8b0f1 hash 362c7180b46c57babeedc50f4e9b00d0 host_hash None kind Conditional private linkage Some(RequireStatic)

The problem is that the panic runtime injection code attempts to include it as direct dependency. My suggestion is to include both panic runtimes (or one if the other hasn't been built) as conditional dependency of libstd replacing the code at

rust/compiler/rustc_metadata/src/creader.rs

Lines 956 to 1025 in ad0c9dc

	fn inject_panic_runtime(&mut self, tcx: TyCtxt<'_>, krate: &ast::Crate) {
	// If we're only compiling an rlib, then there's no need to select a
	// panic runtime, so we just skip this section entirely.
	let only_rlib = tcx.crate_types().iter().all(|ct| *ct == CrateType::Rlib);
	if only_rlib {
	info!("panic runtime injection skipped, only generating rlib");
	return;
	}
	// If we need a panic runtime, we try to find an existing one here. At
	// the same time we perform some general validation of the DAG we've got
	// going such as ensuring everything has a compatible panic strategy.
	let mut needs_panic_runtime = attr::contains_name(&krate.attrs, sym::needs_panic_runtime);
	for (_cnum, data) in self.iter_crate_data() {
	needs_panic_runtime |= data.needs_panic_runtime();
	}
	// If we just don't need a panic runtime at all, then we're done here
	// and there's nothing else to do.
	if !needs_panic_runtime {
	return;
	}
	// By this point we know that we need a panic runtime. Here we just load
	// an appropriate default runtime for our panic strategy.
	//
	// We may resolve to an already loaded crate (as the crate may not have
	// been explicitly linked prior to this), but this is fine.
	//
	// Also note that we have yet to perform validation of the crate graph
	// in terms of everyone has a compatible panic runtime format, that's
	// performed later as part of the `dependency_format` module.
	let desired_strategy = tcx.sess.panic_strategy();
	let name = match desired_strategy {
	PanicStrategy::Unwind => sym::panic_unwind,
	PanicStrategy::Abort => sym::panic_abort,
	PanicStrategy::ImmediateAbort => {
	// Immediate-aborting panics don't use a runtime.
	return;
	}
	};
	info!("panic runtime not found -- loading {}", name);
	// This has to be conditional as both panic_unwind and panic_abort may be present in the
	// crate graph at the same time. One of them will later be activated in dependency_formats.
	let Some(cnum) = self.resolve_crate(
	tcx,
	name,
	DUMMY_SP,
	CrateDepKind::Conditional,
	CrateOrigin::Injected,
	) else {
	return;
	};
	let cdata = self.get_crate_data(cnum);
	// Sanity check the loaded crate to ensure it is indeed a panic runtime
	// and the panic strategy is indeed what we thought it was.
	if !cdata.is_panic_runtime() {
	tcx.dcx().emit_err(diagnostics::CrateNotPanicRuntime { crate_name: name });
	}
	if cdata.required_panic_strategy() != Some(desired_strategy) {
	tcx.dcx().emit_err(diagnostics::NoPanicStrategy {
	crate_name: name,
	strategy: desired_strategy,
	});
	}
	self.injected_panic_runtime = Some(cnum);
	}

This way we don't have to inject it as dependency for any user of libstd. We already have code to activate exactly one panic runtime later on in the dependency format code:

rust/compiler/rustc_metadata/src/dependency_format.rs

Lines 253 to 260 in ad0c9dc

	// We've gotten this far because we're emitting some form of a final
	// artifact which means that we may need to inject dependencies of some
	// form.
	//
	// Things like panic runtimes may not have been activated quite yet, so do so here.
	activate_injected_dep(CStore::from_tcx(tcx).injected_panic_runtime(), &mut ret, &|cnum| {
	tcx.is_panic_runtime(cnum)
	});

[petrochenkov]

We already have code to activate exactly one panic runtime later on in the dependency format code

Then it seems like CrateDepKind::Conditional already mostly implements what I suggested in #160007 (comment).

[Bryanskiy]

I'm curious what should we do with compiler_builtins.

On the one hand, as Adam already said, we want to use only stable names in Cargo and therefore remove compiler_builtins from the build-std implementation:

https://github.com/rust-lang/cargo/blob/0158e40d8638a7de292b7242b1533caaf48cbe5f/src/compiler/standard_lib.rs#L31
+
https://github.com/rust-lang/cargo/blob/0158e40d8638a7de292b7242b1533caaf48cbe5f/src/compiler/standard_lib.rs#L41

On the other hand compiler_builtins is not an explicit dependency of core(and should not be) in Cargo.toml. This means that after removal, Cargo will not be able to figure out whether it needs to build compiler_builtins for -Zbuild-std=core.

[adamgemmell]

I was planning on making it a dependency of sysroot and using that crate as the sort of "global entry point" when needed (such as for -Zbuild-std-features), but not generate an actual unit to build it in Cargo

[Bryanskiy]

For panic runtimes I implemented #160007 (comment).
For compiler_builtins crate -Ldependency= paths are not supported in the case of -Zbuild-std=core, but it's blocked on the mechanism mentioned in #160007 (comment).

[Bryanskiy]

@rustbot ready

While debugging the last CI failure I realized that conditional resolution might fail for some targets when using prebuilt std. I found a much simpler fix: remove the only_rlib early return and check that we already have the panic runtime in the crate graph. Also see the comments.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[petrochenkov]

@bors delegate=try
@Bryanskiy in case you want to check this on wasm or something else.

I'll r+ this in a ~week since this is blocking other work.

[rust-bors]

✌️ @Bryanskiy, you can now perform try builds on this pull request!

Warning

You used the legacy format of the delegate command.
The new format is @bors delegate[=<username>] [<try|review>].

You can now post @bors try to start a try build.

[petrochenkov]

@bors r+

[rust-bors]

📌 Commit 847ad1c has been approved by petrochenkov

It is now in the queue for this repository.

[rust-bors]

☀️ Test successful - CI
Approved by: petrochenkov
Duration: 3h 11m 18s
Pushing e081d66 to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 3ffb26f (parent) -> e081d66 (this PR)

Test differences

Show 6 test diffs

Stage 1

• [run-make] tests/run-make-cargo/panic-strategies: [missing] -> pass (J0)
• [run-make] tests/run-make/locate-panic-runtime: [missing] -> pass (J0)

Stage 2

• [run-make] tests/run-make-cargo/panic-strategies: [missing] -> pass (J1)
• [run-make] tests/run-make/locate-panic-runtime: [missing] -> pass (J1)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

• J0: x86_64-gnu-llvm-21-3, x86_64-gnu-llvm-22-3
• J1: aarch64-apple-1, aarch64-apple-macos-26-1, aarch64-gnu, aarch64-gnu-debug, aarch64-gnu-llvm-21-1, aarch64-msvc-1, arm-android, armhf-gnu, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-debug, x86_64-gnu-gcc, x86_64-gnu-llvm-21-2, x86_64-gnu-llvm-22-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard e081d6616f3be5a96c6b42eecc6410f1a5378b7e --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. armhf-gnu: 56m 39s -> 1h 31m (+61.9%)
2. x86_64-gnu-stdlib-semver-check: 17m 36s -> 9m 51s (-44.0%)
3. dist-android: 23m 28s -> 31m 44s (+35.2%)
4. dist-x86_64-mingw: 2h 3m -> 2h 46m (+34.7%)
5. i686-gnu-1: 2h 12m -> 1h 28m (-33.2%)
6. x86_64-gnu-debug: 2h 2m -> 1h 24m (-31.2%)
7. dist-aarch64-linux: 3h 27m -> 2h 22m (-31.2%)
8. i686-msvc-1: 2h 28m -> 3h 8m (+27.3%)
9. dist-ohos-armv7: 1h 2m -> 1h 18m (+25.4%)
10. aarch64-apple-macos-26-2: 2h 30m -> 1h 53m (-24.6%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (e081d66): comparison URL.

Overall result: ❌ regressions - no action needed

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

Max RSS (memory usage)

Results (primary 3.8%)

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

	mean	range	count
Regressions ❌ (primary)	3.8%	[3.8%, 3.8%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.8%	[3.8%, 3.8%]	1

Cycles

Results (secondary -2.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
Improvements ✅ (secondary)	-2.7%	[-3.1%, -2.2%]	2
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.1%, secondary 0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.2%]	54
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.2%]	54

Bootstrap: 476.994s -> 473.971s (-0.63%)
Artifact size: 402.58 MiB -> 402.61 MiB (0.01%)