From b60954757eb1acd4069ddbc28a51f4a5bb7d42c9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 10:23:10 +0200 Subject: [PATCH 1/3] std: always depend on backtrace, but only enable its features on demand --- src/bootstrap/lib.rs | 2 +- src/libstd/Cargo.toml | 18 +++++++++--------- src/libstd/build.rs | 2 +- src/libstd/panicking.rs | 6 +++--- src/libstd/rt.rs | 4 ++-- src/libstd/sys_common/backtrace.rs | 5 ++++- src/libstd/sys_common/mod.rs | 1 - src/libstd/thread/mod.rs | 4 ++-- src/libtest/Cargo.toml | 2 +- 9 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 5d7581c8211be..e4f1b773a690d 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -490,7 +490,7 @@ impl Build { features.push_str(" llvm-libunwind"); } if self.config.backtrace { - features.push_str(" backtrace"); + features.push_str(" backtrace_support"); } if self.config.profiler { features.push_str(" profiler"); diff --git a/src/libstd/Cargo.toml b/src/libstd/Cargo.toml index 20442abc58890..2da73c11907de 100644 --- a/src/libstd/Cargo.toml +++ b/src/libstd/Cargo.toml @@ -27,15 +27,8 @@ hashbrown = { version = "0.5.0", features = ['rustc-dep-of-std'] } [dependencies.backtrace] version = "0.3.37" -default-features = false # don't use coresymbolication on OSX -features = [ - "rustc-dep-of-std", # enable build support for integrating into libstd - "dbghelp", # backtrace/symbolize on MSVC - "libbacktrace", # symbolize on most platforms - "libunwind", # backtrace on most platforms - "dladdr", # symbolize on platforms w/o libbacktrace -] -optional = true +default-features = false # without the libstd `backtrace` feature, stub out everything +features = [ "rustc-dep-of-std" ] # enable build support for integrating into libstd [dev-dependencies] rand = "0.7" @@ -65,6 +58,13 @@ cc = "1.0" [features] default = ["std_detect_file_io", "std_detect_dlsym_getauxval"] +backtrace_support = [ + "backtrace/dbghelp", # backtrace/symbolize on MSVC + "backtrace/libbacktrace", # symbolize on most platforms + "backtrace/libunwind", # backtrace on most platforms + "backtrace/dladdr", # symbolize on platforms w/o libbacktrace +] + panic-unwind = ["panic_unwind"] profiler = ["profiler_builtins"] compiler-builtins-c = ["alloc/compiler-builtins-c"] diff --git a/src/libstd/build.rs b/src/libstd/build.rs index 8db7bc12cd308..3209332eeb622 100644 --- a/src/libstd/build.rs +++ b/src/libstd/build.rs @@ -49,7 +49,7 @@ fn main() { println!("cargo:rustc-link-lib=zircon"); println!("cargo:rustc-link-lib=fdio"); } else if target.contains("cloudabi") { - if cfg!(feature = "backtrace") { + if cfg!(feature = "backtrace_support") { println!("cargo:rustc-link-lib=unwind"); } println!("cargo:rustc-link-lib=c"); diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index db4089c294812..93a17d6eea54e 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -157,12 +157,12 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] use crate::sys_common::{backtrace as backtrace_mod}; // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let log_backtrace = { let panics = update_panic_count(0); @@ -190,7 +190,7 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] { use crate::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index cf45eb0daba39..f73bd6c6e74fa 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -44,11 +44,11 @@ fn lang_start_internal(main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindS sys::args::init(argc, argv); // Let's run some code! - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let exit_code = panic::catch_unwind(|| { sys_common::backtrace::__rust_begin_short_backtrace(move || main()) }); - #[cfg(not(feature = "backtrace"))] + #[cfg(not(feature = "backtrace_support"))] let exit_code = panic::catch_unwind(move || main()); sys_common::cleanup(); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 1a78abf508620..52890668c35ee 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -7,7 +7,6 @@ use crate::io; use crate::borrow::Cow; use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; -use crate::sync::atomic::{self, Ordering}; use crate::sys::mutex::Mutex; use backtrace::{BacktraceFmt, BytesOrWideString, PrintFmt}; @@ -34,6 +33,7 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. +#[cfg(feature = "backtrace_support")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -129,7 +129,10 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. +#[cfg(feature = "backtrace_support")] pub fn log_enabled() -> Option { + use crate::sync::atomic::{self, Ordering}; + // Setting environment variables for Fuchsia components isn't a standard // or easily supported workflow. For now, always display backtraces. if cfg!(target_os = "fuchsia") { diff --git a/src/libstd/sys_common/mod.rs b/src/libstd/sys_common/mod.rs index 9190a3b0d5fc7..cba3eca538625 100644 --- a/src/libstd/sys_common/mod.rs +++ b/src/libstd/sys_common/mod.rs @@ -41,7 +41,6 @@ macro_rules! rtunwrap { pub mod alloc; pub mod at_exit_imp; -#[cfg(feature = "backtrace")] pub mod backtrace; pub mod condvar; pub mod io; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 764041d2f4239..85fd80e16afb3 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -465,11 +465,11 @@ impl Builder { } thread_info::set(imp::guard::current(), their_thread); - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) })); - #[cfg(not(feature = "backtrace"))] + #[cfg(not(feature = "backtrace_support"))] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(f)); *their_packet.get() = Some(try_result); }; diff --git a/src/libtest/Cargo.toml b/src/libtest/Cargo.toml index 170fbb984cf9b..f0041bcf67cb0 100644 --- a/src/libtest/Cargo.toml +++ b/src/libtest/Cargo.toml @@ -23,7 +23,7 @@ proc_macro = { path = "../libproc_macro" } # Forward features to the `std` crate as necessary [features] -backtrace = ["std/backtrace"] +backtrace_support = ["std/backtrace_support"] compiler-builtins-c = ["std/compiler-builtins-c"] llvm-libunwind = ["std/llvm-libunwind"] panic-unwind = ["std/panic_unwind"] From dac0a158eb78419e3593fa888f9cf1ab81153030 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 12:12:32 +0200 Subject: [PATCH 2/3] rename the crate, not the feature --- src/bootstrap/lib.rs | 2 +- src/libstd/Cargo.toml | 13 +++++++------ src/libstd/backtrace.rs | 1 + src/libstd/build.rs | 2 +- src/libstd/panicking.rs | 8 ++++---- src/libstd/rt.rs | 4 ++-- src/libstd/sys_common/backtrace.rs | 10 +++++----- src/libstd/thread/mod.rs | 4 ++-- src/libtest/Cargo.toml | 2 +- 9 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index e4f1b773a690d..5d7581c8211be 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -490,7 +490,7 @@ impl Build { features.push_str(" llvm-libunwind"); } if self.config.backtrace { - features.push_str(" backtrace_support"); + features.push_str(" backtrace"); } if self.config.profiler { features.push_str(" profiler"); diff --git a/src/libstd/Cargo.toml b/src/libstd/Cargo.toml index 2da73c11907de..af1d2402f88e7 100644 --- a/src/libstd/Cargo.toml +++ b/src/libstd/Cargo.toml @@ -25,7 +25,8 @@ profiler_builtins = { path = "../libprofiler_builtins", optional = true } unwind = { path = "../libunwind" } hashbrown = { version = "0.5.0", features = ['rustc-dep-of-std'] } -[dependencies.backtrace] +[dependencies.backtrace_rs] +package = "backtrace" version = "0.3.37" default-features = false # without the libstd `backtrace` feature, stub out everything features = [ "rustc-dep-of-std" ] # enable build support for integrating into libstd @@ -58,11 +59,11 @@ cc = "1.0" [features] default = ["std_detect_file_io", "std_detect_dlsym_getauxval"] -backtrace_support = [ - "backtrace/dbghelp", # backtrace/symbolize on MSVC - "backtrace/libbacktrace", # symbolize on most platforms - "backtrace/libunwind", # backtrace on most platforms - "backtrace/dladdr", # symbolize on platforms w/o libbacktrace +backtrace = [ + "backtrace_rs/dbghelp", # backtrace/symbolize on MSVC + "backtrace_rs/libbacktrace", # symbolize on most platforms + "backtrace_rs/libunwind", # backtrace on most platforms + "backtrace_rs/dladdr", # symbolize on platforms w/o libbacktrace ] panic-unwind = ["panic_unwind"] diff --git a/src/libstd/backtrace.rs b/src/libstd/backtrace.rs index 5d46ef7dbb10a..61c42a56071e6 100644 --- a/src/libstd/backtrace.rs +++ b/src/libstd/backtrace.rs @@ -97,6 +97,7 @@ use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sync::Mutex; use crate::sys_common::backtrace::{output_filename, lock}; use crate::vec::Vec; +use backtrace_rs as backtrace; use backtrace::BytesOrWideString; /// A captured OS thread stack backtrace. diff --git a/src/libstd/build.rs b/src/libstd/build.rs index 3209332eeb622..8db7bc12cd308 100644 --- a/src/libstd/build.rs +++ b/src/libstd/build.rs @@ -49,7 +49,7 @@ fn main() { println!("cargo:rustc-link-lib=zircon"); println!("cargo:rustc-link-lib=fdio"); } else if target.contains("cloudabi") { - if cfg!(feature = "backtrace_support") { + if cfg!(feature = "backtrace") { println!("cargo:rustc-link-lib=unwind"); } println!("cargo:rustc-link-lib=c"); diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 93a17d6eea54e..e7755af7f07da 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -157,17 +157,17 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] use crate::sys_common::{backtrace as backtrace_mod}; // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let log_backtrace = { let panics = update_panic_count(0); if panics >= 2 { - Some(backtrace::PrintFmt::Full) + Some(backtrace_rs::PrintFmt::Full) } else { backtrace_mod::log_enabled() } @@ -190,7 +190,7 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] { use crate::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index f73bd6c6e74fa..cf45eb0daba39 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -44,11 +44,11 @@ fn lang_start_internal(main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindS sys::args::init(argc, argv); // Let's run some code! - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let exit_code = panic::catch_unwind(|| { sys_common::backtrace::__rust_begin_short_backtrace(move || main()) }); - #[cfg(not(feature = "backtrace_support"))] + #[cfg(not(feature = "backtrace"))] let exit_code = panic::catch_unwind(move || main()); sys_common::cleanup(); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 52890668c35ee..f49adc01659ff 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -9,7 +9,7 @@ use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; use crate::sys::mutex::Mutex; -use backtrace::{BacktraceFmt, BytesOrWideString, PrintFmt}; +use backtrace_rs::{BacktraceFmt, BytesOrWideString, PrintFmt}; /// Max number of frames to print. const MAX_NB_FRAMES: usize = 100; @@ -33,7 +33,7 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. -#[cfg(feature = "backtrace_support")] +#[cfg(feature = "backtrace")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -74,14 +74,14 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt:: bt_fmt.add_context()?; let mut idx = 0; let mut res = Ok(()); - backtrace::trace_unsynchronized(|frame| { + backtrace_rs::trace_unsynchronized(|frame| { if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES { return false; } let mut hit = false; let mut stop = false; - backtrace::resolve_frame_unsynchronized(frame, |symbol| { + backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| { hit = true; if print_fmt == PrintFmt::Short { if let Some(sym) = symbol.name().and_then(|s| s.as_str()) { @@ -129,7 +129,7 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. -#[cfg(feature = "backtrace_support")] +#[cfg(feature = "backtrace")] pub fn log_enabled() -> Option { use crate::sync::atomic::{self, Ordering}; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 85fd80e16afb3..764041d2f4239 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -465,11 +465,11 @@ impl Builder { } thread_info::set(imp::guard::current(), their_thread); - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) })); - #[cfg(not(feature = "backtrace_support"))] + #[cfg(not(feature = "backtrace"))] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(f)); *their_packet.get() = Some(try_result); }; diff --git a/src/libtest/Cargo.toml b/src/libtest/Cargo.toml index f0041bcf67cb0..170fbb984cf9b 100644 --- a/src/libtest/Cargo.toml +++ b/src/libtest/Cargo.toml @@ -23,7 +23,7 @@ proc_macro = { path = "../libproc_macro" } # Forward features to the `std` crate as necessary [features] -backtrace_support = ["std/backtrace_support"] +backtrace = ["std/backtrace"] compiler-builtins-c = ["std/compiler-builtins-c"] llvm-libunwind = ["std/llvm-libunwind"] panic-unwind = ["std/panic_unwind"] From 49854c4f71eb8470c2a4483cbad3f03eb99e67cb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 16:37:44 +0200 Subject: [PATCH 3/3] avoid #[cfg] in favor of cfg! --- src/libstd/panicking.rs | 18 +++++++----------- src/libstd/sys_common/backtrace.rs | 2 -- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index e7755af7f07da..28fb40244043e 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -17,8 +17,7 @@ use crate::ptr; use crate::raw; use crate::sys::stdio::panic_output; use crate::sys_common::rwlock::RWLock; -use crate::sys_common::thread_info; -use crate::sys_common::util; +use crate::sys_common::{thread_info, util, backtrace}; use crate::thread; #[cfg(not(test))] @@ -157,20 +156,18 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace")] - use crate::sys_common::{backtrace as backtrace_mod}; - // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace")] - let log_backtrace = { + let log_backtrace = if cfg!(feature = "backtrace") { let panics = update_panic_count(0); if panics >= 2 { Some(backtrace_rs::PrintFmt::Full) } else { - backtrace_mod::log_enabled() + backtrace::log_enabled() } + } else { + None }; // The current implementation always returns `Some`. @@ -190,14 +187,13 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace")] - { + if cfg!(feature = "backtrace") { use crate::sync::atomic::{AtomicBool, Ordering}; static FIRST_PANIC: AtomicBool = AtomicBool::new(true); if let Some(format) = log_backtrace { - let _ = backtrace_mod::print(err, format); + let _ = backtrace::print(err, format); } else if FIRST_PANIC.compare_and_swap(true, false, Ordering::SeqCst) { let _ = writeln!(err, "note: run with `RUST_BACKTRACE=1` \ environment variable to display a backtrace."); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index f49adc01659ff..01711d415d86c 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -33,7 +33,6 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. -#[cfg(feature = "backtrace")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -129,7 +128,6 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. -#[cfg(feature = "backtrace")] pub fn log_enabled() -> Option { use crate::sync::atomic::{self, Ordering};