Make dbghelp look for PDBs next to their exe/dll.

michaelwoerister commented 6 months ago

As part of the trim-paths RFC we will need the Rust compiler to stop emitting absolute paths to the PDB file into binaries. But we still want backtrace-rs to find the PDB files for symbolication. This PR updates the debuginfo search path used by dbghelp to look in each directory that contains and executable or DLL currently loaded. As far as I can tell, this is the default behavior for debuggers on Windows (see for example here and here).

This should unblock fixing https://github.com/rust-lang/rust/issues/87825.

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 147,968 B
Difference: 10,240 B (7.43%)

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 147,968 B
Difference: 10,240 B (7.43%)

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 147,968 B
Difference: 10,240 B (7.43%)

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 170,496 B
Difference: 32,768 B (23.79%)

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 147,968 B
Difference: 10,240 B (7.43%)

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 147,968 B
Difference: 10,240 B (7.43%)

michaelwoerister commented 6 months ago

Hm, I do not know where this line is coming from on i686:

5: RtlConstructCrossVmEventPath

It seems like a random symbol, even though the other symbols from the same PDB seem to be resolved correctly.

bjorn3 commented 6 months ago

This would allow getting rid of https://github.com/rust-lang/rust-analyzer/blob/ac998a74b3c8ff4b81c3eeb9a18811d4cc76226d/crates/rust-analyzer/src/bin/main.rs#L99, right?

michaelwoerister commented 6 months ago

Yes, that case should be covered.

michaelwoerister commented 6 months ago

It looks to me like backtraces on i686-pc-windows-msvc are already flaky. Running the main branch tests on a local system also fails.

github-actions[bot] commented 6 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 140,288 B
Difference: 2,560 B (1.86%)

workingjubilee commented 5 months ago

Running the main branch tests on a local system also fails.

Hm, @michaelwoerister Can you elaborate on which tests fail?

michaelwoerister commented 5 months ago

When running cargo test --target i686-pc-windows-msvc, tests/accuracy/main.rs fails because one of the stack frames is resolved to the wrong symbol name.

When running tests locally with --target i686-pc-windows-msvc, the test fails reliably even with the main branch version of backtrace-rs, that is, the modifications in this PR are not needed for the test to fail. When ignoring that test then sp_smoke_test and many_threads in tests\smoke.rs also fail.

When debugging the test case in WinDbg, it also doesn't find the stack frames in question and it gives a warning that it can't properly unwind the stack:

0:004> k
ChildEBP RetAddr      
0109f438 00dc6f4a     accuracy_aaf57b0791de17ef!accuracy::verify+0x591 [D:\backtrace-rs\tests\accuracy\main.rs @ 112]
0109f5b0 776c04dd     accuracy_aaf57b0791de17ef!accuracy::inner+0xba [D:\backtrace-rs\tests\accuracy\main.rs @ 60]
WARNING: Stack unwind information not available. Following frames may be wrong.
0109f7ac 00dc6bc8     ntdll!RtlRemovePropertyStore+0x89d
0109f7bc 00dc2a4d     accuracy_aaf57b0791de17ef!accuracy::doit::closure$0+0x18 [D:\backtrace-rs\tests\accuracy\main.rs @ 18]
0109f7f0 00e3118b     accuracy_aaf57b0791de17ef!core::ops::function::FnOnce::call_once<accuracy::doit::closure_env$0,tuple$<> >+0x4d [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112\library\core\src\ops\function.rs @ 250]
(Inline) --------     accuracy_aaf57b0791de17ef!core::ops::function::FnOnce::call_once+0x3 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\core\src\ops\function.rs @ 250]
0109f808 00e29e2a     accuracy_aaf57b0791de17ef!test::__rust_begin_short_backtrace<enum2$<core::result::Result<tuple$<>,alloc::string::String> >,enum2$<core::result::Result<tuple$<>,alloc::string::String> > (*)()>+0xb [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\lib.rs @ 628]
0109f8e8 00e2ffdb     accuracy_aaf57b0791de17ef!test::types::RunnableTest::run+0x6a [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\types.rs @ 146]
(Inline) --------     accuracy_aaf57b0791de17ef!test::run_test_in_process::closure$0+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\lib.rs @ 651]
(Inline) --------     accuracy_aaf57b0791de17ef!core::panic::unwind_safe::impl$23::call_once+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\core\src\panic\unwind_safe.rs @ 272]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panicking::try::do_call+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panicking.rs @ 552]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panicking::try+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panicking.rs @ 516]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panic::catch_unwind+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panic.rs @ 142]
(Inline) --------     accuracy_aaf57b0791de17ef!test::run_test_in_process+0x59d [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\lib.rs @ 651]
0109fd70 00df3ae0     accuracy_aaf57b0791de17ef!test::run_test::closure$0+0x7db [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\lib.rs @ 574]
(Inline) --------     accuracy_aaf57b0791de17ef!test::run_test::closure$1+0x81 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\test\src\lib.rs @ 602]
0109fe74 00df9841     accuracy_aaf57b0791de17ef!std::sys_common::backtrace::__rust_begin_short_backtrace<test::run_test::closure_env$1,tuple$<> >+0xd0 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\sys_common\backtrace.rs @ 154]
(Inline) --------     accuracy_aaf57b0791de17ef!std::thread::impl$0::spawn_unchecked_::closure$1::closure$0+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\thread\mod.rs @ 529]
(Inline) --------     accuracy_aaf57b0791de17ef!core::panic::unwind_safe::impl$23::call_once+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\core\src\panic\unwind_safe.rs @ 272]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panicking::try::do_call+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panicking.rs @ 552]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panicking::try+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panicking.rs @ 516]
(Inline) --------     accuracy_aaf57b0791de17ef!std::panic::catch_unwind+0x17 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\panic.rs @ 142]
(Inline) --------     accuracy_aaf57b0791de17ef!std::thread::impl$0::spawn_unchecked_::closure$1+0x98 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\thread\mod.rs @ 528]
0109febc 00e55536     accuracy_aaf57b0791de17ef!core::ops::function::FnOnce::call_once<std::thread::impl$0::spawn_unchecked_::closure_env$1<test::run_test::closure_env$1,tuple$<> >,tuple$<> >+0xd1 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\core\src\ops\function.rs @ 250]
0109ff04 763b7ba9     accuracy_aaf57b0791de17ef!std::sys::windows::thread::impl$0::new::thread_start+0x66 [/rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library\std\src\sys\windows\thread.rs @ 58]
0109ff14 7769bd2b     KERNEL32!BaseThreadInitThunk+0x19
0109ff6c 7769bcaf     ntdll!RtlInitializeExceptionChain+0x6b
0109ff7c 00000000     ntdll!RtlClearBits+0xbf

When compiling everything with -Cforce-frame-pointers=yes, then all tests pass.

github-actions[bot] commented 5 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 137,728 B
Updated binary size: 140,800 B
Difference: 3,072 B (2.23%)

github-actions[bot] commented 5 months ago

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

Original binary size: 135,168 B
Updated binary size: 137,216 B
Difference: 2,048 B (1.52%)

michaelwoerister commented 5 months ago

I've added -Cforce-frame-pointers to the RUSTFLAGS of the new test case (https://github.com/rust-lang/backtrace-rs/pull/584/commits/b3a2a03cec2600e72d76b0d3f5b3b710bada4eb0). This makes the test pass reliably. I think that's an acceptable workaround. The PR shouldn't be blocked by an unrelated issue.

workingjubilee commented 5 months ago

I agree.

michaelwoerister commented 5 months ago

Thanks for the review, @workingjubilee!

rust-lang / backtrace-rs

Make dbghelp look for PDBs next to their exe/dll. #584