about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-02-02 22:03:23 +0000
committerbors <bors@rust-lang.org>2022-02-02 22:03:23 +0000
commitb3800860e123443ffada615538926beed6bc4f85 (patch)
tree4c666bc883ba940757f1f63ee612d8f8edc5bfc2
parent27f5d830eb11cd7bdc834d6f0d78120976f75443 (diff)
parent85930c8f444e1ece1c92a0f9e39814f72d867e43 (diff)
downloadrust-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.rs114
-rw-r--r--library/std/src/panicking.rs23
-rw-r--r--library/std/src/sys_common/backtrace.rs57
-rw-r--r--src/test/ui/panics/runtime-switch.legacy.run.stderr5
-rw-r--r--src/test/ui/panics/runtime-switch.rs25
-rw-r--r--src/test/ui/panics/runtime-switch.v0.run.stderr5
-rw-r--r--src/tools/tidy/src/pal.rs1
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) {