Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS symbolication failure #72550

Closed
ehuss opened this issue May 24, 2020 · 7 comments · Fixed by rust-lang/cargo#8329
Closed

macOS symbolication failure #72550

ehuss opened this issue May 24, 2020 · 7 comments · Fixed by rust-lang/cargo#8329
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-bug Category: This is a bug. O-macos Operating system: macOS P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor

ehuss commented May 24, 2020

I tried this code:

fn main() {
    panic!();
}

starting with cdb50c6 (nightly-2020-03-26, 1.44), when run with:

RUST_BACKTRACE=1 cargo run

results in:

   Compiling foo v0.1.0 (/Users/eric/Proj/rust/cargo/scratch/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.34s
     Running `target/debug/foo`
thread 'main' panicked at 'explicit panic', src/main.rs:2:5
stack backtrace:
   0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: std::panicking::default_hook::{{closure}}
   4: std::panicking::default_hook
   5: std::panicking::rust_panic_with_hook
   6: std::panicking::begin_panic
   7: foo::main
   8: std::rt::lang_start::{{closure}}
   9: std::rt::lang_start_internal
  10: std::rt::lang_start
  11: main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

when run with the previous commit, I correctly get the line numbers:

   Compiling foo v0.1.0 (/Users/eric/Proj/rust/cargo/scratch/foo)
    Finished dev [unoptimized + debuginfo] target(s) in 0.61s
     Running `target/debug/foo`
thread 'main' panicked at 'explicit panic', src/main.rs:2:5
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.45/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.45/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1069
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1439
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:218
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:511
  11: std::panicking::begin_panic
  12: foo::main
             at src/main.rs:2
  13: std::rt::lang_start::{{closure}}
             at /rustc/02046a5d402c789c006d0da7662f800fe3c45faf/src/libstd/rt.rs:67
  14: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  15: std::panicking::try::do_call
             at src/libstd/panicking.rs:331
  16: std::panicking::try
             at src/libstd/panicking.rs:274
  17: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  18: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  19: std::rt::lang_start
             at /rustc/02046a5d402c789c006d0da7662f800fe3c45faf/src/libstd/rt.rs:67
  20: foo::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

What's interesting, is if you directly run the executable with the metadata hash (like RUST_BACKTRACE=1 target/debug/deps/foo-f5700d28f7af5060), the backtrace looks correct. I tried adjusting the dSYM symlink to include the hash, but that didn't seem to help.

I would assume this is due to #70361. cc @tmiasko @alexcrichton

If the filename mismatch doesn't ring any bells, I can maybe try to dig in and see what changed. Although the changes to backtrace-sys look pretty huge, and I'm unfamiliar with it.

@ehuss ehuss added the C-bug Category: This is a bug. label May 24, 2020
@jonas-schievink jonas-schievink added A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows T-libs Relevant to the library team, which will review and decide on the PR/issue. O-macos Operating system: macOS regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels May 24, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 24, 2020
@JohnTitor JohnTitor added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 25, 2020
@JohnTitor
Copy link
Member

@tmiasko
Copy link
Contributor

tmiasko commented May 25, 2020

After reviewing changes to libbacktrace: the new implementation of Mach-O makes
no attempt to handle the case where an executable is renamed. The previous one
was more robust in that respect.

@alexcrichton
Copy link
Member

This is almost surely caused by rust-lang/backtrace-rs#299 which was updated to handle fixes like ianlancetaylor/libbacktrace#32.

TBH I can't really keep track of the bugginess and bugfixes in libbacktrace any more. I'm totally lost in what's happening over time. I'm hoping that rust-lang/backtrace-rs#328 can put the nail in this coffin.

@ehuss
Copy link
Contributor Author

ehuss commented May 26, 2020

@tmiasko Have you narrowed it down to a specific commit or change? Do you have a sense of how difficult it would be to fix?

I ask because an alternative would be to change Cargo so that it does not include the hash in the filename for macOS. It does this already for Windows for similar reasons (Windows embeds the path of the PDB in the executable which causes problems if it has a hash). It's only a minor loss (such as switching between features can cause rebuilds).

It sounds like switching to gimli would be great, but probably not going to happen soon?

I'm uncertain which approach would be the best approach.

@tmiasko
Copy link
Contributor

tmiasko commented May 26, 2020

@ehuss this upgrade switched from Mach-O implementation contributed by John
Calanduoni, but never successfully upstreamed, to a completely new one now
available upstream.

@ehuss
Copy link
Contributor Author

ehuss commented Jun 3, 2020

@alexcrichton The issue is that the filename inside the dSYM directory includes the hash (target/debug/foo.dSYM/Contents/Resources/DWARF/foo-64db4e4bf99c12dd). The old implementation would use readdir to scan for a .dSYM directory, and then use readdir again to try to load anything inside the DWARF directory.

The new implementation does not use readdir, and instead expects to find a file matching the executable name. But since the filename inside .dSYM is foo-64db4e4bf99c12dd, it doesn't find a file named foo.

Does that seem worth porting the old logic? I'm reluctant to do so, primarily because I'd also like to fix this for 1.45 (far too late for 1.44), and there's only 6 weeks till the next release.

I tried with gimli (I assume that's the default for backtrace with no features set?), and it seems to work, though I didn't dig into the source for where that is handled.

I don't really have a sense of how annoying hashless executables are (I don't use Windows often enough to have experience with that). Another option is that Cargo could put each executable inside a different directory (target/debug/deps/bin-foo-64db4e4bf99c12dd/foo). So I'm still a bit uncertain which is the best approach.

@alexcrichton
Copy link
Member

Ah ok that definitely makes sense. I think it's best to go with a Cargo fix for this rather than a backtrace one. This would involve editing libbacktrace, which I personally have very little confidence in doing. FWIW the dsym loading for gimli is here.

We should probably update gimli to at least only have one level of readdir, not two, but that's a side issue. I think it's fine to avoid hashes on more platforms for Cargo. I agree as well that we can fix this with separate output directories if it becomes a problem.

ehuss pushed a commit to ehuss/cargo that referenced this issue Jun 5, 2020
Don't hash executable filenames on apple platforms.

Due to some recent changes to the backtrace crate, backtraces on apple platforms haven't been working (they are missing line/filename information). The reason is that previously libbacktrace would hunt through the directory for any matching file in the `.dSYM` directory. The new implementation expects a file matching the executable name exactly (which no longer includes the hash because Cargo renames it).

The solution here is to not include a hash in the executable filename. This matches the behavior on Windows which does it for a similar reason (paths are embedded in pdb files).

The downside is that switching between different settings (like different features) causes Cargo to rebuild the binary each time.  I don't think this is a particularly common use case, at least I've not heard any complaints about this behavior on Windows.

Fixes rust-lang/rust#72550
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-runtime Area: std's runtime and "pre-main" init for handling backtraces, unwinds, stack overflows C-bug Category: This is a bug. O-macos Operating system: macOS P-medium Medium priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants