remap libstd paths in backtraces for test crates #85463

jonas-schievink · 2021-05-19T10:02:05Z

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.

The text was updated successfully, but these errors were encountered:

jyn514 · 2021-05-19T10:15:44Z

@jonas-schievink what version of rustc is this?

jonas-schievink · 2021-05-19T10:16:53Z

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

jyn514 · 2021-05-19T10:20:23Z

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

jonas-schievink · 2021-05-19T10:23:09Z

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 · 2021-05-19T10:24:38Z

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

jyn514 · 2021-05-19T10:27:26Z

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

jyn514 · 2021-05-19T10:31:04Z

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

jonas-schievink · 2021-05-19T10:35:35Z

~~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 · 2021-05-19T10:40:08Z

cc @cbeuw

cbeuw · 2021-05-19T10:56:49Z

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 · 2021-05-19T13:29:09Z

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 · 2021-05-19T14:27:21Z

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 · 2021-05-19T19:24:48Z

@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 · 2021-05-19T19:25:34Z

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

cbeuw · 2021-05-19T19:38:20Z

@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 · 2021-05-25T12:04:53Z

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

Implement RFC 3127 sysroot path handling changes Fix rust-lang#105907 Fix rust-lang#85463 Implement parts of rust-lang#111540 Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g. ``` thread 'main' panicked at 'hello world', map-panic.rs:2:50 stack backtrace: 0: std::panicking::begin_panic at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12 1: map_panic::main::{{closure}} at ./map-panic.rs:2:50 2: core::option::Option<T>::map at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29 3: map_panic::main at ./map-panic.rs:2:30 4: core::ops::function::FnOnce::call_once at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ``` [RFC 3127 said](https://rust-lang.github.io/rfcs/3127-trim-paths.html#changing-handling-of-sysroot-path-in-rustc) > We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way. This PR implements this behaviour. When `rust-src` is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply `--remap-path-prefix=<path to rust-src>=foo`.

Implement RFC3137 trim-paths sysroot changes - take 2 This PR is a continuation of rust-lang#118149. Nothing really changed, except for rust-lang#129408 which I was able to trigger locally. Original description: > Implement parts of rust-lang#111540 > > Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g. > > ``` > thread 'main' panicked at 'hello world', map-panic.rs:2:50 > stack backtrace: > 0: std::panicking::begin_panic > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12 > 1: map_panic::main::{{closure}} > at ./map-panic.rs:2:50 > 2: core::option::Option<T>::map > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29 > 3: map_panic::main > at ./map-panic.rs:2:30 > 4: core::ops::function::FnOnce::call_once > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5 > note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. > ``` > > [RFC 3127 said](https://rust-lang.github.io/rfcs/3127-trim-paths.html#changing-handling-of-sysroot-path-in-rustc) > > > We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way. > > This PR implements this behaviour. When `rust-src` is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply `--remap-path-prefix=<path to rust-src>=foo`. cc `@cbeuw` Fix rust-lang#105907 Fix rust-lang#85463 try-job: dist-x86_64-linux try-job: x86_64-msvc try-job: armhf-gnu

Implement RFC3137 trim-paths sysroot changes - take 2 This PR is a continuation of rust-lang#118149. Nothing really changed, except for rust-lang#129408 which I was able to trigger locally. Original description: > Implement parts of rust-lang#111540 > > Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g. > > ``` > thread 'main' panicked at 'hello world', map-panic.rs:2:50 > stack backtrace: > 0: std::panicking::begin_panic > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12 > 1: map_panic::main::{{closure}} > at ./map-panic.rs:2:50 > 2: core::option::Option<T>::map > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29 > 3: map_panic::main > at ./map-panic.rs:2:30 > 4: core::ops::function::FnOnce::call_once > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5 > note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. > ``` > > [RFC 3127 said](https://rust-lang.github.io/rfcs/3127-trim-paths.html#changing-handling-of-sysroot-path-in-rustc) > > > We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way. > > This PR implements this behaviour. When `rust-src` is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply `--remap-path-prefix=<path to rust-src>=foo`. cc ``@cbeuw`` Fix rust-lang#105907 Fix rust-lang#85463 try-job: dist-x86_64-linux try-job: x86_64-msvc try-job: armhf-gnu

Implement RFC3137 trim-paths sysroot changes - take 2 This PR is a continuation of rust-lang#118149. Nothing really changed, except for rust-lang#129408 which I was able to trigger locally. Original description: > Implement parts of rust-lang#111540 > > Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g. > > ``` > thread 'main' panicked at 'hello world', map-panic.rs:2:50 > stack backtrace: > 0: std::panicking::begin_panic > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12 > 1: map_panic::main::{{closure}} > at ./map-panic.rs:2:50 > 2: core::option::Option<T>::map > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29 > 3: map_panic::main > at ./map-panic.rs:2:30 > 4: core::ops::function::FnOnce::call_once > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5 > note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. > ``` > > [RFC 3127 said](https://rust-lang.github.io/rfcs/3127-trim-paths.html#changing-handling-of-sysroot-path-in-rustc) > > > We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way. > > This PR implements this behaviour. When `rust-src` is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply `--remap-path-prefix=<path to rust-src>=foo`. cc ```@cbeuw``` Fix rust-lang#105907 Fix rust-lang#85463 try-job: dist-x86_64-linux try-job: x86_64-msvc try-job: armhf-gnu

Implement RFC3137 trim-paths sysroot changes - take 2 This PR is a continuation of rust-lang#118149. Nothing really changed, except for rust-lang#129408 which I was able to trigger locally. Original description: > Implement parts of rust-lang#111540 > > Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g. > > ``` > thread 'main' panicked at 'hello world', map-panic.rs:2:50 > stack backtrace: > 0: std::panicking::begin_panic > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12 > 1: map_panic::main::{{closure}} > at ./map-panic.rs:2:50 > 2: core::option::Option<T>::map > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29 > 3: map_panic::main > at ./map-panic.rs:2:30 > 4: core::ops::function::FnOnce::call_once > at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5 > note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. > ``` > > [RFC 3127 said](https://rust-lang.github.io/rfcs/3127-trim-paths.html#changing-handling-of-sysroot-path-in-rustc) > > > We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way. > > This PR implements this behaviour. When `rust-src` is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply `--remap-path-prefix=<path to rust-src>=foo`. cc `@cbeuw` Fix rust-lang#105907 Fix rust-lang#85463 try-job: dist-x86_64-linux try-job: x86_64-msvc try-job: dist-x86_64-msvc try-job: armhf-gnu

jonas-schievink added the C-bug Category: This is a bug. label May 19, 2021

jyn514 changed the title ~~Path remapping in backtraces is broken~~ Don't remap libstd paths in backtraces for test crates May 19, 2021

jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels May 19, 2021

jyn514 added the A-reproducibility Area: Reproducible / deterministic builds label May 19, 2021

jyn514 changed the title ~~Don't remap libstd paths in backtraces for test crates~~ remap libstd paths in backtraces for test crates May 19, 2021

cbeuw mentioned this issue Aug 20, 2021

--remap-path-prefix no longer works for compiler messages #87745

Closed

cbeuw mentioned this issue May 26, 2022

New rustc and Cargo options to allow path sanitisation by default rust-lang/rfcs#3127

Merged

ehuss mentioned this issue Dec 20, 2022

/rustc/$hash prefix is not being mapped when expected #105907

Closed

cbeuw mentioned this issue Nov 21, 2023

Implement RFC 3127 sysroot path handling changes #118149

Closed

Urgau mentioned this issue Aug 29, 2024

Implement RFC3137 trim-paths sysroot changes - take 2 #129687

Merged

bors closed this as completed in 1d9162b Sep 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remap libstd paths in backtraces for test crates #85463

remap libstd paths in backtraces for test crates #85463

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021 •

edited

Loading

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

jonas-schievink commented May 19, 2021 •

edited

Loading

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021 •

edited

Loading

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021

cbeuw commented May 25, 2021

remap libstd paths in backtraces for test crates #85463

remap libstd paths in backtraces for test crates #85463

Comments

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021 • edited Loading

jonas-schievink commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

jonas-schievink commented May 19, 2021 • edited Loading

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021 • edited Loading

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021

jyn514 commented May 19, 2021

jyn514 commented May 19, 2021

cbeuw commented May 19, 2021

cbeuw commented May 25, 2021

jyn514 commented May 19, 2021 •

edited

Loading

jonas-schievink commented May 19, 2021 •

edited

Loading

cbeuw commented May 19, 2021 •

edited

Loading