rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.32k stars 12.58k forks source link

--remap-path-prefix doesn't map paths to `.pdb` files, even in release mode #87825

Closed SkiFire13 closed 6 months ago

SkiFire13 commented 3 years ago

Spawned off of https://github.com/rust-lang/rust/issues/87805#issuecomment-894202194

When passing --remap-path-prefix C:\\path\\to\\project=nothing_to_see_here to rustc I would expect all paths that start with C:\path\to\project to be replaced with nothing_to_see_here, instead a C:\path\to\project\target\release\deps\crate_name.pdb is still present.

Note that this happens even in release mode, because rustc always tells the msvc linker to generate .pdb files.

This can be "fixed" by passing either -Clink-arg=/PDBALTPATH:%_PDB%, -Clink-arg=/DEBUG:NONE or -Zstrip=debuginfo to rustc, but doing so will break backtraces with RUST_BACKTRACE='full' (RUST_BACKTRACE=1 already prints almost nothing in release mode). As a sidenote, do we really need backtraces in release mode, and by consequence .pdb files? They will break anyway if only the .exe is distributed or the target directory is deleted.

Moreover this shouldn't be needed in the first place since --remap-path-prefix promises to "remap source names in all output (compiler messages and output files)" (from the output of rustc --help -v), "including [...] debug information" (from the rustc book here). Also note that neither rustc --help -v nor the rustc book mention the need to pass one of those arguments to rustc to remap/remove the path to the .pdb file, which makes it harder for non-experts to know how to remove them.

rustc --version --verbose:

rustc 1.54.0 (a178d0322 2021-07-26)
binary: rustc
commit-hash: a178d0322ce20e33eac124758e837cbd80a6f633
commit-date: 2021-07-26
host: x86_64-pc-windows-msvc
release: 1.54.0
LLVM version: 12.0.1
apiraino commented 3 years ago

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium

danakj commented 3 years ago

Moreover this shouldn't be needed in the first place since --remap-path-prefix promises to "remap source names in all output (compiler messages and output files)" (from the output of rustc --help -v), "including [...] debug information" (from the rustc book here).

Perhaps this should be amended to say compiler output but not linker outputs. PDB files are generated at the linking step, which is probably why https://github.com/rust-lang/rust/issues/88982 sees non-determinism for --crate-type=bin (linked) and not for --crate-type=rlib (not linked).

Passing /pdbaltpath:%_PDB% to the linker to put a relative path to the PDB files is probably the best choice, as it is the least destructive, and the debugger will look for PDB files next to the executables, so it will be able to find it. Chromium uses this for its Windows linking step as well: https://source.chromium.org/chromium/chromium/src/+/main:build/config/compiler/BUILD.gn;drc=21dcb365c1eae5e99eb9ae6ef78bfe3c9372057e;l=2358

Doing this with --remap-path-prefix could work if you can figure out which "to-be-remapped" path is going to be the path being written by the linker, and then pass the resulting path to /pdbaltpath. I must confess I am not sure how linking is being done, but I assume the binary is being linked by a dedicated linker, not by rustc itself? If so that is not too reliable.

rustc could provide a switch to remove the absolute path to PDB files, which would just map to -Clink-arg=/pdbaltpath:%_PDB%. The value proposition there seems low, since you can pass that as the user anyway.

Determinism is important for both release and debug builds, so I don't believe there is anything release-mode specific here or that a solution should be chosen that doesn't deal with debug mode?

but doing so will break backtraces with RUST_BACKTRACE='full'

Could you say what is breaking about backtraces with -Clink-arg=/pdbaltpath:%_PDB%?

SkiFire13 commented 3 years ago

Could you say what is breaking about backtraces with -Clink-arg=/pdbaltpath:%_PDB%?

Without that flag I get something like:

Backtrace

``` thread 'main' panicked at 'foo', src\main.rs:2:5 stack backtrace: 0: 0x7ff6258483af - std::backtrace_rs::backtrace::dbghelp::trace at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\..\..\backtrace\src\backtrace\dbghelp.rs:98 1: 0x7ff6258483af - std::backtrace_rs::backtrace::trace_unsynchronized at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66 2: 0x7ff6258483af - std::sys_common::backtrace::_print_fmt at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\sys_common\backtrace.rs:67 3: 0x7ff6258483af - std::sys_common::backtrace::_print::impl$0::fmt at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\sys_common\backtrace.rs:46 4: 0x7ff625855e7a - core::fmt::write at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\core\src\fmt\mod.rs:1115 5: 0x7ff6258464d8 - std::io::Write::write_fmt at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\io\mod.rs:1665 6: 0x7ff62584abe6 - std::sys_common::backtrace::_print at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\sys_common\backtrace.rs:49 7: 0x7ff62584abe6 - std::sys_common::backtrace::print at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\sys_common\backtrace.rs:36 8: 0x7ff62584abe6 - std::panicking::default_hook::closure$1 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:208 9: 0x7ff62584a6c9 - std::panicking::default_hook at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:225 10: 0x7ff62584b245 - std::panicking::rust_panic_with_hook at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:622 11: 0x7ff62584208c - std::panicking::begin_panic::closure$0 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\panicking.rs:542 12: 0x7ff62584249f - std::sys_common::backtrace::__rust_end_short_backtrace at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\sys_common\backtrace.rs:141 13: 0x7ff625841faf - std::panicking::begin_panic at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\panicking.rs:541 14: 0x7ff62584152c - playground::foo at G:\WorkSpace\rust-playground\src\main.rs:2 15: 0x7ff625841539 - playground::main at G:\WorkSpace\rust-playground\src\main.rs:6 16: 0x7ff6258423bb - core::ops::function::FnOnce::call_once > at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\core\src\ops\function.rs:227 17: 0x7ff6258424cb - std::sys_common::backtrace::__rust_begin_short_backtrace > at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\sys_common\backtrace.rs:125 18: 0x7ff625841251 - std::rt::lang_start::closure$0 > at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\rt.rs:63 19: 0x7ff62584b666 - core::ops::function::impls::impl$2::call_once at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\core\src\ops\function.rs:259 20: 0x7ff62584b666 - std::panicking::try::do_call at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:401 21: 0x7ff62584b666 - std::panicking::try at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:365 22: 0x7ff62584b666 - std::panic::catch_unwind at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panic.rs:434 23: 0x7ff62584b666 - std::rt::lang_start_internal::closure$2 at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\rt.rs:45 24: 0x7ff62584b666 - std::panicking::try::do_call at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:401 25: 0x7ff62584b666 - std::panicking::try at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panicking.rs:365 26: 0x7ff62584b666 - std::panic::catch_unwind at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\panic.rs:434 27: 0x7ff62584b666 - std::rt::lang_start_internal at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\/library\std\src\rt.rs:45 28: 0x7ff62584121f - std::rt::lang_start > at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b\library\std\src\rt.rs:62 29: 0x7ff625841556 - main 30: 0x7ff6258596b8 - invoke_main at d:\A01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78 31: 0x7ff6258596b8 - __scrt_common_main_seh at d:\A01\_work\12\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 32: 0x7ff99c1d7034 - BaseThreadInitThunk 33: 0x7ff99d262651 - RtlUserThreadStart ```

But with it it just prints:

Backtrace

``` thread 'main' panicked at 'foo', src\main.rs:2:5 stack backtrace: 0: 0x7ff689dd83af - 1: 0x7ff689de5e7a - 2: 0x7ff689dd64d8 - 3: 0x7ff689ddabe6 - 4: 0x7ff689dda6c9 - 5: 0x7ff689ddb245 - 6: 0x7ff689dd208c - 7: 0x7ff689dd249f - 8: 0x7ff689dd1faf - 9: 0x7ff689dd152c - 10: 0x7ff689dd1539 - 11: 0x7ff689dd23bb - 12: 0x7ff689dd24cb - 13: 0x7ff689dd1251 - 14: 0x7ff689ddb666 - 15: 0x7ff689dd121f - 16: 0x7ff689dd1556 - 17: 0x7ff689de96b8 - 18: 0x7ff99c1d7034 - BaseThreadInitThunk 19: 0x7ff99d262651 - RtlUserThreadStart ```

This seems to happen because the pdb file is searched relative to the current working directory, and when running cargo run that's often not the directory with the executable and pdb files. -Clink-args=/pdbaltpath:target/release/%_PDB% appears to work, but only as long as you execute it from the crate root, which is definitely not ideal.

danakj commented 3 years ago

Can cargo run move to the crate root before executing the target executable? Then cargo could pass -Clink-args=/pdbaltpath:target/release/%_PDB% (or debug) while other build systems pass -Clink-arg=/pdbaltpath:%_PDB% or whatever works for them?

SkiFire13 commented 3 years ago

Wouldn't that break any workflow that relies on the current working directory always being where cargo run is executed from?

danakj commented 3 years ago

Wouldn't that break any workflow that relies on the current working directory always being where cargo run is executed from?

I think this meant the crate root instead of the cwd? I had no idea that would be a common thing, but you're right. It would change any use of std::env::current_dir(). I would expect std::env::current_exe() to be more commonly used for relative paths, as it doesn't depend on running from a cargo source tree, but maybe I am wrong.

Regardless, I can see why changing run behaviour isn't ideal. The MSVC linker doesn't present a whole lot of options though, as the path relative to where you plan to run the binary has to be encoded into the pdb files if you want a deterministic output.

It seems the options are:

  1. Non-determinisic output, cargo run backtraces always work, debuggers work: PDB absolute paths. This is the default behaviour.
  2. Deterministic output, debuggers work, and cargo run backtraces work from the exe's directory: -Clink-arg=/pdbaltpath:%_PDB%
  3. Deterministic output, cargo run backtraces work from the crate root, but debuggers won't work: -Clink-args=/pdbaltpath:target/release/%_PDB%

Have I missed an option?

There's no option that I can see with both deterministic output and backtraces working from arbitrary working directories.

AIUI, this github issue is about --remap-path-prefix not adding a -Clink-args flag, essentially. It would be nice if deterministic builds were easier to get! But based on all of the above, it seems to me that is better solved through Cargo than through rustc's --remap-path-prefix, perhaps behind a Cargo flag in https://github.com/rust-lang/rfcs/pull/3127.

As a convenience, maybe cargo run could provide an option to move to the exe's directory to run it, instead of users doing (cd target/release && cargo run), which would let option 2 be more ergonomic while also supporting debuggers.

SkiFire13 commented 3 years ago

Would it be possible to have cargo run create a symbolic link to the pdb file or the target directory when it's not already present in the current working directory? This would partially solve the ergonomic problems of approaches 2 and 3

Kobata commented 2 years ago

It'd probably be easier to have cargo run set either _NT_SYMBOL_PATH or _NT_ALTERNATE_SYMBOL_PATH (_NT_ALT_SYMBOL_PATH? they seem to spell it both ways), which effect the default search path for the windows APIs used for backtraces

Boscop commented 2 years ago

Any update on this? :)

michaelwoerister commented 7 months ago

I've just opened https://github.com/rust-lang/rust/pull/121297 which solves the problem by making rustc always pass /PDBALTPATH:%_PDB% to the linker. This seems like the best solution to me. The PR is currently blocked on backtrace-rs not handling this properly, but I've opened https://github.com/rust-lang/backtrace-rs/pull/584, which will hopefully fix that.