From 70aa27ffc18a2d494c559085192002d737f4811c Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Mon, 20 Feb 2023 16:40:58 +0100
Subject: [PATCH 01/10] Pack preparation steps into `Canary::prepare`

---
 src/canary.rs | 103 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 64 insertions(+), 39 deletions(-)

diff --git a/src/canary.rs b/src/canary.rs
index db0429aa..9e41b45f 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -2,7 +2,11 @@ use std::time::Instant;
 
 use probe_rs::{Core, MemoryInterface, RegisterId};
 
-use crate::{registers::PC, Elf, TargetInfo, TIMEOUT};
+use crate::{
+    registers::PC,
+    target_info::{StackInfo, TargetInfo},
+    Elf, TIMEOUT,
+};
 
 /// Canary value
 const CANARY_U8: u8 = 0xAA;
@@ -45,6 +49,7 @@ pub struct Canary {
     stack_available: u32,
     data_below_stack: bool,
     measure_stack: bool,
+    size_kb: f64,
 }
 
 impl Canary {
@@ -57,55 +62,27 @@ impl Canary {
         elf: &Elf,
         measure_stack: bool,
     ) -> Result<Option<Self>, anyhow::Error> {
-        let stack_info = match &target_info.stack_info {
-            Some(stack_info) => stack_info,
-            None => {
-                log::debug!("couldn't find valid stack range, not placing stack canary");
-                return Ok(None);
-            }
-        };
-
-        if elf.program_uses_heap() {
-            log::debug!("heap in use, not placing stack canary");
-            return Ok(None);
-        }
-
-        let stack_start = *stack_info.range.start();
-        let stack_available = *stack_info.range.end() - stack_start;
-
-        let size = if measure_stack {
-            // When measuring stack consumption, we have to color the whole stack.
-            stack_available as usize
-        } else {
-            round_up(stack_available / 10, 4) as usize
+        let canary = match Self::prepare(&target_info.stack_info, elf, measure_stack) {
+            Some(canary) => canary,
+            None => return Ok(None),
         };
 
-        log::debug!(
-            "{stack_available} bytes of stack available ({:#010X} ..= {:#010X}), using {size} byte canary",
-            stack_info.range.start(),
-            stack_info.range.end(),
-        );
-
-        let size_kb = size as f64 / 1024.0;
         if measure_stack {
             // Painting 100KB or more takes a few seconds, so provide user feedback.
-            log::info!("painting {size_kb:.2} KiB of RAM for stack usage estimation");
+            log::info!(
+                "painting {:.2} KiB of RAM for stack usage estimation",
+                canary.size_kb
+            );
         }
         let start = Instant::now();
-        paint_subroutine::execute(core, stack_start, size as u32)?;
+        paint_subroutine::execute(core, canary.address, canary.size as u32)?;
         let seconds = start.elapsed().as_secs_f64();
         log::trace!(
             "setting up canary took {seconds:.3}s ({:.2} KiB/s)",
-            size_kb / seconds
+            canary.size_kb / seconds
         );
 
-        Ok(Some(Canary {
-            address: stack_start,
-            size,
-            stack_available,
-            data_below_stack: stack_info.data_below_stack,
-            measure_stack,
-        }))
+        Ok(Some(canary))
     }
 
     /// Detect if the stack canary was touched.
@@ -164,6 +141,54 @@ impl Canary {
             }
         }
     }
+
+    /// Prepare, but not place the canary.
+    ///
+    /// If this succeeds, we have all the information we need in order to place the canary.
+    fn prepare(stack_info: &Option<StackInfo>, elf: &Elf, measure_stack: bool) -> Option<Self> {
+        let stack_info = match stack_info {
+            Some(stack_info) => stack_info,
+            None => {
+                log::debug!("couldn't find valid stack range, not placing stack canary");
+                return None;
+            }
+        };
+
+        if elf.program_uses_heap() {
+            log::debug!("heap in use, not placing stack canary");
+            return None;
+        }
+
+        let stack_start = *stack_info.range.start();
+        let stack_available = *stack_info.range.end() - stack_start;
+
+        let size = if measure_stack {
+            // When measuring stack consumption, we have to color the whole stack.
+            stack_available as usize
+        } else {
+            // We consider >90% stack usage a potential stack overflow, but don't go beyond 1 kb
+            // since filling a lot of RAM is slow (and 1 kb should be "good enough" for what we're
+            // doing).
+            round_up(1024.min(stack_available / 10), 4) as usize
+        };
+
+        log::debug!(
+            "{stack_available} bytes of stack available ({:#010X} ..= {:#010X}), using {size} byte canary",
+            stack_info.range.start(),
+            stack_info.range.end(),
+        );
+
+        let size_kb = size as f64 / 1024.0;
+
+        Some(Canary {
+            address: stack_start,
+            size,
+            stack_available,
+            data_below_stack: stack_info.data_below_stack,
+            measure_stack,
+            size_kb,
+        })
+    }
 }
 
 /// Rounds up to the next multiple of `k` that is greater or equal to `n`.

From 387b3bd8ce8f68a976194facb3d8ae2567538280 Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Mon, 20 Feb 2023 16:58:25 +0100
Subject: [PATCH 02/10] Deprecate `--measure-stack`

---
 src/canary.rs | 133 +++++++++++++-------------------------------------
 src/cli.rs    |   4 ++
 src/main.rs   |   6 +--
 3 files changed, 40 insertions(+), 103 deletions(-)

diff --git a/src/canary.rs b/src/canary.rs
index 9e41b45f..1d8bc607 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -45,11 +45,8 @@ const CANARY_U32: u32 = u32::from_le_bytes([CANARY_U8, CANARY_U8, CANARY_U8, CAN
 #[derive(Clone, Copy)]
 pub struct Canary {
     address: u32,
-    size: usize,
-    stack_available: u32,
+    size: u32,
     data_below_stack: bool,
-    measure_stack: bool,
-    size_kb: f64,
 }
 
 impl Canary {
@@ -60,26 +57,23 @@ impl Canary {
         core: &mut Core,
         target_info: &TargetInfo,
         elf: &Elf,
-        measure_stack: bool,
     ) -> Result<Option<Self>, anyhow::Error> {
-        let canary = match Self::prepare(&target_info.stack_info, elf, measure_stack) {
+        let canary = match Self::prepare(&target_info.stack_info, elf) {
             Some(canary) => canary,
             None => return Ok(None),
         };
 
-        if measure_stack {
-            // Painting 100KB or more takes a few seconds, so provide user feedback.
-            log::info!(
-                "painting {:.2} KiB of RAM for stack usage estimation",
-                canary.size_kb
-            );
-        }
+        let size_kb = canary.size_kb();
+
+        // Painting 100KB or more takes a few seconds, so provide user feedback.
+        log::info!("painting {size_kb:.2} KiB of RAM for stack usage estimation");
+
         let start = Instant::now();
-        paint_subroutine::execute(core, canary.address, canary.size as u32)?;
+        paint_subroutine::execute(core, canary.address, canary.size)?;
         let seconds = start.elapsed().as_secs_f64();
         log::trace!(
             "setting up canary took {seconds:.3}s ({:.2} KiB/s)",
-            canary.size_kb / seconds
+            size_kb / seconds
         );
 
         Ok(Some(canary))
@@ -87,65 +81,42 @@ impl Canary {
 
     /// Detect if the stack canary was touched.
     pub fn touched(self, core: &mut Core, elf: &Elf) -> anyhow::Result<bool> {
-        let size_kb = self.size as f64 / 1024.0;
-        if self.measure_stack {
-            log::info!("reading {size_kb:.2} KiB of RAM for stack usage estimation");
-        }
+        let size_kb = self.size_kb();
+        log::info!("reading {size_kb:.2} KiB of RAM for stack usage estimation",);
         let start = Instant::now();
-        let touched_address = measure_subroutine::execute(core, self.address, self.size as u32)?;
+
+        let touched_address = measure_subroutine::execute(core, self.address, self.size)?;
+
         let seconds = start.elapsed().as_secs_f64();
         log::trace!(
             "reading canary took {seconds:.3}s ({:.2} KiB/s)",
             size_kb / seconds
         );
 
-        let min_stack_usage = match touched_address {
-            Some(touched_address) => {
-                log::debug!("canary was touched at {touched_address:#010X}");
-                Some(elf.vector_table.initial_stack_pointer - touched_address)
-            }
-            None => None,
-        };
+        let min_stack_usage = touched_address.map(|touched_address| {
+            log::debug!("canary was touched at {touched_address:#010X}");
+            elf.vector_table.initial_stack_pointer - touched_address
+        });
 
-        if self.measure_stack {
-            let min_stack_usage = min_stack_usage.unwrap_or(0);
-            let used_kb = min_stack_usage as f64 / 1024.0;
-            let avail_kb = self.stack_available as f64 / 1024.0;
-            let pct = used_kb / avail_kb * 100.0;
-            log::info!(
-                "program has used at least {used_kb:.2}/{avail_kb:.2} KiB ({pct:.1}%) of stack space"
-            );
+        let min_stack_usage = min_stack_usage.unwrap_or(0);
+        let used_kb = min_stack_usage as f64 / 1024.0;
+        let pct = used_kb / size_kb * 100.0;
+        log::info!(
+            "program has used at least {used_kb:.2}/{size_kb:.2} KiB ({pct:.1}%) of stack space"
+        );
 
-            // Don't test for stack overflows if we're measuring stack usage.
-            Ok(false)
+        if pct > 90.0 && self.data_below_stack {
+            log::warn!("data segments might be corrupted due to stack overflow");
+            Ok(true)
         } else {
-            match min_stack_usage {
-                Some(min_stack_usage) => {
-                    let used_kb = min_stack_usage as f64 / 1024.0;
-                    let avail_kb = self.stack_available as f64 / 1024.0;
-                    let pct = used_kb / avail_kb * 100.0;
-                    log::warn!(
-                        "program has used at least {used_kb:.2}/{avail_kb:.2} KiB ({pct:.1}%) of stack space",
-                    );
-
-                    if self.data_below_stack {
-                        log::warn!("data segments might be corrupted due to stack overflow");
-                    }
-
-                    Ok(true)
-                }
-                None => {
-                    log::debug!("stack canary intact");
-                    Ok(false)
-                }
-            }
+            Ok(false)
         }
     }
 
     /// Prepare, but not place the canary.
     ///
     /// If this succeeds, we have all the information we need in order to place the canary.
-    fn prepare(stack_info: &Option<StackInfo>, elf: &Elf, measure_stack: bool) -> Option<Self> {
+    fn prepare(stack_info: &Option<StackInfo>, elf: &Elf) -> Option<Self> {
         let stack_info = match stack_info {
             Some(stack_info) => stack_info,
             None => {
@@ -160,42 +131,23 @@ impl Canary {
         }
 
         let stack_start = *stack_info.range.start();
-        let stack_available = *stack_info.range.end() - stack_start;
-
-        let size = if measure_stack {
-            // When measuring stack consumption, we have to color the whole stack.
-            stack_available as usize
-        } else {
-            // We consider >90% stack usage a potential stack overflow, but don't go beyond 1 kb
-            // since filling a lot of RAM is slow (and 1 kb should be "good enough" for what we're
-            // doing).
-            round_up(1024.min(stack_available / 10), 4) as usize
-        };
+        let size = *stack_info.range.end() - stack_start;
 
         log::debug!(
-            "{stack_available} bytes of stack available ({:#010X} ..= {:#010X}), using {size} byte canary",
+            "{size} bytes of stack available ({:#010X} ..= {:#010X})",
             stack_info.range.start(),
             stack_info.range.end(),
         );
 
-        let size_kb = size as f64 / 1024.0;
-
         Some(Canary {
             address: stack_start,
             size,
-            stack_available,
             data_below_stack: stack_info.data_below_stack,
-            measure_stack,
-            size_kb,
         })
     }
-}
 
-/// Rounds up to the next multiple of `k` that is greater or equal to `n`.
-fn round_up(n: u32, k: u32) -> u32 {
-    match n % k {
-        0 => n,
-        rem => n + k - rem,
+    fn size_kb(self) -> f64 {
+        self.size as f64 / 1024.0
     }
 }
 
@@ -465,22 +417,3 @@ fn execute_subroutine<const N: usize>(
 
     Ok(())
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    use rstest::rstest;
-
-    #[rstest]
-    #[case(2, 4, 4)]
-    #[case(4, 4, 4)]
-    #[case(6, 4, 8)]
-    #[case(8, 4, 8)]
-    #[case::odd(5, 3, 6)]
-    #[should_panic]
-    #[case::div_zero(4, 0, 0)]
-    fn test_round_up(#[case] n: u32, #[case] k: u32, #[case] res: u32) {
-        assert_eq!(round_up(n, k), res);
-    }
-}
diff --git a/src/cli.rs b/src/cli.rs
index dadece14..2314f56e 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -123,6 +123,10 @@ pub fn handle_arguments() -> anyhow::Result<i32> {
         }
     });
 
+    if opts.measure_stack {
+        log::warn!("use of deprecated option `--measure-stack`: Has no effect anymore and will vanish on next minor release")
+    }
+
     if opts.version {
         print_version();
         Ok(EXIT_SUCCESS)
diff --git a/src/main.rs b/src/main.rs
index dc276aee..6f9c38e4 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -71,9 +71,9 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any
     let target_info = TargetInfo::new(elf, memory_map, probe_target, stack_start)?;
 
     // install stack canary
-    let canary = Canary::install(core, &target_info, elf, opts.measure_stack)?;
-    if opts.measure_stack && canary.is_none() {
-        bail!("failed to set up stack measurement");
+    let canary = Canary::install(core, &target_info, elf)?;
+    if canary.is_none() {
+        log::info!("stack measurement was not set up");
     }
 
     // run program and print logs until there is an exception

From 9b26807220dce6303cc850c8b2091fe5245b8fa2 Mon Sep 17 00:00:00 2001
From: Johann Hemmann <johann.hemmann@code.berlin>
Date: Mon, 19 Jun 2023 15:48:32 +0200
Subject: [PATCH 03/10] Refactor `backtrace::Settings`

---
 src/backtrace/mod.rs | 23 +++++++++++------------
 src/main.rs          |  8 ++++----
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs
index 1a1ee2cf..f1f920bc 100644
--- a/src/backtrace/mod.rs
+++ b/src/backtrace/mod.rs
@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::PathBuf;
 
 use probe_rs::Core;
 use signal_hook::consts::signal;
@@ -27,36 +27,36 @@ impl From<&String> for BacktraceOptions {
     }
 }
 
-pub struct Settings<'p> {
+pub struct Settings {
     pub backtrace_limit: u32,
     pub backtrace: BacktraceOptions,
-    pub canary_touched: bool,
-    pub current_dir: &'p Path,
+    pub current_dir: PathBuf,
     pub halted_due_to_signal: bool,
     pub include_addresses: bool,
     pub shorten_paths: bool,
+    pub stack_overflow: bool,
 }
 
-impl<'p> Settings<'p> {
+impl Settings {
     pub fn new(
-        canary_touched: bool,
-        current_dir: &'p Path,
+        current_dir: PathBuf,
         halted_due_to_signal: bool,
         opts: &Opts,
+        stack_overflow: bool,
     ) -> Self {
         Self {
             backtrace_limit: opts.backtrace_limit,
             backtrace: (&opts.backtrace).into(),
-            canary_touched,
             current_dir,
             halted_due_to_signal,
             include_addresses: opts.verbose > 0,
             shorten_paths: opts.shorten_paths,
+            stack_overflow,
         }
     }
 
     fn panic_present(&self) -> bool {
-        self.canary_touched || self.halted_due_to_signal
+        self.stack_overflow || self.halted_due_to_signal
     }
 }
 
@@ -65,11 +65,10 @@ pub fn print(
     core: &mut Core,
     elf: &Elf,
     target_info: &TargetInfo,
-    settings: &mut Settings<'_>,
+    settings: &mut Settings,
 ) -> anyhow::Result<Outcome> {
     let mut unwind = unwind::target(core, elf, target_info);
-
-    let frames = symbolicate::frames(&unwind.raw_frames, settings.current_dir, elf);
+    let frames = symbolicate::frames(&unwind.raw_frames, &settings.current_dir, elf);
 
     let contains_exception = unwind
         .raw_frames
diff --git a/src/main.rs b/src/main.rs
index 6f9c38e4..6aad31ac 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -78,19 +78,19 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any
 
     // run program and print logs until there is an exception
     start_program(core, elf)?;
-    let current_dir = &env::current_dir()?;
-    let halted_due_to_signal = print_logs(core, current_dir, elf, &target_info.memory_map, opts)?; // blocks until exception
+    let current_dir = env::current_dir()?;
+    let halted_due_to_signal = print_logs(core, &current_dir, elf, &target_info.memory_map, opts)?; // blocks until exception
     print_separator()?;
 
     // analyze stack canary
-    let canary_touched = canary
+    let stack_overflow = canary
         .map(|canary| canary.touched(core, elf))
         .transpose()?
         .unwrap_or(false);
 
     // print the backtrace
     let mut backtrace_settings =
-        backtrace::Settings::new(canary_touched, current_dir, halted_due_to_signal, opts);
+        backtrace::Settings::new(current_dir, halted_due_to_signal, opts, stack_overflow);
     let outcome = backtrace::print(core, elf, &target_info, &mut backtrace_settings)?;
 
     // reset the target

From ad0770a8eb333279ab89ef968fd72a60d5b85e39 Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Mon, 20 Feb 2023 19:19:26 +0100
Subject: [PATCH 04/10] Refactor `Canary`

---
 src/canary.rs | 102 +++++++++++++++++++++++++++-----------------------
 src/main.rs   |   4 +-
 2 files changed, 58 insertions(+), 48 deletions(-)

diff --git a/src/canary.rs b/src/canary.rs
index 1d8bc607..2fb6cc9a 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -44,9 +44,10 @@ const CANARY_U32: u32 = u32::from_le_bytes([CANARY_U8, CANARY_U8, CANARY_U8, CAN
 /// overflow.
 #[derive(Clone, Copy)]
 pub struct Canary {
-    address: u32,
-    size: u32,
+    addr: u32,
     data_below_stack: bool,
+    size: u32,
+    size_kb: f64,
 }
 
 impl Canary {
@@ -55,60 +56,66 @@ impl Canary {
     /// Assumes that the target was reset-halted.
     pub fn install(
         core: &mut Core,
-        target_info: &TargetInfo,
         elf: &Elf,
-    ) -> Result<Option<Self>, anyhow::Error> {
-        let canary = match Self::prepare(&target_info.stack_info, elf) {
+        target_info: &TargetInfo,
+    ) -> anyhow::Result<Option<Self>> {
+        let canary = match Self::prepare(elf, &target_info.stack_info) {
             Some(canary) => canary,
             None => return Ok(None),
         };
 
-        let size_kb = canary.size_kb();
+        let start = Instant::now();
 
-        // Painting 100KB or more takes a few seconds, so provide user feedback.
-        log::info!("painting {size_kb:.2} KiB of RAM for stack usage estimation");
+        // paint stack
+        paint_subroutine::execute(core, canary.addr, canary.size)?;
 
-        let start = Instant::now();
-        paint_subroutine::execute(core, canary.address, canary.size)?;
         let seconds = start.elapsed().as_secs_f64();
-        log::trace!(
-            "setting up canary took {seconds:.3}s ({:.2} KiB/s)",
-            size_kb / seconds
-        );
+        canary.log_time("painting", seconds);
 
         Ok(Some(canary))
     }
 
-    /// Detect if the stack canary was touched.
-    pub fn touched(self, core: &mut Core, elf: &Elf) -> anyhow::Result<bool> {
-        let size_kb = self.size_kb();
-        log::info!("reading {size_kb:.2} KiB of RAM for stack usage estimation",);
+    /// Measure the stack usage.
+    ///
+    /// Returns `true` if a stack overflow is likely.
+    pub fn measure(self, core: &mut Core, elf: &Elf) -> anyhow::Result<bool> {
         let start = Instant::now();
 
-        let touched_address = measure_subroutine::execute(core, self.address, self.size)?;
+        // measure stack usage
+        let touched_address = measure_subroutine::execute(core, self.addr, self.size)?;
 
         let seconds = start.elapsed().as_secs_f64();
-        log::trace!(
-            "reading canary took {seconds:.3}s ({:.2} KiB/s)",
-            size_kb / seconds
-        );
+        self.log_time("reading", seconds);
 
-        let min_stack_usage = touched_address.map(|touched_address| {
-            log::debug!("canary was touched at {touched_address:#010X}");
-            elf.vector_table.initial_stack_pointer - touched_address
-        });
+        let min_stack_usage = match touched_address {
+            Some(touched_address) => {
+                log::debug!("stack was touched at {touched_address:#010X}");
+                elf.vector_table.initial_stack_pointer - touched_address
+            }
+            None => {
+                log::warn!("stack was not used at all");
+                0
+            }
+        };
 
-        let min_stack_usage = min_stack_usage.unwrap_or(0);
         let used_kb = min_stack_usage as f64 / 1024.0;
-        let pct = used_kb / size_kb * 100.0;
-        log::info!(
-            "program has used at least {used_kb:.2}/{size_kb:.2} KiB ({pct:.1}%) of stack space"
+        let pct = used_kb / self.size_kb * 100.0;
+        let msg = format!(
+            "program has used at least {used_kb:.2}/{:.2} KiB ({pct:.1}%) of stack space",
+            self.size_kb
         );
 
-        if pct > 90.0 && self.data_below_stack {
-            log::warn!("data segments might be corrupted due to stack overflow");
+        // stack touched?
+        //
+        // We consider >90% stack usage a potential stack overflow
+        if pct > 90.0 {
+            log::warn!("{}", msg);
+            if self.data_below_stack {
+                log::warn!("data segments might be corrupted due to stack overflow");
+            }
             Ok(true)
         } else {
+            log::info!("{}", msg);
             Ok(false)
         }
     }
@@ -116,7 +123,7 @@ impl Canary {
     /// Prepare, but not place the canary.
     ///
     /// If this succeeds, we have all the information we need in order to place the canary.
-    fn prepare(stack_info: &Option<StackInfo>, elf: &Elf) -> Option<Self> {
+    fn prepare(elf: &Elf, stack_info: &Option<StackInfo>) -> Option<Self> {
         let stack_info = match stack_info {
             Some(stack_info) => stack_info,
             None => {
@@ -130,24 +137,28 @@ impl Canary {
             return None;
         }
 
-        let stack_start = *stack_info.range.start();
-        let size = *stack_info.range.end() - stack_start;
+        let stack_addr = *stack_info.range.start();
+        let stack_size = *stack_info.range.end() - stack_addr;
 
         log::debug!(
-            "{size} bytes of stack available ({:#010X} ..= {:#010X})",
-            stack_info.range.start(),
+            "{stack_size} bytes of stack available ({stack_addr:#010X} ..= {:#010X})",
             stack_info.range.end(),
         );
 
         Some(Canary {
-            address: stack_start,
-            size,
+            addr: stack_addr,
             data_below_stack: stack_info.data_below_stack,
+            size: stack_size,
+            size_kb: stack_size as f64 / 1024.0,
         })
     }
 
-    fn size_kb(self) -> f64 {
-        self.size as f64 / 1024.0
+    fn log_time(&self, action: &str, seconds: f64) {
+        log::debug!(
+            "{action} {:.2} KiB of RAM took {seconds:.3}s ({:.2} KiB/s)",
+            self.size_kb,
+            self.size_kb / seconds
+        )
     }
 }
 
@@ -315,9 +326,8 @@ mod measure_subroutine {
         assert_subroutine!(low_addr, stack_size, self::SUBROUTINE.len() as u32);
 
         // use probe to search through the memory the subroutine will be written to
-        match self::search_with_probe(core, low_addr)? {
-            addr @ Some(_) => return Ok(addr), // if we find a touched value, return early ...
-            None => {}                         // ... otherwise we continue
+        if let Some(addr) = self::search_with_probe(core, low_addr)? {
+            return Ok(Some(addr)); // return early, if we find a touched value
         }
 
         super::execute_subroutine(core, low_addr, stack_size, self::SUBROUTINE)?;
@@ -352,7 +362,7 @@ mod measure_subroutine {
             .to_le_bytes()
             .into_iter()
             .position(|b| b != CANARY_U8)
-            .unwrap();
+            .expect("some byte has to be touched, if `word_addr != 0`");
 
         Ok(Some(word_addr + offset as u32))
     }
diff --git a/src/main.rs b/src/main.rs
index 6aad31ac..f7f81e63 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -71,7 +71,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any
     let target_info = TargetInfo::new(elf, memory_map, probe_target, stack_start)?;
 
     // install stack canary
-    let canary = Canary::install(core, &target_info, elf)?;
+    let canary = Canary::install(core, elf, &target_info)?;
     if canary.is_none() {
         log::info!("stack measurement was not set up");
     }
@@ -84,7 +84,7 @@ fn run_target_program(elf_path: &Path, chip_name: &str, opts: &cli::Opts) -> any
 
     // analyze stack canary
     let stack_overflow = canary
-        .map(|canary| canary.touched(core, elf))
+        .map(|canary| canary.measure(core, elf))
         .transpose()?
         .unwrap_or(false);
 

From e5637ac9ba2676cbed335a8a5a9573e9994c9f34 Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Mon, 20 Feb 2023 19:36:26 +0100
Subject: [PATCH 05/10] Do subroutine assertions earlier

---
 src/canary.rs | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/canary.rs b/src/canary.rs
index 2fb6cc9a..44664d89 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -145,6 +145,8 @@ impl Canary {
             stack_info.range.end(),
         );
 
+        Self::assert_subroutines(stack_addr, stack_size)?;
+
         Some(Canary {
             addr: stack_addr,
             data_below_stack: stack_info.data_below_stack,
@@ -160,23 +162,28 @@ impl Canary {
             self.size_kb / seconds
         )
     }
-}
 
-/// Assert 4-byte-alignment and that subroutine fits inside stack.
-macro_rules! assert_subroutine {
-    ($low_addr:expr, $stack_size:expr, $subroutine_size:expr) => {
-        assert_eq!($low_addr % 4, 0, "low_addr needs to be 4-byte-aligned");
-        assert_eq!($stack_size % 4, 0, "stack_size needs to be 4-byte-aligned");
+    /// Assert 4-byte-alignment and that subroutine fits inside stack.
+    fn assert_subroutines(stack_addr: u32, stack_size: u32) -> Option<()> {
+        assert_eq!(stack_addr % 4, 0, "low_addr needs to be 4-byte-aligned");
+        assert_eq!(stack_size % 4, 0, "stack_size needs to be 4-byte-aligned");
         assert_eq!(
-            $subroutine_size % 4,
+            paint_subroutine::size() % 4,
             0,
-            "subroutine needs to be 4-byte-aligned"
+            "paint subroutine needs to be 4-byte-aligned"
         );
-        assert!(
-            $subroutine_size < $stack_size,
-            "subroutine does not fit inside stack"
+        assert_eq!(
+            measure_subroutine::size() % 4,
+            0,
+            "measure subroutine needs to be 4-byte-aligned"
         );
-    };
+        if (stack_size < paint_subroutine::size()) || (stack_size < measure_subroutine::size()) {
+            log::warn!("subroutines do not fit in stack; not placing stack canary");
+            None
+        } else {
+            Some(())
+        }
+    }
 }
 
 /// Paint-stack subroutine.
@@ -226,7 +233,6 @@ mod paint_subroutine {
     /// paints the whole memory, except of itself. After the subroutine finishes
     /// executing we overwrite the subroutine using the probe.
     pub fn execute(core: &mut Core, low_addr: u32, stack_size: u32) -> Result<(), probe_rs::Error> {
-        assert_subroutine!(low_addr, stack_size, self::SUBROUTINE.len() as u32);
         super::execute_subroutine(core, low_addr, stack_size, self::SUBROUTINE)?;
         self::overwrite_subroutine(core, low_addr)?;
         Ok(())
@@ -247,6 +253,10 @@ mod paint_subroutine {
         0x00, 0xbe, // bkpt     0x0000
         0x00, 0xbe, // bkpt     0x0000 (padding instruction)
     ];
+
+    pub const fn size() -> u32 {
+        self::SUBROUTINE.len() as _
+    }
 }
 
 /// Measure-stack subroutine.
@@ -323,8 +333,6 @@ mod measure_subroutine {
         low_addr: u32,
         stack_size: u32,
     ) -> Result<Option<u32>, probe_rs::Error> {
-        assert_subroutine!(low_addr, stack_size, self::SUBROUTINE.len() as u32);
-
         // use probe to search through the memory the subroutine will be written to
         if let Some(addr) = self::search_with_probe(core, low_addr)? {
             return Ok(Some(addr)); // return early, if we find a touched value
@@ -379,6 +387,10 @@ mod measure_subroutine {
         0x00, 0xbe, // bkpt     0x0000
         0x00, 0xbe, // bkpt     0x0000 (padding instruction)
     ];
+
+    pub const fn size() -> u32 {
+        self::SUBROUTINE.len() as _
+    }
 }
 
 /// Prepare and run subroutine. Also clean up afterwards.

From cb68c28440cd8fcec68240b9f1bd08834c265104 Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Tue, 7 Mar 2023 20:36:30 +0100
Subject: [PATCH 06/10] Update snapshot tests

---
 ...ase_1_successful_run_has_no_backtrace.snap |  3 +-
 .../snapshot__case_2_raw_encoding.snap        |  3 +-
 ..._successful_run_can_enforce_backtrace.snap |  3 +-
 ...hot__case_5_panic_is_reported_as_such.snap |  3 +-
 .../snapshot__case_6_panic_verbose.snap       | 29 ++++++++++---------
 ...successful_run_can_suppress_backtrace.snap |  2 ++
 ...t__ctrl_c_by_user_is_reported_as_such.snap |  3 +-
 7 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/tests/snapshots/snapshot__case_1_successful_run_has_no_backtrace.snap b/tests/snapshots/snapshot__case_1_successful_run_has_no_backtrace.snap
index 5b955dbe..f98a1e13 100644
--- a/tests/snapshots/snapshot__case_1_successful_run_has_no_backtrace.snap
+++ b/tests/snapshots/snapshot__case_1_successful_run_has_no_backtrace.snap
@@ -1,6 +1,6 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 113
+assertion_line: 114
 expression: run_result.output
 
 ---
@@ -9,5 +9,6 @@ expression: run_result.output
 ────────────────────────────────────────────────────────────────────────────────
 Hello, world!
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.16/254.93 KiB (0.1%) of stack space
 (HOST) INFO  device halted without error
 
diff --git a/tests/snapshots/snapshot__case_2_raw_encoding.snap b/tests/snapshots/snapshot__case_2_raw_encoding.snap
index 5b955dbe..d6365197 100644
--- a/tests/snapshots/snapshot__case_2_raw_encoding.snap
+++ b/tests/snapshots/snapshot__case_2_raw_encoding.snap
@@ -1,6 +1,6 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 113
+assertion_line: 114
 expression: run_result.output
 
 ---
@@ -9,5 +9,6 @@ expression: run_result.output
 ────────────────────────────────────────────────────────────────────────────────
 Hello, world!
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.12/254.93 KiB (0.0%) of stack space
 (HOST) INFO  device halted without error
 
diff --git a/tests/snapshots/snapshot__case_3_successful_run_can_enforce_backtrace.snap b/tests/snapshots/snapshot__case_3_successful_run_can_enforce_backtrace.snap
index 3570b3ef..e7a492ec 100644
--- a/tests/snapshots/snapshot__case_3_successful_run_can_enforce_backtrace.snap
+++ b/tests/snapshots/snapshot__case_3_successful_run_can_enforce_backtrace.snap
@@ -1,6 +1,6 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 113
+assertion_line: 114
 expression: run_result.output
 
 ---
@@ -9,6 +9,7 @@ expression: run_result.output
 ────────────────────────────────────────────────────────────────────────────────
 Hello, world!
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.16/254.93 KiB (0.1%) of stack space
 stack backtrace:
    0: lib::inline::__bkpt
         at ./asm/inline.rs:14:5
diff --git a/tests/snapshots/snapshot__case_5_panic_is_reported_as_such.snap b/tests/snapshots/snapshot__case_5_panic_is_reported_as_such.snap
index fe659bf4..ac045cd7 100644
--- a/tests/snapshots/snapshot__case_5_panic_is_reported_as_such.snap
+++ b/tests/snapshots/snapshot__case_5_panic_is_reported_as_such.snap
@@ -1,6 +1,6 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 113
+assertion_line: 114
 expression: run_result.output
 
 ---
@@ -9,6 +9,7 @@ expression: run_result.output
 ────────────────────────────────────────────────────────────────────────────────
 ERROR panicked at 'explicit panic'
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.16/254.93 KiB (0.1%) of stack space
 stack backtrace:
    0: HardFaultTrampoline
       <exception entry>
diff --git a/tests/snapshots/snapshot__case_6_panic_verbose.snap b/tests/snapshots/snapshot__case_6_panic_verbose.snap
index 545c83a0..f306235f 100644
--- a/tests/snapshots/snapshot__case_6_panic_verbose.snap
+++ b/tests/snapshots/snapshot__case_6_panic_verbose.snap
@@ -1,33 +1,34 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 113
+assertion_line: 114
 expression: run_result.output
 
 ---
+(HOST) DEBUG found 1 probes
+(HOST) DEBUG opened probe
+(HOST) DEBUG started session
+(HOST) INFO  flashing program (2 pages / 8.00 KiB)
+(HOST) DEBUG Erased sector of size 4096 bytes in 122 ms
+(HOST) DEBUG Erased sector of size 4096 bytes in 101 ms
+(HOST) DEBUG Programmed page of size 4096 bytes in 66 ms
+(HOST) DEBUG Programmed page of size 4096 bytes in 67 ms
+(HOST) INFO  success!
 (HOST) DEBUG vector table: VectorTable { initial_stack_pointer: 2003fbc0, hard_fault: 17d3 }
 (HOST) DEBUG RAM region: 0x20000000-0x2003FFFF
 (HOST) DEBUG section `.data` is in RAM at 0x2003FBC0..=0x2003FBF7
 (HOST) DEBUG section `.bss` is in RAM at 0x2003FBF8..=0x2003FBFF
 (HOST) DEBUG section `.uninit` is in RAM at 0x2003FC00..=0x2003FFFF
 (HOST) DEBUG valid SP range: 0x20000000..=0x2003FBBC
-(HOST) DEBUG found 1 probes
-(HOST) DEBUG opened probe
-(HOST) DEBUG started session
-(HOST) INFO  flashing program (2 pages / 8.00 KiB)
-(HOST) DEBUG Erased sector of size 4096 bytes in 121 ms
-(HOST) DEBUG Erased sector of size 4096 bytes in 99 ms
-(HOST) DEBUG Programmed page of size 4096 bytes in 62 ms
-(HOST) DEBUG Programmed page of size 4096 bytes in 62 ms
-(HOST) INFO  success!
-(HOST) DEBUG 261052 bytes of stack available (0x20000000 ..= 0x2003FBBC), using 1024 byte canary
-(HOST) TRACE setting up canary took 0.020s (51.19 KiB/s)
+(HOST) DEBUG 261052 bytes of stack available (0x20000000 ..= 0x2003FBBC)
+(HOST) DEBUG painting 254.93 KiB of RAM took 0.031s (8287.09 KiB/s)
 (HOST) DEBUG starting device
 (HOST) DEBUG Successfully attached RTT
 ────────────────────────────────────────────────────────────────────────────────
 ERROR panicked at 'explicit panic'
 ────────────────────────────────────────────────────────────────────────────────
-(HOST) TRACE reading canary took 0.024s (41.48 KiB/s)
-(HOST) DEBUG stack canary intact
+(HOST) DEBUG reading 254.93 KiB of RAM took 0.048s (5308.39 KiB/s)
+(HOST) DEBUG stack was touched at 0x2003FB20
+(HOST) INFO  program has used at least 0.16/254.93 KiB (0.1%) of stack space
 (HOST) TRACE 0x000017d2: found FDE for 0x000017d2 .. 0x000017ea at offset 5672
 (HOST) TRACE uwt row for pc 0x000017d2: UnwindTableRow { start_address: 6098, end_address: 6122, saved_args_size: 0, cfa: RegisterAndOffset { register: Register(13), offset: 0 }, registers: RegisterRuleMap { rules: [] } }
 (HOST) DEBUG LR=0xFFFFFFF9 PC=0x000017D2
diff --git a/tests/snapshots/snapshot__case_7_unsuccessful_run_can_suppress_backtrace.snap b/tests/snapshots/snapshot__case_7_unsuccessful_run_can_suppress_backtrace.snap
index d0dcc24f..580c4546 100644
--- a/tests/snapshots/snapshot__case_7_unsuccessful_run_can_suppress_backtrace.snap
+++ b/tests/snapshots/snapshot__case_7_unsuccessful_run_can_suppress_backtrace.snap
@@ -1,5 +1,6 @@
 ---
 source: tests/snapshot.rs
+assertion_line: 114
 expression: run_result.output
 
 ---
@@ -8,5 +9,6 @@ expression: run_result.output
 ────────────────────────────────────────────────────────────────────────────────
 ERROR panicked at 'explicit panic'
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.16/254.93 KiB (0.1%) of stack space
 (HOST) ERROR the program panicked
 
diff --git a/tests/snapshots/snapshot__ctrl_c_by_user_is_reported_as_such.snap b/tests/snapshots/snapshot__ctrl_c_by_user_is_reported_as_such.snap
index 60a6f287..b4b092cc 100644
--- a/tests/snapshots/snapshot__ctrl_c_by_user_is_reported_as_such.snap
+++ b/tests/snapshots/snapshot__ctrl_c_by_user_is_reported_as_such.snap
@@ -1,6 +1,6 @@
 ---
 source: tests/snapshot.rs
-assertion_line: 123
+assertion_line: 124
 expression: run_result.output
 
 ---
@@ -8,6 +8,7 @@ expression: run_result.output
 (HOST) INFO  success!
 ────────────────────────────────────────────────────────────────────────────────
 ────────────────────────────────────────────────────────────────────────────────
+(HOST) INFO  program has used at least 0.13/254.93 KiB (0.1%) of stack space
 stack backtrace:
    0: silent_loop::__cortex_m_rt_main
         at /tmp/app/src/bin/silent-loop.rs:9:5

From 907821be147d9569e4c7e999467acc2149b55d07 Mon Sep 17 00:00:00 2001
From: Johann Hemmann <johann.hemmann@code.berlin>
Date: Tue, 13 Jun 2023 16:32:30 +0200
Subject: [PATCH 07/10] Update docs

---
 src/canary.rs | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/canary.rs b/src/canary.rs
index 44664d89..17963475 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -15,33 +15,40 @@ const CANARY_U32: u32 = u32::from_le_bytes([CANARY_U8, CANARY_U8, CANARY_U8, CAN
 
 /// (Location of) the stack canary
 ///
-/// The stack canary is used to detect *potential* stack overflows
+/// The stack canary is used to detect *potential* stack overflows and report the
+/// amount of stack used.
 ///
-/// The canary is placed in memory as shown in the diagram below:
+/// The whole stack is initialized to `CANARY_U8` before the target program is started.
 ///
+/// When the programs ends (due to panic or breakpoint) the size of the canary is checked. If more
+/// than 90%  is "touched" (bytes != `CANARY_U8`) then that is considered to be a *potential* stack
+/// overflow. In any case the amount of used stack is reported.
+///
+/// Before execution:
 /// ``` text
-/// +--------+ -> initial_stack_pointer / stack_range.end()
+/// +--------+ -> stack_range.end() (initial_stack_pointer)
 /// |        |
-/// | stack  | (grows downwards)
 /// |        |
-/// +--------+
+/// | canary |
 /// |        |
 /// |        |
+/// +--------+ -> stack_range.start()
+/// | static | (variables, fixed size)
+/// +--------+ -> lowest RAM address
+/// ```
+///
+/// After execution:
+/// ``` text
+/// +--------+ -> stack_range.end() (initial_stack_pointer)
+/// |        |
+/// | stack  | (grows downwards)
+/// |        |
 /// +--------+
 /// | canary |
 /// +--------+ -> stack_range.start()
-/// |        |
 /// | static | (variables, fixed size)
-/// |        |
 /// +--------+ -> lowest RAM address
 /// ```
-///
-/// The whole canary is initialized to `CANARY_U8` before the target program is started.
-/// The canary size is 10% of the available stack space or 1 KiB, whichever is smallest.
-///
-/// When the programs ends (due to panic or breakpoint) the integrity of the canary is checked. If it was
-/// "touched" (any of its bytes != `CANARY_U8`) then that is considered to be a *potential* stack
-/// overflow.
 #[derive(Clone, Copy)]
 pub struct Canary {
     addr: u32,

From 30f2284d080a4a1d2d57894a3172e6397c6d63d1 Mon Sep 17 00:00:00 2001
From: Urhengulas <johann.hemmann@code.berlin>
Date: Wed, 22 Feb 2023 13:22:27 +0100
Subject: [PATCH 08/10] Update `CHANGELOG.md`

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d82a825..85fae69f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,10 +6,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 ## [Unreleased]
 
+- [#410] Simplify canary
 - [#405] Also enable merge queue for changelog enforcer
 - [#404] Switch from bors to merge queue
 - [#402] Do not panic if locations do not contain the frame
 
+[#410]: https://github.com/knurling-rs/probe-run/pull/410
 [#405]: https://github.com/knurling-rs/probe-run/pull/405
 [#404]: https://github.com/knurling-rs/probe-run/pull/404
 [#402]: https://github.com/knurling-rs/probe-run/pull/402

From 27082f07030dce2750d3812be993fab88160e8cd Mon Sep 17 00:00:00 2001
From: Johann Hemmann <johann.hemmann@code.berlin>
Date: Thu, 22 Jun 2023 11:16:07 +0200
Subject: [PATCH 09/10] Fix typo

---
 src/canary.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/canary.rs b/src/canary.rs
index 17963475..6e568644 100644
--- a/src/canary.rs
+++ b/src/canary.rs
@@ -404,7 +404,7 @@ mod measure_subroutine {
 ///
 /// # How?
 ///
-/// We place the parameters in the registers (see table below), place the subroutien
+/// We place the parameters in the registers (see table below), place the subroutine
 /// in memory, set the program counter to the beginning of the subroutine, execute
 /// the subroutine and reset the program counter afterwards.
 ///

From 1b2140b9b87d5cf24bf94c2b201e93ca1d55217c Mon Sep 17 00:00:00 2001
From: Johann Hemmann <johann.hemmann@code.berlin>
Date: Thu, 22 Jun 2023 11:17:31 +0200
Subject: [PATCH 10/10] Improve warning message

---
 src/cli.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cli.rs b/src/cli.rs
index 2314f56e..4ec4c162 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -124,7 +124,7 @@ pub fn handle_arguments() -> anyhow::Result<i32> {
     });
 
     if opts.measure_stack {
-        log::warn!("use of deprecated option `--measure-stack`: Has no effect anymore and will vanish on next minor release")
+        log::warn!("use of deprecated option `--measure-stack`: Has no effect and will vanish on next breaking release")
     }
 
     if opts.version {