Skip to content

Commit

Permalink
Auto merge of rust-lang#93101 - Mark-Simulacrum:library-backtrace, r=…
Browse files Browse the repository at this point in the history
…yaahc

Support configuring whether to capture backtraces at runtime

Tracking issue: rust-lang#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](rust-lang#79085 (comment)) 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 rust-lang#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.
  • Loading branch information
bors committed Feb 2, 2022
2 parents 27f5d83 + 85930c8 commit b380086
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 65 deletions.
114 changes: 114 additions & 0 deletions library/std/src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
23 changes: 15 additions & 8 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#![deny(unsafe_op_in_unsafe_fn)]

use crate::panic::BacktraceStyle;
use core::panic::{BoxMeUp, Location, PanicInfo};

use crate::any::Any;
Expand All @@ -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;
Expand Down Expand Up @@ -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`.
Expand All @@ -286,17 +287,23 @@ 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,
"note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace"
);
}
}
// If backtraces aren't supported, do nothing.
None => {}
}
};

Expand Down
57 changes: 0 additions & 57 deletions library/std/src/sys_common/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`.
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/panics/runtime-switch.legacy.run.stderr
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 25 additions & 0 deletions src/test/ui/panics/runtime-switch.rs
Original file line number Diff line number Diff line change
@@ -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!()
}
5 changes: 5 additions & 0 deletions src/test/ui/panics/runtime-switch.v0.run.stderr
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions src/tools/tidy/src/pal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b380086

Please sign in to comment.