Merge #250

250: Backtrace options r=BriocheBerlin a=BriocheBerlin

Closes #198

Co-authored-by: BriocheBerlin <brigitte.markmann@asquera.de>
Co-authored-by: BriocheBerlin <brigitte.markmann@ferrous-systems.com>

README.md

@@ -186,14 +186,35 @@ $ echo $?
 
 ⚠️ **NOTE** when you run your application with `probe-run`, the `HardFault` handler (default or user-defined) will *NOT* be executed.
 
-### Forcing backtraces
+### Backtrace options
+#### --backtrace
 
-If you'd like to see a backtrace at the end of successful program runs as well, you can enable this by setting the `--force-backtrace` flag:
+The `--backtrace` flag is optional and can get passed the following values:
+
+* `--backtrace=always`   - forced backtrace (if you'd like to see a backtrace at the end of successful program run)
+* `--backtrace=never`    - suppresed backtrace
+* `--backtrace=auto`     - default, shows a backtrace if the program panics or the stack overflows
+
+Run it like this (example for a forced backtrace):
+
+``` console
+$ cargo run --bin hello --backtrace=always
+```
+
+#### --backtrace-limit
+
+The `--backtrace-limit` flag is optional and defaults to 50. It is possible to set any number.
+
+`--backtrace-limit=0` is accepted and means "no limit".
+
+To show a shortened backtrace showing 5 frames, run:
 
 ``` console
-$ cargo run --bin hello --force-backtrace
+$ cargo run --bin panic --backtrace-limit=5
 ```
 
+Note: if `--backtrace=never` is set, setting `--backtrace-limit` has no effect.
+
 ## Troubleshooting
 
 ### `probe-run --list-probes` says "No devices were found."
@@ -267,7 +288,7 @@ For easier copy-paste-ability, here's an example how to try out your local `prob
 
 ```console
 $ cd probe-run/
-$ PROBE_RUN_IGNORE_VERSION=1 cargo run -- --chip nRF52840_xxAA --max-backtrace-len=10 hello
+$ PROBE_RUN_IGNORE_VERSION=1 cargo run -- --chip nRF52840_xxAA --backtrace-limit=10 hello
   ˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆ                                   ˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆˆ ˆˆˆˆˆ
   environment variables                                        extra flags             binary to be
   (optional)                                                   (optional)              flashed & run

src/backtrace/mod.rs

@@ -8,10 +8,29 @@ mod pp;
 mod symbolicate;
 mod unwind;
 
+#[derive(PartialEq, Eq)]
+pub(crate) enum BacktraceOptions {
+    Auto,
+    Never,
+    Always,
+}
+
+impl From<&String> for BacktraceOptions {
+    fn from(item: &String) -> Self {
+        match item.as_str() {
+            "auto" | "Auto" => BacktraceOptions::Auto,
+            "never" | "Never" => BacktraceOptions::Never,
+            "always" | "Always" => BacktraceOptions::Always,
+            _ => panic!("options for `--backtrace` are `auto`, `never`, `always`."),
+        }
+    }
+}
+
 pub(crate) struct Settings<'p> {
     pub(crate) current_dir: &'p Path,
-    pub(crate) max_backtrace_len: u32,
-    pub(crate) force_backtrace: bool,
+    pub(crate) backtrace: BacktraceOptions,
+    pub(crate) panic_present: bool,
+    pub(crate) backtrace_limit: u32,
     pub(crate) shorten_paths: bool,
 }
 
@@ -20,7 +39,7 @@ pub(crate) fn print(
     core: &mut Core,
     elf: &Elf,
     active_ram_region: &Option<RamRegion>,
-    settings: &Settings,
+    settings: &mut Settings<'_>,
 ) -> anyhow::Result<Outcome> {
     let unwind = unwind::target(core, elf, active_ram_region);
 
@@ -31,12 +50,24 @@ pub(crate) fn print(
         .iter()
         .any(|raw_frame| raw_frame.is_exception());
 
-    let print_backtrace = settings.force_backtrace
-        || unwind.outcome == Outcome::StackOverflow
-        || unwind.corrupted
-        || contains_exception;
+    let print_backtrace = match settings.backtrace {
+        BacktraceOptions::Never => false,
+        BacktraceOptions::Always => true,
+        BacktraceOptions::Auto => {
+            settings.panic_present
+                || unwind.outcome == Outcome::StackOverflow
+                || unwind.corrupted
+                || contains_exception
+        }
+    };
+
+    // `0` disables the limit and we want to show _all_ frames
+    if settings.backtrace_limit == 0 {
+        let frames_number = &frames.len();
+        settings.backtrace_limit = *frames_number as u32;
+    }
 
-    if print_backtrace && settings.max_backtrace_len > 0 {
+    if print_backtrace && settings.backtrace_limit > 0 {
         pp::backtrace(&frames, settings);
 
         if unwind.corrupted {

src/backtrace/pp.rs

@@ -8,7 +8,7 @@ use crate::dep;
 
 use super::{symbolicate::Frame, Settings};
 
-/// Pretty prints processed backtrace frames up to `max_backtrace_len`
+/// Pretty prints processed backtrace frames up to `backtrace_limit`
 pub(crate) fn backtrace(frames: &[Frame], settings: &Settings) {
     println!("{}", "stack backtrace:".dimmed());
 
@@ -59,10 +59,10 @@ pub(crate) fn backtrace(frames: &[Frame], settings: &Settings) {
 
                 frame_index += 1;
 
-                if frame_index >= settings.max_backtrace_len {
+                if frame_index >= settings.backtrace_limit {
                     log::warn!(
                         "maximum backtrace length of {} reached; cutting off the rest.const ",
-                        settings.max_backtrace_len
+                        settings.backtrace_limit
                     );
                     log::warn!("note: re-run with `--max-backtrace-len=<your maximum>` to extend this limit");
 

src/cli.rs

@@ -55,13 +55,13 @@ pub(crate) struct Opts {
     #[structopt(short = "V", long)]
     version: bool,
 
-    /// Print a backtrace even if the program ran successfully
-    #[structopt(long)]
-    pub(crate) force_backtrace: bool,
+    /// Disable or enable backtrace (auto in case of panic or stack overflow).
+    #[structopt(long, default_value = "auto")]
+    pub(crate) backtrace: String,
 
     /// Configure the number of lines to print before a backtrace gets cut off
     #[structopt(long, default_value = "50")]
-    pub(crate) max_backtrace_len: u32,
+    pub(crate) backtrace_limit: u32,
 
     /// Whether to shorten paths (e.g. to crates.io dependencies) in backtraces and defmt logs
     #[structopt(long)]

src/main.rs

@@ -105,18 +105,22 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any
         .map(|canary| canary.touched(&mut core, elf))
         .transpose()?
         .unwrap_or(false);
-    let backtrace_settings = backtrace::Settings {
+
+    let panic_present = canary_touched || halted_due_to_signal;
+
+    let mut backtrace_settings = backtrace::Settings {
         current_dir,
-        max_backtrace_len: opts.max_backtrace_len,
-        force_backtrace: opts.force_backtrace || canary_touched || halted_due_to_signal,
+        backtrace_limit: opts.backtrace_limit,
+        backtrace: (&opts.backtrace).into(),
+        panic_present,
         shorten_paths: opts.shorten_paths,
     };
 
     let outcome = backtrace::print(
         &mut core,
         elf,
         &target_info.active_ram_region,
-        &backtrace_settings,
+        &mut backtrace_settings,
     )?;
 
     core.reset_and_halt(TIMEOUT)?;

tests/README.md

@@ -46,12 +46,12 @@ rustflags = [
 Write your test that captures `probe-run`s output for your test ELF and check the result with `insta::assert_snapshot!(run_output);`
 
 ### 3. cargo insta review
-When you run `cargo test -- -ignored` for the first time after you've added your new test, it will fail.
+When you run `cargo test -- --ignored` for the first time after you've added your new test, it will fail.
 This first run creates a snapshot which you can then store as a "known good"
 
 run
 ```console
-$ cargo install insta
+$ cargo install cargo-insta
 $ cargo insta review
 ```