diff options
| author | bors <bors@rust-lang.org> | 2022-02-02 22:03:23 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-02-02 22:03:23 +0000 |
| commit | b3800860e123443ffada615538926beed6bc4f85 (patch) | |
| tree | 4c666bc883ba940757f1f63ee612d8f8edc5bfc2 | |
| parent | 27f5d830eb11cd7bdc834d6f0d78120976f75443 (diff) | |
| parent | 85930c8f444e1ece1c92a0f9e39814f72d867e43 (diff) | |
| download | rust-b3800860e123443ffada615538926beed6bc4f85.tar.gz rust-b3800860e123443ffada615538926beed6bc4f85.zip | |
Auto merge of #93101 - Mark-Simulacrum:library-backtrace, r=yaahc
Support configuring whether to capture backtraces at runtime
Tracking issue: https://github.com/rust-lang/rust/issues/93346
This adds a new API to the `std::panic` module which configures whether and how the default panic hook will emit a backtrace when a panic occurs.
After discussion with `@yaahc` on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/backtrace.20lib.20vs.2E.20panic), this PR chooses to avoid adjusting or seeking to provide a similar API for the (currently unstable) std::backtrace API. It seems likely that the users of that API may wish to expose more specific settings rather than just a global one (e.g., emulating the `env_logger`, `tracing` per-module configuration) to avoid the cost of capture in hot code. The API added here could plausibly be copied and/or re-exported directly from std::backtrace relatively easily, but I don't think that's the right call as of now.
```rust
mod panic {
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[non_exhaustive]
pub enum BacktraceStyle {
Short,
Full,
Off,
}
fn set_backtrace_style(BacktraceStyle);
fn get_backtrace_style() -> Option<BacktraceStyle>;
}
```
Several unresolved questions:
* Do we need to move to a thread-local or otherwise more customizable strategy for whether to capture backtraces? See [this comment](https://github.com/rust-lang/rust/pull/79085#issuecomment-727845826) for some potential use cases for this.
* Proposed answer: no, leave this for third-party hooks.
* Bikeshed on naming of all the options, as usual.
* Should BacktraceStyle be moved into `std::backtrace`?
* It's already somewhat annoying to import and/or re-type the `std::panic::` prefix necessary to use these APIs, probably adding a second module to the mix isn't worth it.
Note that PR #79085 proposed a much simpler API, but particularly in light of the desire to fully replace setting environment variables via `env::set_var` to control the backtrace API, a more complete API seems preferable. This PR likely subsumes that one.
| -rw-r--r-- | library/std/src/panic.rs | 114 | ||||
| -rw-r--r-- | library/std/src/panicking.rs | 23 | ||||
| -rw-r--r-- | library/std/src/sys_common/backtrace.rs | 57 | ||||
| -rw-r--r-- | src/test/ui/panics/runtime-switch.legacy.run.stderr | 5 | ||||
| -rw-r--r-- | src/test/ui/panics/runtime-switch.rs | 25 | ||||
| -rw-r--r-- | src/test/ui/panics/runtime-switch.v0.run.stderr | 5 | ||||
| -rw-r--r-- | src/tools/tidy/src/pal.rs | 1 |
7 files changed, 165 insertions, 65 deletions
diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs index 02ecf2e3e82..ac16f476143 100644 --- a/library/std/src/panic.rs +++ b/library/std/src/panic.rs @@ -5,6 +5,7 @@ use crate::any::Any; use crate::collections; use crate::panicking; +use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sync::{Mutex, RwLock}; use crate::thread::Result; @@ -202,5 +203,118 @@ pub fn always_abort() { crate::panicking::panic_count::set_always_abort(); } +/// The configuration for whether and how the default panic hook will capture +/// and display the backtrace. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[unstable(feature = "panic_backtrace_config", issue = "93346")] +#[non_exhaustive] +pub enum BacktraceStyle { + /// Prints a terser backtrace which ideally only contains relevant + /// information. + Short, + /// Prints a backtrace with all possible information. + Full, + /// Disable collecting and displaying backtraces. + Off, +} + +impl BacktraceStyle { + pub(crate) fn full() -> Option<Self> { + if cfg!(feature = "backtrace") { Some(BacktraceStyle::Full) } else { None } + } + + fn as_usize(self) -> usize { + match self { + BacktraceStyle::Short => 1, + BacktraceStyle::Full => 2, + BacktraceStyle::Off => 3, + } + } + + fn from_usize(s: usize) -> Option<Self> { + Some(match s { + 0 => return None, + 1 => BacktraceStyle::Short, + 2 => BacktraceStyle::Full, + 3 => BacktraceStyle::Off, + _ => unreachable!(), + }) + } +} + +// Tracks whether we should/can capture a backtrace, and how we should display +// that backtrace. +// +// Internally stores equivalent of an Option<BacktraceStyle>. +static SHOULD_CAPTURE: AtomicUsize = AtomicUsize::new(0); + +/// Configure whether the default panic hook will capture and display a +/// backtrace. +/// +/// The default value for this setting may be set by the `RUST_BACKTRACE` +/// environment variable; see the details in [`get_backtrace_style`]. +#[unstable(feature = "panic_backtrace_config", issue = "93346")] +pub fn set_backtrace_style(style: BacktraceStyle) { + if !cfg!(feature = "backtrace") { + // If the `backtrace` feature of this crate isn't enabled, skip setting. + return; + } + SHOULD_CAPTURE.store(style.as_usize(), Ordering::Release); +} + +/// Checks whether the standard library's panic hook will capture and print a +/// backtrace. +/// +/// This function will, if a backtrace style has not been set via +/// [`set_backtrace_style`], read the environment variable `RUST_BACKTRACE` to +/// determine a default value for the backtrace formatting: +/// +/// The first call to `get_backtrace_style` may read the `RUST_BACKTRACE` +/// environment variable if `set_backtrace_style` has not been called to +/// override the default value. After a call to `set_backtrace_style` or +/// `get_backtrace_style`, any changes to `RUST_BACKTRACE` will have no effect. +/// +/// `RUST_BACKTRACE` is read according to these rules: +/// +/// * `0` for `BacktraceStyle::Off` +/// * `full` for `BacktraceStyle::Full` +/// * `1` for `BacktraceStyle::Short` +/// * Other values are currently `BacktraceStyle::Short`, but this may change in +/// the future +/// +/// Returns `None` if backtraces aren't currently supported. +#[unstable(feature = "panic_backtrace_config", issue = "93346")] +pub fn get_backtrace_style() -> Option<BacktraceStyle> { + if !cfg!(feature = "backtrace") { + // If the `backtrace` feature of this crate isn't enabled quickly return + // `Unsupported` so this can be constant propagated all over the place + // to optimize away callers. + return None; + } + if let Some(style) = BacktraceStyle::from_usize(SHOULD_CAPTURE.load(Ordering::Acquire)) { + return Some(style); + } + + // Setting environment variables for Fuchsia components isn't a standard + // or easily supported workflow. For now, display backtraces by default. + let format = if cfg!(target_os = "fuchsia") { + BacktraceStyle::Full + } else { + crate::env::var_os("RUST_BACKTRACE") + .map(|x| { + if &x == "0" { + BacktraceStyle::Off + } else if &x == "full" { + BacktraceStyle::Full + } else { + BacktraceStyle::Short + } + }) + .unwrap_or(BacktraceStyle::Off) + }; + set_backtrace_style(format); + Some(format) +} + #[cfg(test)] mod tests; diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 83ab13c963d..2b9ae3210de 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -9,6 +9,7 @@ #![deny(unsafe_op_in_unsafe_fn)] +use crate::panic::BacktraceStyle; use core::panic::{BoxMeUp, Location, PanicInfo}; use crate::any::Any; @@ -18,7 +19,7 @@ use crate::mem::{self, ManuallyDrop}; use crate::process; use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys::stdio::panic_output; -use crate::sys_common::backtrace::{self, RustBacktrace}; +use crate::sys_common::backtrace; use crate::sys_common::rwlock::StaticRWLock; use crate::sys_common::thread_info; use crate::thread; @@ -262,10 +263,10 @@ where fn default_hook(info: &PanicInfo<'_>) { // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - let backtrace_env = if panic_count::get_count() >= 2 { - backtrace::rust_backtrace_print_full() + let backtrace = if panic_count::get_count() >= 2 { + BacktraceStyle::full() } else { - backtrace::rust_backtrace_env() + crate::panic::get_backtrace_style() }; // The current implementation always returns `Some`. @@ -286,10 +287,14 @@ fn default_hook(info: &PanicInfo<'_>) { static FIRST_PANIC: AtomicBool = AtomicBool::new(true); - match backtrace_env { - RustBacktrace::Print(format) => drop(backtrace::print(err, format)), - RustBacktrace::Disabled => {} - RustBacktrace::RuntimeDisabled => { + match backtrace { + Some(BacktraceStyle::Short) => { + drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Short)) + } + Some(BacktraceStyle::Full) => { + drop(backtrace::print(err, crate::backtrace_rs::PrintFmt::Full)) + } + Some(BacktraceStyle::Off) => { if FIRST_PANIC.swap(false, Ordering::SeqCst) { let _ = writeln!( err, @@ -297,6 +302,8 @@ fn default_hook(info: &PanicInfo<'_>) { ); } } + // If backtraces aren't supported, do nothing. + None => {} } }; diff --git a/library/std/src/sys_common/backtrace.rs b/library/std/src/sys_common/backtrace.rs index dc581a0675b..b0b55592f6f 100644 --- a/library/std/src/sys_common/backtrace.rs +++ b/library/std/src/sys_common/backtrace.rs @@ -7,7 +7,6 @@ use crate::fmt; use crate::io; use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; -use crate::sync::atomic::{self, Ordering}; use crate::sys_common::mutex::StaticMutex; /// Max number of frames to print. @@ -144,62 +143,6 @@ where result } -pub enum RustBacktrace { - Print(PrintFmt), - Disabled, - RuntimeDisabled, -} - -// If the `backtrace` feature of this crate isn't enabled quickly return -// `Disabled` so this can be constant propagated all over the place to -// optimize away callers. -#[cfg(not(feature = "backtrace"))] -pub fn rust_backtrace_env() -> RustBacktrace { - RustBacktrace::Disabled -} - -// 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 rust_backtrace_env() -> RustBacktrace { - // Setting environment variables for Fuchsia components isn't a standard - // or easily supported workflow. For now, always display backtraces. - if cfg!(target_os = "fuchsia") { - return RustBacktrace::Print(PrintFmt::Full); - } - - static ENABLED: atomic::AtomicIsize = atomic::AtomicIsize::new(0); - match ENABLED.load(Ordering::SeqCst) { - 0 => {} - 1 => return RustBacktrace::RuntimeDisabled, - 2 => return RustBacktrace::Print(PrintFmt::Short), - _ => return RustBacktrace::Print(PrintFmt::Full), - } - - let (format, cache) = env::var_os("RUST_BACKTRACE") - .map(|x| { - if &x == "0" { - (RustBacktrace::RuntimeDisabled, 1) - } else if &x == "full" { - (RustBacktrace::Print(PrintFmt::Full), 3) - } else { - (RustBacktrace::Print(PrintFmt::Short), 2) - } - }) - .unwrap_or((RustBacktrace::RuntimeDisabled, 1)); - ENABLED.store(cache, Ordering::SeqCst); - format -} - -/// Setting for printing the full backtrace, unless backtraces are completely disabled -pub(crate) fn rust_backtrace_print_full() -> RustBacktrace { - if cfg!(feature = "backtrace") { - RustBacktrace::Print(PrintFmt::Full) - } else { - RustBacktrace::Disabled - } -} - /// Prints the filename of the backtrace frame. /// /// See also `output`. diff --git a/src/test/ui/panics/runtime-switch.legacy.run.stderr b/src/test/ui/panics/runtime-switch.legacy.run.stderr new file mode 100644 index 00000000000..979cc56b831 --- /dev/null +++ b/src/test/ui/panics/runtime-switch.legacy.run.stderr @@ -0,0 +1,5 @@ +thread 'main' panicked at 'explicit panic', $DIR/runtime-switch.rs:24:5 +stack backtrace: + 0: std::panicking::begin_panic + 1: runtime_switch::main +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. diff --git a/src/test/ui/panics/runtime-switch.rs b/src/test/ui/panics/runtime-switch.rs new file mode 100644 index 00000000000..c1634811406 --- /dev/null +++ b/src/test/ui/panics/runtime-switch.rs @@ -0,0 +1,25 @@ +// Test for std::panic::set_backtrace_style. + +// compile-flags: -O +// run-fail +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +// ignore-msvc see #62897 and `backtrace-debuginfo.rs` test +// ignore-android FIXME #17520 +// ignore-openbsd no support for libbacktrace without filename +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support +// ignore-sgx no subprocess support + +// NOTE(eddyb) output differs between symbol mangling schemes +// revisions: legacy v0 +// [legacy] compile-flags: -Zunstable-options -Csymbol-mangling-version=legacy +// [v0] compile-flags: -Csymbol-mangling-version=v0 + +#![feature(panic_backtrace_config)] + +fn main() { + std::panic::set_backtrace_style(std::panic::BacktraceStyle::Short); + panic!() +} diff --git a/src/test/ui/panics/runtime-switch.v0.run.stderr b/src/test/ui/panics/runtime-switch.v0.run.stderr new file mode 100644 index 00000000000..48f829c26d4 --- /dev/null +++ b/src/test/ui/panics/runtime-switch.v0.run.stderr @@ -0,0 +1,5 @@ +thread 'main' panicked at 'explicit panic', $DIR/runtime-switch.rs:24:5 +stack backtrace: + 0: std::panicking::begin_panic::<&str> + 1: runtime_switch::main +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 24a10018779..cb6d56a167d 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -58,6 +58,7 @@ const EXCEPTION_PATHS: &[&str] = &[ "library/std/src/path.rs", "library/std/src/sys_common", // Should only contain abstractions over platforms "library/std/src/net/test.rs", // Utility helpers for tests + "library/std/src/panic.rs", // fuchsia-specific panic backtrace handling ]; pub fn check(path: &Path, bad: &mut bool) { |
