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

Backtraces don't work during ICE #71060

Closed
mati865 opened this issue Apr 12, 2020 · 25 comments · Fixed by #74682
Closed

Backtraces don't work during ICE #71060

mati865 opened this issue Apr 12, 2020 · 25 comments · Fixed by #74682
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mati865
Copy link
Contributor

mati865 commented Apr 12, 2020

Backtraces work fine when using panic!() inside crate but they don't when rustc ICEs.

Meta

Backtrace from building Rust with `debuginfo-level = 1`

error: internal compiler error: src\librustc_mir\monomorphize\collector.rs:606: collection encountered polymorphic constant
  --> C:\Users\mateusz\.cargo\registry\src\github.com-1ecc6299db9ec823\compiler_builtins-0.1.25\src\int\mod.rs:50:5
   |
50 |     const ONE: Self;
   |     ^^^^^^^^^^^^^^^^

thread 'rustc' panicked at 'Box<Any>', <::std::macros::panic macros>:2:4
stack backtrace:
   0:         0x67afae48 - _iob_func
   1:         0x67af99b0 - _iob_func
   2:         0x67b3968c - _iob_func
   3:         0x67aed175 - _iob_func
   4:         0x67b08fb4 - _iob_func
   5:         0x67b08c40 - _iob_func
   6:           0xe6f3eb - _iob_func
   7:         0x67b096e0 - _iob_func
   8:          0x1f8df75 - _iob_func
   9:          0x1eaea19 - _iob_func
  10:          0x1eaef52 - _iob_func
  11:          0x2292275 - _iob_func
  12:          0x229386f - _iob_func
  13:          0x2293817 - _iob_func
  14:          0x208c6f3 - _iob_func
  15:          0x208c2a9 - _iob_func
  16:          0x208a98f - _iob_func
  17:          0x208adff - _iob_func
  18:          0x1f722a4 - _iob_func
  19:          0x20894f2 - _iob_func
  20:          0x21298b6 - _iob_func
  21:          0x12411ae - _iob_func
  22:          0x12ba632 - _iob_func
  23:          0x12681f0 - _iob_func
  24:          0x138d820 - _iob_func
  25:          0x1316579 - _iob_func
  26:          0x11084b7 - _iob_func
  27:          0x1196334 - _iob_func
  28:          0x10cbc29 - _iob_func
  29:          0x11d6da7 - _iob_func
  30:           0xf7c788 - _iob_func
  31:           0xe7655c - _iob_func
  32:           0xe73391 - _iob_func
  33:           0xe80fc1 - _iob_func
  34:         0x67b090d7 - _iob_func
  35:           0xf801ae - _iob_func
  36:         0x67af2356 - _iob_func
  37:         0x67aea359 - _iob_func
  38:     0x7ffbf5f27bd4 - _iob_func
  39:     0x7ffbf650ced1 - _iob_func

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.44.0-dev running on x86_64-pc-windows-gnu

note: compiler flags: -Z macro-backtrace -Z save-analysis -Z binary-dep-depinfo -Z force-unstable-if-unmarked -C opt-level=3 -C debuginfo=1 -C linker=clang1 -C prefer-dynamic -C panic=abort -C debug-assertions=no --crate-type lib

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack

Backtrace from nightly

error: internal compiler error: src\librustc_mir_build\build\matches\test.rs:417: non_scalar_compare called on non-reference type: impl std::marker::Copy

thread 'rustc' panicked at 'Box<Any>', src\librustc_errors\lib.rs:880:9
stack backtrace:
   0:         0x6e4e7f9b - mingw_set_invalid_parameter_handler
   1:         0x6e51432b - mingw_set_invalid_parameter_handler
   2:         0x6e4d8d15 - mingw_set_invalid_parameter_handler
   3:         0x6e4ec7e1 - mingw_set_invalid_parameter_handler
   4:         0x6e4ec456 - mingw_set_invalid_parameter_handler
   5:         0x63cdedb5 - mingw_set_invalid_parameter_handler
   6:         0x6e4ece85 - mingw_set_invalid_parameter_handler
   7:         0x664647b5 - mingw_set_invalid_parameter_handler
   8:         0x664a18e2 - mingw_set_invalid_parameter_handler
   9:         0x664a0662 - mingw_set_invalid_parameter_handler
  10:         0x65e54e20 - mingw_set_invalid_parameter_handler
  11:         0x65e48d30 - mingw_set_invalid_parameter_handler
  12:         0x65e48cd3 - mingw_set_invalid_parameter_handler
  13:         0x65e54d1f - mingw_set_invalid_parameter_handler
  14:         0x65e54c7d - mingw_set_invalid_parameter_handler
  15:         0x64bc06dc - mingw_set_invalid_parameter_handler
  16:         0x64bc6d2e - mingw_set_invalid_parameter_handler
  17:         0x64bc3a91 - mingw_set_invalid_parameter_handler
  18:         0x64bc1055 - mingw_set_invalid_parameter_handler
  19:         0x64bb945f - mingw_set_invalid_parameter_handler
  20:         0x64bb9a5b - mingw_set_invalid_parameter_handler
  21:         0x64bab77f - mingw_set_invalid_parameter_handler
  22:         0x64bb926f - mingw_set_invalid_parameter_handler
  23:         0x64bb9a5b - mingw_set_invalid_parameter_handler
  24:         0x64bb9a5b - mingw_set_invalid_parameter_handler
  25:         0x64ba6f63 - mingw_set_invalid_parameter_handler
  26:         0x64b727a2 - mingw_set_invalid_parameter_handler
  27:         0x64ba5b80 - mingw_set_invalid_parameter_handler
  28:         0x64ce9818 - mingw_set_invalid_parameter_handler
  29:         0x64e29faf - mingw_set_invalid_parameter_handler
  30:         0x64d4bbbf - mingw_set_invalid_parameter_handler
  31:         0x651937d6 - mingw_set_invalid_parameter_handler
  32:         0x64ceb781 - mingw_set_invalid_parameter_handler
  33:         0x64e2d3e1 - mingw_set_invalid_parameter_handler
  34:         0x64d33c16 - mingw_set_invalid_parameter_handler
  35:         0x6508c4df - mingw_set_invalid_parameter_handler
  36:         0x64ce98e8 - mingw_set_invalid_parameter_handler
  37:         0x64e38cbf - mingw_set_invalid_parameter_handler
  38:         0x64d321ef - mingw_set_invalid_parameter_handler
  39:         0x6508cab0 - mingw_set_invalid_parameter_handler
  40:         0x64cea128 - mingw_set_invalid_parameter_handler
  41:         0x64e2eb0d - mingw_set_invalid_parameter_handler
  42:         0x64d27c13 - mingw_set_invalid_parameter_handler
  43:         0x650c81d4 - mingw_set_invalid_parameter_handler
  44:         0x64057538 - mingw_set_invalid_parameter_handler
  45:         0x64096cff - mingw_set_invalid_parameter_handler
  46:         0x6407236d - mingw_set_invalid_parameter_handler
  47:         0x64058af4 - mingw_set_invalid_parameter_handler
  48:         0x63fc1e60 - mingw_set_invalid_parameter_handler
  49:         0x64055395 - mingw_set_invalid_parameter_handler
  50:         0x63cf76ab - mingw_set_invalid_parameter_handler
  51:         0x63e2fd24 - mingw_set_invalid_parameter_handler
  52:         0x63cf9b41 - mingw_set_invalid_parameter_handler
  53:         0x63e669d6 - mingw_set_invalid_parameter_handler
  54:         0x63e3b4e0 - mingw_set_invalid_parameter_handler
  55:         0x63ce5a8c - mingw_set_invalid_parameter_handler
  56:         0x63e70701 - mingw_set_invalid_parameter_handler
  57:         0x63cea8b1 - mingw_set_invalid_parameter_handler
  58:         0x63e3d75a - mingw_set_invalid_parameter_handler
  59:         0x6e4c8b06 - mingw_set_invalid_parameter_handler
  60:         0x6e4fb7fc - mingw_set_invalid_parameter_handler
  61:     0x7ffbf5f27bd4 - mingw_set_invalid_parameter_handler
  62:     0x7ffbf650ced1 - mingw_set_invalid_parameter_handler

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.44.0-nightly (94d346360 2020-04-09) running on x86_64-pc-windows-gnu

query stack during panic:
#0 [mir_built] building MIR for
#1 [unsafety_check_result] unsafety-checking `main`
#2 [mir_const] processing `main`
#3 [mir_validated] processing `main`
#4 [mir_borrowck] borrow-checking `main`
#5 [analysis] running analysis passes on this crate
end of query stack
error: aborting due to previous error

@mati865 mati865 added the C-bug Category: This is a bug. label Apr 12, 2020
@mati865

This comment has been minimized.

@rustbot rustbot added the O-windows-gnu Toolchain: GNU, Operating system: Windows label Apr 12, 2020
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 12, 2020
@alexcrichton
Copy link
Member

My guess is that this is highly likely to be caused by rust-lang/backtrace-rs#333. Dynamic libraries are not found by backtrace-rs at this time and symbolicated at all. Currently I don't know how to iterate over loaded libraries and register what address ranges they occupy.

@mati865
Copy link
Contributor Author

mati865 commented Jun 9, 2020

@spastorino this might be something for wg-prioritization to talk about.

@spastorino
Copy link
Member

@mati865 do you want wg-prioritization to prioritize this issue? or do you want to nominate it for discussion on a T-compiler meeting?. Feel free to tag it with I-prioritize if this needs to be prioritized or with I-nominate if you want to nominate it for discussion.

@mati865
Copy link
Contributor Author

mati865 commented Jun 9, 2020

@spastorino I just want "increase visibility" of this issue hoping that somebody who knows about this stuff would step in. Sorry if that was "wrong address".

@spastorino
Copy link
Member

spastorino commented Jun 9, 2020

@mati865 not at all, it's not the wrong address, I'm trying to understand how can we help. If you consider this issue important and want to increase its visibility, you can nominate the issue for discussion on the next T-compiler meeting by labelling with I-nominate. If you do so, please explain why you're nominating it. The wg checks all nominations before the meeting and evaluate them in terms of the existing agenda.

@mati865
Copy link
Contributor Author

mati865 commented Jun 9, 2020

@spastorino thank you for the explanation, although I don't think I can use I-nominated label.

@rustbot modify labels: +I-nominated

Reason for nomination:
Rustc ICE backtraces are totally broken on windows-gnu (Tier 1) platform.
This not only makes life harder for people developing Rust using windows-gnu toolchain but also makes backtraces provided by the users completely useless (example here #73114).
Is there anybody who knows enough about Windows unwinding enough to assist here? Or in the other case could you announce something like "call for help"?

@rustbot
Copy link
Collaborator

rustbot commented Jun 9, 2020

Error: Label I-nominated can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

@spastorino
Copy link
Member

@rustbot ping windows

@rustbot
Copy link
Collaborator

rustbot commented Jun 10, 2020

Error: This team (windows) cannot be pinged via this command;it may need to be added to triagebot.toml on the master branch.

Please let @rust-lang/release know if you're having trouble with this bot.

@spastorino
Copy link
Member

Ohh right, there are still some pending PRs to be able to ping windows notify group. Doing so manually for now ...

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @retep998 @rylev @sivadeilra

@kennykerr
Copy link
Contributor

@manodasanW may be able to help. He is our resident debugging expert, but doesn't necessarily know anything about Rust. 🙂

@lzybkr
Copy link
Contributor

lzybkr commented Jun 10, 2020

@alexcrichton - Regarding checking which libraries are loaded - you can enumerate the memory regions and query the filenames for the region. Working code (modulo error handling and long paths):

use std::{ffi::OsString, mem::{size_of, MaybeUninit}, os::windows::ffi::OsStringExt, path::Path};

use winapi::{
    shared::{minwindef::{HMODULE, LPVOID}, ntdef::NULL},
    um::{
        libloaderapi::GetModuleFileNameW, memoryapi::VirtualQuery, sysinfoapi::GetSystemInfo,
        winnt::MEMORY_BASIC_INFORMATION,
    },
};

unsafe fn print_loaded_images() {
    let mut system_info = MaybeUninit::zeroed().assume_init();
    GetSystemInfo(&mut system_info);

    let mut buf: Vec<u16> = Vec::with_capacity(1000);
    let mut memory_basic_info = MaybeUninit::zeroed().assume_init();

    let start = system_info.lpMinimumApplicationAddress;
    let end = system_info.lpMaximumApplicationAddress;
    let mut base = start;
    while base < end {
        VirtualQuery(base, &mut memory_basic_info, size_of::<MEMORY_BASIC_INFORMATION>());
        if base != NULL && memory_basic_info.AllocationBase == base {
            let len = GetModuleFileNameW(base as HMODULE, buf.as_mut_ptr(), 1000) as usize;
            if len > 0 {
                buf.set_len(len);
                let filename = OsString::from_wide(&buf[..]);
                let path = Path::new(&filename);
                println!("base: {:x} module: {} ", base as usize, path.display());
            }
        }

        base = ((base as usize) + memory_basic_info.RegionSize) as LPVOID;
    }
}

fn main() {
    unsafe { print_loaded_images() };
}

And in your Cargo.toml, add:

[dependencies.winapi]
version = "0.3.8"
features = ["libloaderapi", "memoryapi", "sysinfoapi"]

@alexcrichton
Copy link
Member

Whoa that's perfect, thanks @lzybkr!

One question I'd have for that, how does the call to GetModuleFileNameW work there? How it it safe to cast an arbitrary memory address to an HMODULE?

In any case I can look to implement this at least for the gimli feature on the backtrace crate. That's unfortunately not a silver bullet for this issue because the compiler still uses libbacktrace, and I'm not personally up for modifying the C code of libbacktrace. I hope in the somewhat near future to switch over libstd though...

@alexcrichton
Copy link
Member

And another follow-up question. I've got an initial commit implementing this in the backtrace crate. I then whipped up a procedural macro which prints the backtrace using the backtrace crate to see if it works. (shows all <unknown> today, this patch is supposed to fix that).

I'm getting some odd behavior, though, and was wondering if other windows folks knew what might be going on. I'm reliably seeing a backtrace for everything in the procedural macro itself:

procedural macro's stack frames
   0:         0x67dfeb08 - backtrace::backtrace::dbghelp::trace::hd396a5624bee7ab4
                               at C:\msys64\home\alex\code\backtrace-rs/C:\msys64\home\alex\code\backtrace-rs\src\backtrace/dbghelp.rs:99
                           backtrace::backtrace::trace_unsynchronized::hf2dd82ace568fe9f
                               at C:\msys64\home\alex\code\backtrace-rs/C:\msys64\home\alex\code\backtrace-rs\src\backtrace/mod.rs:66
   1:         0x67dfe9c9 - backtrace::backtrace::trace::hde1671460dc308a4
                               at C:\msys64\home\alex\code\backtrace-rs/C:\msys64\home\alex\code\backtrace-rs\src\backtrace/mod.rs:53
   2:         0x67d8d5d8 - backtrace::capture::Backtrace::create::h1a8aa354e3fb0be1
                               at C:\msys64\home\alex\code\backtrace-rs/C:\msys64\home\alex\code\backtrace-rs\src/capture.rs:164
   3:         0x67d8d4c5 - backtrace::capture::Backtrace::new::ha19592734a1d7e0e
                               at C:\msys64\home\alex\code\backtrace-rs/C:\msys64\home\alex\code\backtrace-rs\src/capture.rs:128
   4:         0x67d8527c - pm::foo::h1e509f19291595a2
                               at C:\msys64\home\alex\code\wut/pm\src/lib.rs:3
   5:         0x67d83148 - core::ops::function::FnOnce::call_once::hc034a2591886bbd3
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libcore\ops/function.rs:232
   6:         0x67d82766 - proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>::expand1::run::{{closure}}::h412d0ed1ab20a412
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:394
   7:         0x67d81db4 - proc_macro::bridge::client::run_client::{{closure}}::{{closure}}::h240b46d3e60e1d66
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:362
   8:         0x67d85db8 - proc_macro::bridge::scoped_cell::ScopedCell<T>::set::{{closure}}::he8b02e8e643901e2
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/scoped_cell.rs:79
   9:         0x67d865f2 - proc_macro::bridge::scoped_cell::ScopedCell<T>::replace::h506f21e840707659
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/scoped_cell.rs:74
  10:         0x67d85d95 - proc_macro::bridge::scoped_cell::ScopedCell<T>::set::hc549006b80555ddd
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/scoped_cell.rs:79
  11:         0x67d8256c - proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter::{{closure}}::habdf288ae2ac6b64
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:310
  12:         0x67d82ee4 - std::thread::local::LocalKey<T>::try_with::hb7dcab13b0a81215
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd\thread/local.rs:263
  13:         0x67d82a0e - std::thread::local::LocalKey<T>::with::h6e17ed637ca840bf
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd\thread/local.rs:239
  14:         0x67d85636 - proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter::h5b16ebd94254838d
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:310
  15:         0x67d81c95 - proc_macro::bridge::client::run_client::{{closure}}::h5bb0f9a769e901a6
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:355
  16:         0x67d828a6 - <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h575dcb772ab8c60e
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd/panic.rs:318
  17:         0x67d86d90 - std::panicking::try::do_call::h6beb5bf073c74cf8
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd/panicking.rs:331
  18:         0x67d86e5d - __rust_try
  19:         0x67d86cba - std::panicking::try::hbb9d78f95702cc2f
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd/panicking.rs:274
  20:         0x67d828e6 - std::panic::catch_unwind::hcfc29a4c0e696d4f
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libstd/panic.rs:394
  21:         0x67d81a7f - proc_macro::bridge::client::run_client::hb6fa1f767ba54e5b
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:354
  22:         0x67d8271d - proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>::expand1::run::h0c73b131837ea539
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\libproc_macro\bridge/client.rs:394

What's odd though is that the compiler sometimes works but sometimes doesn't:

rest of backtrace, working
  23:         0x6a638cbd - proc_macro::bridge::server::<impl proc_macro::bridge::client::Client<fn(proc_macro::TokenStream) .> proc_macro::TokenStream>>::run::h8451c6e6d532dd65
  24:         0x6a59d9b5 - <rustc_expand::proc_macro::BangProcMacro as rustc_expand::base::ProcMacro>::expand::hc80692347382d719
  25:         0x6a6747c2 - rustc_expand::expand::MacroExpander::fully_expand_fragment::h67c40150e695d877
  26:         0x6a671be0 - rustc_expand::expand::MacroExpander::expand_crate::h46b9604bed1bde36
  27:         0x68ec8bdd - rustc_session::utils::<impl rustc_session::session::Session>::time::h5755693d665e9670
  28:         0x68f3f0a1 - rustc_interface::passes::configure_and_expand_inner::ha76e4262d31ad9f2
  29:         0x68ebe7a5 - rustc_interface::passes::configure_and_expand::{{closure}}::hc2f52cc5b7384658
  30:         0x68ea59df - rustc_data_structures::box_region::PinnedGenerator<I,A,R>::new::hf915f6fc867ca12d
  31:         0x68f3de83 - rustc_interface::passes::configure_and_expand::h9a61a4a61b282d50
  32:         0x68f94dda - rustc_interface::queries::Queries::expansion::h345f822d2112a3a9
  33:         0x68d3dcef - rustc_interface::interface::run_compiler_in_existing_thread_pool::h1a0541d00d7be99e
  34:         0x68be895c - scoped_tls::ScopedKey<T>::set::h101bd01f16771cfc
  35:         0x68be5741 - rustc_ast::attr::with_globals::hed19924516f613c2
  36:         0x68bf196d - std::sys_common::backtrace::__rust_begin_short_backtrace::h83a806688af07659
  37:         0x68d414da - core::ops::function::FnOnce::call_once{{vtable.shim}}::h92c4857f441fa433
  38:           0xd2b7f0 - <unknown>
  39:     0x7ff86fac7bd4 - <unknown>
  40:     0x7ff871a6ce51 - <unknown>
rest of backtrace, not working
  23:          0x2898cbd - <unknown>
  24:          0x27fd9b5 - <unknown>
  25:          0x28d47c2 - <unknown>
  26:          0x28d1be0 - <unknown>
  27:          0x1128bdd - <unknown>
  28:          0x119f0a1 - <unknown>
  29:          0x111e7a5 - <unknown>
  30:          0x11059df - <unknown>
  31:          0x119de83 - <unknown>
  32:          0x11f4dda - <unknown>
  33:           0xf9dcef - <unknown>
  34:           0xe4895c - <unknown>
  35:           0xe45741 - <unknown>
  36:           0xe5196d - <unknown>
  37:           0xfa14da - <unknown>
  38:         0x6e97b7f0 - <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::h3d2694c6b85e876c
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\liballoc/boxed.rs:1008
                           <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::h36cfd5eb012ef4dc
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\src\liballoc/boxed.rs:1008
                           std::sys::windows::thread::Thread::new::thread_start::h527df9e3fd88a21a
                               at /rustc/49cae55760da0a43428eba73abcb659bb70cf2e4\/src\libstd\sys\windows/thread.rs:56
  39:     0x7ff86fac7bd4 - <unknown>
  40:     0x7ff871a6ce51 - <unknown>

My guess here is that there's something to do with ASLR and/or some randomization of the address space. Currently that's not accounted for in the MinGW implementation of backtrace either.

In the "not working" backtrace, for example, the address 0x2898cbd (frame 23) is detected as being inside of rustc_driver-*.dll (correctly), but then that address fails lookup in the symbol table. I think the address that actually needs to be looked up is something like the successful backtrace (0x6a638cbd).

Is there a way to detect at runtime the "offset" of where a DLL is loaded? I'm basically trying to figure out the rough equivalent of _dyld_get_image_vmaddr_slide (macOS) or dlpi_addr (ELF) on Windows.

@kennykerr
Copy link
Contributor

kennykerr commented Jun 10, 2020

You can use GetModuleHandleW to get the base address of a DLL or GetModuleHandleExW if you want to find the DLL that a certain address is contained in. The HMODULE is just the base address of the module.

@lzybkr
Copy link
Contributor

lzybkr commented Jun 10, 2020

Regarding the cast - I don't think it's documented but seems unlikely to change.

After a bit of digging, I think the officially supported api (and maybe slightly more reliable in multi-threaded code) uses the toolhelp api - here is an example in C++: https://docs.microsoft.com/en-us/windows/win32/toolhelp/traversing-the-module-list (though the sample should probably use Module32FirstW and Module32NextW.

@mati865
Copy link
Contributor Author

mati865 commented Jun 10, 2020

@alexcrichton ASLR is still not enabled for windows-gnu: #16514
This can be confirmed by running dumpbin -headers "hello.exe" | findstr -i dynamic (Dynamic base means ASLR).
FTR DLL Characteristics in binaries compiled with Rust:

  • windows-msvc:
            8160 DLL characteristics
                   High Entropy Virtual Addresses
                   Dynamic base
                   NX compatible
                   Terminal Server Aware
  • windows-gnu:
             100 DLL characteristics
                   NX compatible

alexcrichton added a commit to rust-lang/backtrace-rs that referenced this issue Jun 10, 2020
This commit uses the information and APIs learned from
rust-lang/rust#71060 to implement support for dynamic libraries on
MinGW. Previously symbols only worked if they came from the main
executable, but now the process's list of dynamic libraries are also
searched so we can symbolicate, for example, symbols in the compiler
which come from loaded libraries rather than the main executable.
alexcrichton added a commit to rust-lang/backtrace-rs that referenced this issue Jun 10, 2020
This commit uses the information and APIs learned from
rust-lang/rust#71060 to implement support for dynamic libraries on
MinGW. Previously symbols only worked if they came from the main
executable, but now the process's list of dynamic libraries are also
searched so we can symbolicate, for example, symbols in the compiler
which come from loaded libraries rather than the main executable.
alexcrichton added a commit to rust-lang/backtrace-rs that referenced this issue Jun 10, 2020
This commit uses the information and APIs learned from
rust-lang/rust#71060 to implement support for dynamic libraries on
MinGW. Previously symbols only worked if they came from the main
executable, but now the process's list of dynamic libraries are also
searched so we can symbolicate, for example, symbols in the compiler
which come from loaded libraries rather than the main executable.
@alexcrichton
Copy link
Member

Ok I've whipped up rust-lang/backtrace-rs#345 which I believe works at least for the gimli-symbolize feature. This does not affect libstd and transitively means it, with no other changes, will continue to not help the compiler itself (e.g. ICEs). Fixing ICEs can be done a few different ways:

alexcrichton added a commit to rust-lang/backtrace-rs that referenced this issue Jun 10, 2020
This commit uses the information and APIs learned from
rust-lang/rust#71060 to implement support for dynamic libraries on
MinGW. Previously symbols only worked if they came from the main
executable, but now the process's list of dynamic libraries are also
searched so we can symbolicate, for example, symbols in the compiler
which come from loaded libraries rather than the main executable.
@mati865
Copy link
Contributor Author

mati865 commented Jun 10, 2020

Well, it turns out libbacktrace itself works fine for MinGW. Libgcc has a bug in _Unwind_GetIPInfo but LLVM's compiler-rt doesn't suffer from it: ianlancetaylor/libbacktrace#43 (comment)

I'll try running backtraces-rs testsuite with clang+compiler-rt.
EDIT: It doesn't help.

@spastorino
Copy link
Member

spastorino commented Jun 11, 2020

Removing nomination as it was discussed during today's meeting.

@pnkfelix
Copy link
Member

pnkfelix commented Jun 11, 2020

@alexcrichton wrote:

Ok I've whipped up rust-lang/backtrace-rs#345 which I believe works at least for the gimli-symbolize feature. This does not affect libstd and transitively means it, with no other changes, will continue to not help the compiler itself (e.g. ICEs). Fixing ICEs can be done a few different ways:

  • Write the same code for libbacktrace itself, and update that in the backtrace-sys crate.

  • Update the compiler to use the backtrace crate instead of the standard library for ICE backtraces

  • Push on rust-lang/backtrace-rs#328 to get libstd to use gimli, likely as outlined here

Discussed in today's T-compiler meeting

No one had a terribly strong opinion.

The team leads both agreed that for the short-term, if its easy to change the compiler to use the backtrace crate for ICE backtraces, then we should just do that as a crutch for now. That is, from our point-of-view, there's enough value in getting ICE backtraces working quickly, especially for somewhat niche targets like this, that we should just go ahead and do it.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 17, 2020
This commit is a proof-of-concept for switching the standard library's
backtrace symbolication mechanism on most platforms from libbacktrace to
gimli. The standard library's support for `RUST_BACKTRACE=1` requires
in-process parsing of object files and DWARF debug information to
interpret it and print the filename/line number of stack frames as part
of a backtrace.

Historically this support in the standard library has come from a
library called "libbacktrace". The libbacktrace library seems to have
been extracted from gcc at some point and is written in C. We've had a
lot of issues with libbacktrace over time, unfortunately, though. The
library does not appear to be actively maintained since we've had
patches sit for months-to-years without comments. We have discovered a
good number of soundness issues with the library itself, both when
parsing valid DWARF as well as invalid DWARF. This is enough of an issue
that the libs team has previously decided that we cannot feed untrusted
inputs to libbacktrace. This also doesn't take into account the
portability of libbacktrace which has been difficult to manage and
maintain over time. While possible there are lots of exceptions and it's
the main C dependency of the standard library right now.

For years it's been the desire to switch over to a Rust-based solution
for symbolicating backtraces. It's been assumed that we'll be using the
Gimli family of crates for this purpose, which are targeted at safely
and efficiently parsing DWARF debug information. I've been working
recently to shore up the Gimli support in the `backtrace` crate. As of a
few weeks ago the `backtrace` crate, by default, uses Gimli when loaded
from crates.io. This transition has gone well enough that I figured it
was time to start talking seriously about this change to the standard
library.

This commit is a preview of what's probably the best way to integrate
the `backtrace` crate into the standard library with the Gimli feature
turned on. While today it's used as a crates.io dependency, this commit
switches the `backtrace` crate to a submodule of this repository which
will need to be updated manually. This is not done lightly, but is
thought to be the best solution. The primary reason for this is that the
`backtrace` crate needs to do some pretty nontrivial filesystem
interactions to locate debug information. Working without `std::fs` is
not an option, and while it might be possible to do some sort of
trait-based solution when prototyped it was found to be too unergonomic.
Using a submodule allows the `backtrace` crate to build as a submodule
of the `std` crate itself, enabling it to use `std::fs` and such.

Otherwise this adds new dependencies to the standard library. This step
requires extra attention because this means that these crates are now
going to be included with all Rust programs by default. It's important
to note, however, that we're already shipping libbacktrace with all Rust
programs by default and it has a bunch of C code implementing all of
this internally anyway, so we're basically already switching
already-shipping functionality to Rust from C.

* `object` - this crate is used to parse object file headers and
  contents. Very low-level support is used from this crate and almost
  all of it is disabled. Largely we're just using struct definitions as
  well as convenience methods internally to read bytes and such.

* `addr2line` - this is the main meat of the implementation for
  symbolication. This crate depends on `gimli` for DWARF parsing and
  then provides interfaces needed by the `backtrace` crate to turn an
  address into a filename / line number. This crate is actually pretty
  small (fits in a single file almost!) and mirrors most of what
  `dwarf.c` does for libbacktrace.

* `miniz_oxide` - the libbacktrace crate transparently handles
  compressed debug information which is compressed with zlib. This crate
  is used to decompress compressed debug sections.

* `gimli` - not actually used directly, but a dependency of `addr2line`.

* `adler32`- not used directly either, but a dependency of
  `miniz_oxide`.

The goal of this change is to improve the safety of backtrace
symbolication in the standard library, especially in the face of
possibly malformed DWARF debug information. Even to this day we're still
seeing segfaults in libbacktrace which could possibly become security
vulnerabilities. This change should almost entirely eliminate this
possibility whilc also paving the way forward to adding more features
like split debug information.

Some references for those interested are:

* Original addition of libbacktrace - rust-lang#12602
* OOM with libbacktrace - rust-lang#24231
* Backtrace failure due to use of uninitialized value - rust-lang#28447
* Possibility to feed untrusted data to libbacktrace - rust-lang#21889
* Soundness fix for libbacktrace - rust-lang#33729
* Crash in libbacktrace - rust-lang#39468
* Support for macOS, never merged - ianlancetaylor/libbacktrace#2
* Performance issues with libbacktrace - rust-lang#29293, rust-lang#37477
* Update procedure is quite complicated due to how many patches we
  need to carry - rust-lang#50955
* Libbacktrace doesn't work on MinGW with dynamic libs - rust-lang#71060
* Segfault in libbacktrace on macOS - rust-lang#71397

Switching to Rust will not make us immune to all of these issues. The
crashes are expected to go away, but correctness and performance may
still have bugs arise. The gimli and `backtrace` crates, however, are
actively maintained unlike libbacktrace, so this should enable us to at
least efficiently apply fixes as situations come up.
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 18, 2020
…Simulacrum

std: Switch from libbacktrace to gimli

This commit is a proof-of-concept for switching the standard library's
backtrace symbolication mechanism on most platforms from libbacktrace to
gimli. The standard library's support for `RUST_BACKTRACE=1` requires
in-process parsing of object files and DWARF debug information to
interpret it and print the filename/line number of stack frames as part
of a backtrace.

Historically this support in the standard library has come from a
library called "libbacktrace". The libbacktrace library seems to have
been extracted from gcc at some point and is written in C. We've had a
lot of issues with libbacktrace over time, unfortunately, though. The
library does not appear to be actively maintained since we've had
patches sit for months-to-years without comments. We have discovered a
good number of soundness issues with the library itself, both when
parsing valid DWARF as well as invalid DWARF. This is enough of an issue
that the libs team has previously decided that we cannot feed untrusted
inputs to libbacktrace. This also doesn't take into account the
portability of libbacktrace which has been difficult to manage and
maintain over time. While possible there are lots of exceptions and it's
the main C dependency of the standard library right now.

For years it's been the desire to switch over to a Rust-based solution
for symbolicating backtraces. It's been assumed that we'll be using the
Gimli family of crates for this purpose, which are targeted at safely
and efficiently parsing DWARF debug information. I've been working
recently to shore up the Gimli support in the `backtrace` crate. As of a
few weeks ago the `backtrace` crate, by default, uses Gimli when loaded
from crates.io. This transition has gone well enough that I figured it
was time to start talking seriously about this change to the standard
library.

This commit is a preview of what's probably the best way to integrate
the `backtrace` crate into the standard library with the Gimli feature
turned on. While today it's used as a crates.io dependency, this commit
switches the `backtrace` crate to a submodule of this repository which
will need to be updated manually. This is not done lightly, but is
thought to be the best solution. The primary reason for this is that the
`backtrace` crate needs to do some pretty nontrivial filesystem
interactions to locate debug information. Working without `std::fs` is
not an option, and while it might be possible to do some sort of
trait-based solution when prototyped it was found to be too unergonomic.
Using a submodule allows the `backtrace` crate to build as a submodule
of the `std` crate itself, enabling it to use `std::fs` and such.

Otherwise this adds new dependencies to the standard library. This step
requires extra attention because this means that these crates are now
going to be included with all Rust programs by default. It's important
to note, however, that we're already shipping libbacktrace with all Rust
programs by default and it has a bunch of C code implementing all of
this internally anyway, so we're basically already switching
already-shipping functionality to Rust from C.

* `object` - this crate is used to parse object file headers and
  contents. Very low-level support is used from this crate and almost
  all of it is disabled. Largely we're just using struct definitions as
  well as convenience methods internally to read bytes and such.

* `addr2line` - this is the main meat of the implementation for
  symbolication. This crate depends on `gimli` for DWARF parsing and
  then provides interfaces needed by the `backtrace` crate to turn an
  address into a filename / line number. This crate is actually pretty
  small (fits in a single file almost!) and mirrors most of what
  `dwarf.c` does for libbacktrace.

* `miniz_oxide` - the libbacktrace crate transparently handles
  compressed debug information which is compressed with zlib. This crate
  is used to decompress compressed debug sections.

* `gimli` - not actually used directly, but a dependency of `addr2line`.

* `adler32`- not used directly either, but a dependency of
  `miniz_oxide`.

The goal of this change is to improve the safety of backtrace
symbolication in the standard library, especially in the face of
possibly malformed DWARF debug information. Even to this day we're still
seeing segfaults in libbacktrace which could possibly become security
vulnerabilities. This change should almost entirely eliminate this
possibility whilc also paving the way forward to adding more features
like split debug information.

Some references for those interested are:

* Original addition of libbacktrace - rust-lang#12602
* OOM with libbacktrace - rust-lang#24231
* Backtrace failure due to use of uninitialized value - rust-lang#28447
* Possibility to feed untrusted data to libbacktrace - rust-lang#21889
* Soundness fix for libbacktrace - rust-lang#33729
* Crash in libbacktrace - rust-lang#39468
* Support for macOS, never merged - ianlancetaylor/libbacktrace#2
* Performance issues with libbacktrace - rust-lang#29293, rust-lang#37477
* Update procedure is quite complicated due to how many patches we
  need to carry - rust-lang#50955
* Libbacktrace doesn't work on MinGW with dynamic libs - rust-lang#71060
* Segfault in libbacktrace on macOS - rust-lang#71397

Switching to Rust will not make us immune to all of these issues. The
crashes are expected to go away, but correctness and performance may
still have bugs arise. The gimli and `backtrace` crates, however, are
actively maintained unlike libbacktrace, so this should enable us to at
least efficiently apply fixes as situations come up.

---

I want to note that my purpose for creating a PR here is to start a conversation about this. I think that all the various pieces are in place that this is compelling enough that I think this transition should be talked about seriously. There are a number of items which still need to be addressed before actually merging this PR, however:

* [ ] `gimli` needs to be published to crates.io
* [ ] `addr2line` needs a publish
* [ ] `miniz_oxide` needs a publish
* [ ] Tests probably shouldn't recommend the `gimli` crate's traits for implementing
* [ ] The `backtrace` crate's branch changes need to be merged to the master branch (rust-lang/backtrace-rs#349)
* [ ] The support for `libbacktrace` on some platforms needs to be audited to see if we should support more strategies in the gimli implementation - rust-lang/backtrace-rs#325, rust-lang/backtrace-rs#326, rust-lang/backtrace-rs#350, rust-lang/backtrace-rs#351

Most of the merging/publishing I'm not actively pushing on right now. It's a bit wonky for crates to support libstd so I'm holding off on pulling the trigger everywhere until there's a bit more discussion about how to go through with this. Namely rust-lang/backtrace-rs#349 I'm going to hold off merging until we decide to go through with the submodule strategy.

In any case this is a pretty major change, so I suspect that the compiler team is likely going to be interested in this. I don't mean to force changes by dumping a bunch of code by any means. Integration of external crates into the standard library is so difficult I wanted to have a proof-of-concept to review while talking about whether to do this at all (hence the PR), but I'm more than happy to follow any processes needed to merge this. I must admit though that I'm not entirely sure myself at this time what the process would be to decide to merge this, so I'm hoping others can help me figure that out!
@mati865
Copy link
Contributor Author

mati865 commented Jul 21, 2020

Example of beautiful ICE backtrace on current nightly with gimli 🚀:

thread 'rustc' panicked at 'Box<Any>', /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\src\libstd\macros.rs:13:23
stack backtrace:
   0: std::backtrace_rs::backtrace::dbghelp::trace
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\..\backtrace\src\backtrace/dbghelp.rs:98
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\..\backtrace\src\backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\sys_common/backtrace.rs:77
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\sys_common/backtrace.rs:58
   4: core::fmt::write
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libcore\fmt/mod.rs:1117
   5: std::io::Write::write_fmt
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\io/mod.rs:1508
   6: std::sys_common::backtrace::_print
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\sys_common/backtrace.rs:61
   7: std::sys_common::backtrace::print
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd\sys_common/backtrace.rs:48
   8: std::panicking::default_hook::{{closure}}
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd/panicking.rs:198
   9: std::panicking::default_hook
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd/panicking.rs:217
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at /rustc/f9a3086363f214f2b56bef30f0ac572e1a9127f1\/src\libstd/panicking.rs:530
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::span_bug
  14: rustc_errors::Handler::span_bug
  15: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc_middle::ty::context::tls::with_opt::{{closure}}
  17: rustc_middle::ty::context::tls::with_opt
  18: rustc_middle::util::bug::opt_span_bug_fmt
  19: rustc_middle::util::bug::span_bug_fmt
  20: rustc_typeck::check::FnCtxt::local_ty::{{closure}}
  21: rustc_typeck::check::FnCtxt::local_ty
  22: rustc_typeck::check::FnCtxt::instantiate_value_path
  23: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  24: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  25: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  26: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  27: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  28: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  29: <smallvec::SmallVec<A> as core::iter::traits::collect::Extend<<A as smallvec::Array>::Item>>::extend
  30: <T as rustc_middle::ty::context::InternIteratorElement<T,R>>::intern_with
  31: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  32: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  33: rustc_typeck::check::_match::<impl rustc_typeck::check::FnCtxt>::check_match
  34: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  35: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  36: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  37: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  38: rustc_typeck::check::FnCtxt::check_argument_types
  39: rustc_typeck::check::callee::<impl rustc_typeck::check::FnCtxt>::confirm_builtin_call
  40: rustc_typeck::check::callee::<impl rustc_typeck::check::FnCtxt>::check_call
  41: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  42: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  43: rustc_typeck::check::FnCtxt::check_argument_types
  44: rustc_typeck::check::callee::<impl rustc_typeck::check::FnCtxt>::confirm_builtin_call
  45: rustc_typeck::check::callee::<impl rustc_typeck::check::FnCtxt>::check_call
  46: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  47: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  48: rustc_typeck::check::FnCtxt::check_stmt
  49: rustc_typeck::check::FnCtxt::check_block_with_expected
  50: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  51: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  52: rustc_typeck::check::_match::<impl rustc_typeck::check::FnCtxt>::check_match
  53: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  54: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  55: rustc_typeck::check::FnCtxt::check_stmt
  56: rustc_typeck::check::FnCtxt::check_block_with_expected
  57: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_kind
  58: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_expr_with_expectation
  59: rustc_typeck::check::expr::<impl rustc_typeck::check::FnCtxt>::check_return_expr
  60: rustc_typeck::check::check_fn
  61: rustc_infer::infer::InferCtxtBuilder::enter
  62: rustc_typeck::check::typeck
  63: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck>::compute
  64: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  65: rustc_query_system::query::plumbing::get_query_impl
  66: rustc_query_system::query::plumbing::ensure_query_impl
  67: rustc_typeck::check::typeck_item_bodies
  68: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck_item_bodies>::compute
  69: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  70: rustc_query_system::query::plumbing::get_query_impl
  71: rustc_typeck::check_crate
  72: rustc_interface::passes::analysis
  73: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  74: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  75: rustc_data_structures::stack::ensure_sufficient_stack
  76: rustc_query_system::query::plumbing::get_query_impl
  77: rustc_middle::ty::context::tls::enter_global
  78: rustc_interface::interface::create_compiler_and_run
  79: scoped_tls::ScopedKey<T>::set
  80: rustc_ast::attr::with_session_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

@alexcrichton should we keep this issue open considering #73441 will be probably reverted and relanded?

@alexcrichton
Copy link
Member

I think let's go ahead and close and if that's backed out we can reopen.

@alexcrichton
Copy link
Member

Ok PR ended up being backed out.

@alexcrichton alexcrichton reopened this Jul 23, 2020
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 28, 2020
This commit is a proof-of-concept for switching the standard library's
backtrace symbolication mechanism on most platforms from libbacktrace to
gimli. The standard library's support for `RUST_BACKTRACE=1` requires
in-process parsing of object files and DWARF debug information to
interpret it and print the filename/line number of stack frames as part
of a backtrace.

Historically this support in the standard library has come from a
library called "libbacktrace". The libbacktrace library seems to have
been extracted from gcc at some point and is written in C. We've had a
lot of issues with libbacktrace over time, unfortunately, though. The
library does not appear to be actively maintained since we've had
patches sit for months-to-years without comments. We have discovered a
good number of soundness issues with the library itself, both when
parsing valid DWARF as well as invalid DWARF. This is enough of an issue
that the libs team has previously decided that we cannot feed untrusted
inputs to libbacktrace. This also doesn't take into account the
portability of libbacktrace which has been difficult to manage and
maintain over time. While possible there are lots of exceptions and it's
the main C dependency of the standard library right now.

For years it's been the desire to switch over to a Rust-based solution
for symbolicating backtraces. It's been assumed that we'll be using the
Gimli family of crates for this purpose, which are targeted at safely
and efficiently parsing DWARF debug information. I've been working
recently to shore up the Gimli support in the `backtrace` crate. As of a
few weeks ago the `backtrace` crate, by default, uses Gimli when loaded
from crates.io. This transition has gone well enough that I figured it
was time to start talking seriously about this change to the standard
library.

This commit is a preview of what's probably the best way to integrate
the `backtrace` crate into the standard library with the Gimli feature
turned on. While today it's used as a crates.io dependency, this commit
switches the `backtrace` crate to a submodule of this repository which
will need to be updated manually. This is not done lightly, but is
thought to be the best solution. The primary reason for this is that the
`backtrace` crate needs to do some pretty nontrivial filesystem
interactions to locate debug information. Working without `std::fs` is
not an option, and while it might be possible to do some sort of
trait-based solution when prototyped it was found to be too unergonomic.
Using a submodule allows the `backtrace` crate to build as a submodule
of the `std` crate itself, enabling it to use `std::fs` and such.

Otherwise this adds new dependencies to the standard library. This step
requires extra attention because this means that these crates are now
going to be included with all Rust programs by default. It's important
to note, however, that we're already shipping libbacktrace with all Rust
programs by default and it has a bunch of C code implementing all of
this internally anyway, so we're basically already switching
already-shipping functionality to Rust from C.

* `object` - this crate is used to parse object file headers and
  contents. Very low-level support is used from this crate and almost
  all of it is disabled. Largely we're just using struct definitions as
  well as convenience methods internally to read bytes and such.

* `addr2line` - this is the main meat of the implementation for
  symbolication. This crate depends on `gimli` for DWARF parsing and
  then provides interfaces needed by the `backtrace` crate to turn an
  address into a filename / line number. This crate is actually pretty
  small (fits in a single file almost!) and mirrors most of what
  `dwarf.c` does for libbacktrace.

* `miniz_oxide` - the libbacktrace crate transparently handles
  compressed debug information which is compressed with zlib. This crate
  is used to decompress compressed debug sections.

* `gimli` - not actually used directly, but a dependency of `addr2line`.

* `adler32`- not used directly either, but a dependency of
  `miniz_oxide`.

The goal of this change is to improve the safety of backtrace
symbolication in the standard library, especially in the face of
possibly malformed DWARF debug information. Even to this day we're still
seeing segfaults in libbacktrace which could possibly become security
vulnerabilities. This change should almost entirely eliminate this
possibility whilc also paving the way forward to adding more features
like split debug information.

Some references for those interested are:

* Original addition of libbacktrace - rust-lang#12602
* OOM with libbacktrace - rust-lang#24231
* Backtrace failure due to use of uninitialized value - rust-lang#28447
* Possibility to feed untrusted data to libbacktrace - rust-lang#21889
* Soundness fix for libbacktrace - rust-lang#33729
* Crash in libbacktrace - rust-lang#39468
* Support for macOS, never merged - ianlancetaylor/libbacktrace#2
* Performance issues with libbacktrace - rust-lang#29293, rust-lang#37477
* Update procedure is quite complicated due to how many patches we
  need to carry - rust-lang#50955
* Libbacktrace doesn't work on MinGW with dynamic libs - rust-lang#71060
* Segfault in libbacktrace on macOS - rust-lang#71397

Switching to Rust will not make us immune to all of these issues. The
crashes are expected to go away, but correctness and performance may
still have bugs arise. The gimli and `backtrace` crates, however, are
actively maintained unlike libbacktrace, so this should enable us to at
least efficiently apply fixes as situations come up.
@bors bors closed this as completed in c058a8b Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-windows-gnu Toolchain: GNU, Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants