remap libstd paths in backtraces for test crates

[jonas-schievink]

I just got this backtrace when running RUST_BACKTRACE=1 cargo test on salsa-rs/salsa@bb4b42a:

thread 'frozen::in_par_get_set_cancelation' panicked at 'called `Result::unwrap()` on an `Err` value: Any { .. }', tests/parallel/frozen.rs:64:33
stack backtrace:
   0: rust_begin_unwind
             at /rustc/881c1ac408d93bb7adaa3a51dabab9266e82eee8/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/881c1ac408d93bb7adaa3a51dabab9266e82eee8/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/881c1ac408d93bb7adaa3a51dabab9266e82eee8/library/core/src/result.rs:1355:5
   3: core::result::Result<T,E>::unwrap
             at /home/jonas/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:1037:23
   4: parallel::frozen::in_par_get_set_cancelation
             at ./tests/parallel/frozen.rs:64:18
   5: parallel::frozen::in_par_get_set_cancelation::{{closure}}
             at ./tests/parallel/frozen.rs:13:1
   6: core::ops::function::FnOnce::call_once
             at /home/jonas/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/881c1ac408d93bb7adaa3a51dabab9266e82eee8/library/core/src/ops/function.rs:227:5

The remapping from /rustc/... paths to the rust-src component only seems to kick in once here, but I expected it to always work (since the component is installed). Frames 6 and 7 also point to the exact same location, but path remapping still failed for one of them.

[jyn514]

@jonas-schievink what version of rustc is this?

[jonas-schievink]

This is on rustc 1.54.0-nightly (881c1ac40 2021-05-08)

[jyn514]

Can you try with latest nightly? #83813 was merged recently and may have fixed this.

[jonas-schievink]

The most recent nightly seems to work even less:

thread 'frozen::in_par_get_set_cancelation' panicked at 'called `Result::unwrap()` on an `Err` value: Any { .. }', tests/parallel/frozen.rs:64:33
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/core/src/result.rs:1355:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/core/src/result.rs:1037:23
   4: parallel::frozen::in_par_get_set_cancelation
             at ./tests/parallel/frozen.rs:64:18
   5: parallel::frozen::in_par_get_set_cancelation::{{closure}}
             at ./tests/parallel/frozen.rs:13:1
   6: core::ops::function::FnOnce::call_once
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/core/src/ops/function.rs:227:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/4e3e6db011c5b482d2bef8ba02274657f93b5e0d/library/core/src/ops/function.rs:227:5

[jyn514]

@jonas-schievink what do you mean? Those look correctly remapped to /rustc/hash to me, were you expecting something else?

[jyn514]

Oh I see, you want it to not remap and use the local path.

[jyn514]

Note that people have requested the opposite behavior in the past, but it seems reasonable to do this only for tests: #82188

[jonas-schievink]

The /rustc/<hash> path is the non-remapped path. I have the rust-src component installed, so they should be remapped to point there. (my mistake, the terminology used here wasn't clear to me)

[jyn514]

cc @cbeuw

[cbeuw]

I see there are some terminology confusions. /rustc/<hash> are technically remapped as it was remapped from some real path during compiler bootstrapping. At compile time it's correlated back to rust-src path if it exists. I would call /rustc/<hash> "virtualised" path and the actual .../.rustup/toolchains/... path "real" or "local" path.

I'm not confident in saying this but I think this can be resolved within std::backtrace? We have a CFG_VIRTUAL_RUST_SOURCE_BASE_DIR containing the local path to the sources, and the backtrace code should be able to use it to map things back? This avoids emitting local paths into the binary which was what we tried to avoid.

[jyn514]

CFG_VIRTUAL_RUST_SOURCE_BASE_DIR

I haven't looked into the code, but that sounds like it's a compile time constant, so it has the path when rustc is compiled, not when rustc is running. So if you use that it will be unhelpful, it will show things like /home/bors/builds.

[cbeuw]

nvm CFG_VIRTUAL_RUST_SOURCE_BASE_DIR actually contains what the path is virtualised to (i.e. /rustc/<hash>), so it's not too useful here. (It may be used to identify if the path on hand is actually for the Rust source rather than a real path on local system coincidentally starting with /rustc/[a-z0-9]{40}/library/... but that'd be highly unlikely unless the user is deliberately trying to trip things up. In any case we could hardcode CFG_VIRTUAL_RUST_SOURCE_BASE_DIR into rustc during bootstrapping).

The bit that finds the local path to rust source is here

rust/compiler/rustc_session/src/config.rs

Lines 2077 to 2093 in 3e827cc

	let real_rust_source_base_dir = {
	// This is the location used by the `rust-src` `rustup` component.
	let mut candidate = sysroot.join("lib/rustlib/src/rust");
	if let Ok(metadata) = candidate.symlink_metadata() {
	// Replace the symlink rustbuild creates, with its destination.
	// We could try to use `fs::canonicalize` instead, but that might
	// produce unnecessarily verbose path.
	if metadata.file_type().is_symlink() {
	if let Ok(symlink_dest) = std::fs::read_link(&candidate) {
	candidate = symlink_dest;
	}
	}
	}
	// Only use this directory if it has a file we can expect to always find.
	if candidate.join("library/std/src/lib.rs").is_file() { Some(candidate) } else { None }
	};

The only environment-dependent thing we need here is sysroot on the local file system. Obviously we can't emit that into the binary so maybe we could ask Cargo to supply it?

[jyn514]

@cbeuw that has the same issue, cargo runs at compiletime, not runtime. There's no way for this to be done at runtime without it being part of the binary, which is why I suggested doing it only for test binaries.

[jyn514]

I guess as an alternative libtest could read a SYSROOT environment variable set by cargo, but that seems kind of hacky.

[cbeuw]

@jyn514 I'm in the middle of writing an RFC for Cargo that adds a profile setting called trim-path, which is basically an implementation of #40552 (sanitising every potential absolute paths, like paths to Cargo registry) and is enabled by default for release builds.

Prehaps we could make the emission of virtual vs real sysroot paths also dependant on that? It seems a logical thing to do if trim-path gets added. This way both test and debug builds will have real paths embedded by default, and can be overriden manually if the user wishes.

[cbeuw]

RFC's here: rust-lang/rfcs#3127