Repository: probe-rs/probe-rs (rust)

Issue:
probe-rs run: missing stack trace on hardfault
### Describe the bug

As of #3003 (bisected to 77e9f3253bc5b851807ba759c3a7c9839d703e69), `probe-rs run` no longer automatically prints a stack trace on hardfault.

A stack trace is correctly printed with `--always-print-stacktrace`.

### To Reproduce

1. `probe-rs run` a program which hardfaults (e.g. https://github.com/jackwilsdon/rp2040-panic-issue, which uses `panic_probe` and panics).
2. Note that it does not output a stack trace on hardfault:
   ```text
   $ probe-rs run --chip RP2040 --protocol swd target/thumbv6m-none-eabi/debug/rp2040-project-template --catch-hardfault
         Erasing ✔ 100% [####################]  12.00 KiB @  46.34 KiB/s (took 0s)
     Programming ✔ 100% [####################]  12.00 KiB @  29.16 KiB/s (took 0s)
        Finished in 0.74s
   INFO  Program start
   └─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:27  
   INFO  on!
   └─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:68  
   INFO  off!
   └─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:71  
   ERROR panicked at src/main.rs:74:9:
   at the disco
   └─ panic_probe::print_defmt::print @ /home/jack/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/panic-probe-1.0.0/src/lib.rs:104 
   Error: CPU halted unexpectedly.
   ```
   **Note**: as of #3140 (bisected to a9872509e72884c6a28b4043328b6a704abb5467), the last message before the halt appears to be printed inconsistently, I've raised #3308 for this.

### Expected behavior

A stack trace should be printed on hardfault:
```text
$ probe-rs run --chip RP2040 --protocol swd target/thumbv6m-none-eabi/debug/rp2040-project-template --cat
ch-hardfault 
      Erasing ✔ 100% [####################]  12.00 KiB @  45.60 KiB/s (took 0s)
  Programming ✔ 100% [####################]  12.00 KiB @  32.05 KiB/s (took 0s)
     Finished in 0.64s
INFO  Program start
└─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:27  
INFO  on!
└─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:68  
INFO  off!
└─ rp2040_project_template::__cortex_m_rt_main @ src/main.rs:71  
ERROR panicked at src/main.rs:74:9:
at the disco
└─ panic_probe::print_defmt::print @ /home/jack/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/panic-probe-1.0.0/src/lib.rs:104 
Frame 0: HardFault_ @ 0x10002684
       /home/jack/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cortex-m-rt-0.7.5/src/lib.rs:1105:5
Frame 1: HardFault @ 0x10000e5c
Frame 2: __udf @ 0x10000e5c inline
       ./asm/lib.rs:48:1
Frame 3: __udf @ 0x0000000010000e5c
       ./asm/lib.rs:51:17
Frame 4: delay_us @ 0x10000d74
       /home/jack/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cortex-m-0.7.7/src/delay.rs:40:1
Error: CPU halted unexpectedly.
```
**Note**: the above stack trace is not correct for the program that was run, I've raised #3309 for this.

### Operating System

Other Linux Distribution

Pull request description:
Separate probing errors from firmware exits
This PR separates errors and abnormal firmware exit conditions in `monitor`. This technically closes #3307 although the solution is "good enough" as opposed to "good".

Patch:
diff --git a/probe-rs-tools/src/bin/probe-rs/rpc/client.rs b/probe-rs-tools/src/bin/probe-rs/rpc/client.rs
index 054e094d..422f5192 100644
--- a/probe-rs-tools/src/bin/probe-rs/rpc/client.rs
+++ b/probe-rs-tools/src/bin/probe-rs/rpc/client.rs
@@ -45,7 +45,7 @@ use crate::{
             },
             info::{InfoEvent, TargetInfoRequest},
             memory::{ReadMemoryRequest, WriteMemoryRequest},
-            monitor::{MonitorMode, MonitorOptions, MonitorRequest},
+            monitor::{MonitorExitReason, MonitorMode, MonitorOptions, MonitorRequest},
             probe::{
                 AttachRequest, AttachResult, DebugProbeEntry, DebugProbeSelector,
                 ListProbesRequest, SelectProbeRequest, SelectProbeResult,
@@ -501,7 +501,7 @@ impl SessionInterface {
         mode: MonitorMode,
         options: MonitorOptions,
         on_msg: impl AsyncFnMut(MonitorEvent),
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<MonitorExitReason> {
         self.client
             .send_and_read_stream::<MonitorEndpoint, MonitorEvent, _>(
                 &MonitorRequest {
diff --git a/probe-rs-tools/src/bin/probe-rs/rpc/functions.rs b/probe-rs-tools/src/bin/probe-rs/rpc/functions.rs
index 2a911ffd..8b8ad9cb 100644
--- a/probe-rs-tools/src/bin/probe-rs/rpc/functions.rs
+++ b/probe-rs-tools/src/bin/probe-rs/rpc/functions.rs
@@ -4,8 +4,6 @@ use std::{convert::Infallible, future::Future};
 use crate::rpc::functions::file::{
     AppendFileRequest, CreateFileResponse, append_temp_file, create_temp_file,
 };
-use crate::rpc::functions::probe::{SelectProbeRequest, SelectProbeResponse, select_probe};
-use crate::rpc::transport::memory::{WireRx, WireTx};
 use crate::{
     rpc::{
         Key, SessionState,
@@ -20,10 +18,10 @@ use crate::{
             },
             info::{InfoEvent, TargetInfoRequest, target_info},
             memory::{ReadMemoryRequest, WriteMemoryRequest, read_memory, write_memory},
-            monitor::{MonitorRequest, RttEvent, SemihostingEvent, monitor},
+            monitor::{MonitorRequest, MonitorResponse, RttEvent, SemihostingEvent, monitor},
             probe::{
-                AttachRequest, AttachResponse, ListProbesRequest, ListProbesResponse, attach,
-                list_probes,
+                AttachRequest, AttachResponse, ListProbesRequest, ListProbesResponse,
+                SelectProbeRequest, SelectProbeResponse, attach, list_probes, select_probe,
             },
             reset::{ResetCoreRequest, reset},
             resume::{ResumeAllCoresRequest, resume_all_cores},
@@ -34,6 +32,7 @@ use crate::{
                 run_test,
             },
         },
+        transport::memory::{WireRx, WireTx},
     },
     util::common_options::OperationError,
 };
@@ -467,7 +466,7 @@ endpoints! {
     | FlashEndpoint             | FlashRequest           | NoResponse              | "flash/flash"      |
     | EraseEndpoint             | EraseRequest           | NoResponse              | "flash/erase"      |
     | VerifyEndpoint            | VerifyRequest          | VerifyResponse          | "flash/verify"     |
-    | MonitorEndpoint           | MonitorRequest         | NoResponse              | "monitor"          |
+    | MonitorEndpoint           | MonitorRequest         | MonitorResponse         | "monitor"          |
 
     | ListTestsEndpoint         | ListTestsRequest       | ListTestsResponse       | "tests/list"       |
     | RunTestEndpoint           | RunTestRequest         | RunTestResponse         | "tests/run"        |
diff --git a/probe-rs-tools/src/bin/probe-rs/rpc/functions/monitor.rs b/probe-rs-tools/src/bin/probe-rs/rpc/functions/monitor.rs
index c0ffeb1d..49c5c4fe 100644
--- a/probe-rs-tools/src/bin/probe-rs/rpc/functions/monitor.rs
+++ b/probe-rs-tools/src/bin/probe-rs/rpc/functions/monitor.rs
@@ -4,10 +4,10 @@ use crate::{
     rpc::{
         Key,
         functions::{
-            MonitorEndpoint, MultiTopicPublisher, MultiTopicWriter, RpcSpawnContext, RttTopic,
-            SemihostingTopic, WireTxImpl, flash::BootInfo,
+            MonitorEndpoint, MultiTopicPublisher, MultiTopicWriter, RpcResult, RpcSpawnContext,
+            RttTopic, SemihostingTopic, WireTxImpl, flash::BootInfo,
         },
-        utils::run_loop::{RunLoop, RunLoopPoller},
+        utils::run_loop::{ReturnReason, RunLoop, RunLoopPoller},
     },
     util::rtt::client::RttClient,
 };
@@ -63,6 +63,27 @@ pub struct MonitorRequest {
     pub options: MonitorOptions,
 }
 
+/// Reasons why the firmware exited.
+#[derive(Serialize, Deserialize, Schema)]
+pub enum MonitorExitReason {
+    Success,
+    SemihostingExit(Result<(), SemihostingExitError>),
+    UnexpectedExit(String),
+}
+
+/// Details of an unexpected exit, triggered by a semihosting call.
+#[derive(Serialize, Deserialize, Schema)]
+pub struct SemihostingExitError {
+    /// The reason for the exit.
+    pub reason: u32,
+    /// The subcode of the exit, if the call was EXIT_EXTENDED.
+    pub subcode: Option<u32>,
+}
+
+/// If a communication error occurs, an error is returned. If we detect that the firmware exited,
+/// a `MonitorExitReason` is returned.
+pub type MonitorResponse = RpcResult<MonitorExitReason>;
+
 pub async fn monitor(
     mut ctx: RpcSpawnContext,
     header: VarHeader,
@@ -153,7 +174,7 @@ fn monitor_impl(
     ctx: RpcSpawnContext,
     request: MonitorRequest,
     sender: MonitorSender,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<MonitorExitReason> {
     let mut session = ctx.session_blocking(request.sessid);
 
     let mut semihosting_sink =
@@ -189,7 +210,7 @@ fn monitor_impl(
         },
     });
 
-    run_loop.run_until(
+    let exit_reason = run_loop.run_until(
         &mut core,
         request.options.catch_hardfault,
         request.options.catch_reset,
@@ -198,7 +219,12 @@ fn monitor_impl(
         |halt_reason, core| semihosting_sink.handle_halt(halt_reason, core),
     )?;
 
-    Ok(())
+    match exit_reason {
+        ReturnReason::Predicate(reason) => Ok(reason),
+        ReturnReason::Timeout => anyhow::bail!("Run loop exited due to an unexpected timeout"),
+        ReturnReason::Cancelled => anyhow::bail!("Exited due to user request"),
+        ReturnReason::LockedUp => anyhow::bail!("Run loop exited due to a locked up core"),
+    }
 }
 
 pub struct RttPoller<'c, S>
@@ -283,16 +309,22 @@ impl<F: FnMut(SemihostingEvent)> MonitorEventHandler<F> {
         &mut self,
         halt_reason: HaltReason,
         core: &mut Core<'_>,
-    ) -> anyhow::Result<Option<()>> {
+    ) -> anyhow::Result<Option<MonitorExitReason>> {
         let HaltReason::Breakpoint(BreakpointCause::Semihosting(cmd)) = halt_reason else {
-            anyhow::bail!("CPU halted unexpectedly. Halt reason: {halt_reason:?}");
+            return Ok(Some(MonitorExitReason::UnexpectedExit(format!(
+                "{:?}",
+                halt_reason
+            ))));
         };
 
         match cmd {
-            SemihostingCommand::ExitSuccess => Ok(Some(())), // Exit the run loop
-            SemihostingCommand::ExitError(details) => {
-                anyhow::bail!("Semihosting indicated exit with {details}")
-            }
+            SemihostingCommand::ExitSuccess => Ok(Some(MonitorExitReason::SemihostingExit(Ok(())))), // Exit the run loop
+            SemihostingCommand::ExitError(details) => Ok(Some(MonitorExitReason::SemihostingExit(
+                Err(SemihostingExitError {
+                    reason: details.reason,
+                    subcode: details.exit_status.or(details.subcode),
+                }),
+            ))),
             SemihostingCommand::Unknown(details) => {
                 tracing::warn!(
                     "Target wanted to run semihosting operation {:#x} with parameter {:#x},\
@@ -315,7 +347,10 @@ impl<F: FnMut(SemihostingEvent)> MonitorEventHandler<F> {
                 }
                 Ok(None)
             }
-            other => anyhow::bail!("Unexpected semihosting command {:?}", other),
+            other => Ok(Some(MonitorExitReason::UnexpectedExit(format!(
+                "Unexpected semihosting command {:?}",
+                other,
+            )))),
         }
     }
 }
diff --git a/probe-rs-tools/src/bin/probe-rs/rpc/utils/run_loop.rs b/probe-rs-tools/src/bin/probe-rs/rpc/utils/run_loop.rs
index 9509ca6e..bb507ac4 100644
--- a/probe-rs-tools/src/bin/probe-rs/rpc/utils/run_loop.rs
+++ b/probe-rs-tools/src/bin/probe-rs/rpc/utils/run_loop.rs
@@ -20,6 +20,8 @@ pub enum ReturnReason<R> {
     Timeout,
     /// Cancelled
     Cancelled,
+    /// The core locked up
+    LockedUp,
 }
 
 impl RunLoop {
@@ -124,26 +126,27 @@ impl RunLoop {
         // check for halt first, poll rtt after.
         // this is important so we do one last poll after halt, so we flush all messages
         // the core printed before halting, such as a panic message.
-        let mut return_reason = None;
-        match core.status()? {
+        let return_reason = match core.status()? {
             probe_rs::CoreStatus::Halted(reason) => match predicate(reason, core) {
-                Ok(Some(r)) => return_reason = Some(Ok(ReturnReason::Predicate(r))),
-                Err(e) => return_reason = Some(Err(e)),
+                Ok(Some(r)) => Some(Ok(ReturnReason::Predicate(r))),
+                Err(e) => Some(Err(e)),
                 Ok(None) => {
                     // Poll at 1kHz if the core was halted, to speed up reading strings
                     // from semihosting. The core is not expected to be halted for other reasons.
                     next_poll = Duration::from_millis(1);
                     core.run()?;
+                    None
                 }
             },
             probe_rs::CoreStatus::Running
             | probe_rs::CoreStatus::Sleeping
             | probe_rs::CoreStatus::Unknown => {
                 // Carry on
+                None
             }
 
-            probe_rs::CoreStatus::LockedUp => anyhow::bail!("The core is locked up."),
-        }
+            probe_rs::CoreStatus::LockedUp => Some(Ok(ReturnReason::LockedUp)),
+        };
 
         let poller_result = poller.poll(core);
 
diff --git a/probe-rs-tools/src/bin/probe-rs/util/cli.rs b/probe-rs-tools/src/bin/probe-rs/util/cli.rs
index 7bb3786a..c72eb25c 100644
--- a/probe-rs-tools/src/bin/probe-rs/util/cli.rs
+++ b/probe-rs-tools/src/bin/probe-rs/util/cli.rs
@@ -13,6 +13,7 @@ use tokio::io::AsyncWriteExt;
 use tokio::{runtime::Handle, sync::mpsc::UnboundedSender};
 use tokio_util::sync::CancellationToken;
 
+use crate::rpc::functions::monitor::MonitorExitReason;
 use crate::{
     FormatOptions,
     rpc::{
@@ -443,11 +444,61 @@ pub async fn monitor(
     })
     .await;
 
+    let print_stack_trace = match &result {
+        Ok(MonitorExitReason::Success | MonitorExitReason::SemihostingExit(Ok(_))) => {
+            println!("Firmware exited successfully");
+            print_stack_trace // On success, we only printi the user asked for it.
+        }
+        Ok(MonitorExitReason::UnexpectedExit(reason)) => {
+            println!("Firmware exited unexpectedly: {reason}");
+            true
+        }
+        Ok(MonitorExitReason::SemihostingExit(Err(details))) => {
+            let reason = match details.reason {
+                // HW vector reason codes
+                0x20000 => String::from("Branch through zero"),
+                0x20001 => String::from("Undefined instrution"),
+                0x20002 => String::from("Software interrupt"),
+                0x20003 => String::from("Prefetch abort"),
+                0x20004 => String::from("Data abort"),
+                0x20005 => String::from("Address exception"),
+                0x20006 => String::from("IRQ"),
+                0x20007 => String::from("FIQ"),
+                // SW reason codes
+                0x20020 => String::from("Breakpoint"),
+                0x20021 => String::from("Watchpoint"),
+                0x20022 => String::from("Step complete"),
+                0x20023 => String::from("Unknown runtime error"),
+                0x20024 => String::from("Internal error"),
+                0x20025 => String::from("User interruption"),
+                0x20026 => String::from("Application exit"),
+                0x20027 => String::from("Stack overflow"),
+                0x20028 => String::from("Division by zero"),
+                0x20029 => String::from("OS specific error"),
+                other => format!("Unknown exit reason {other}"),
+            };
+
+            let subcode = match details.reason {
+                0x20026 => match details.subcode {
+                    Some(134) => String::from(" (Aborted)"),
+                    Some(other) => format!(" (Unknown exit code {other})"),
+                    None => String::from(""),
+                },
+                _ => String::from(""),
+            };
+
+            println!("Firmware exited with: {}{}", reason, subcode);
+
+            true
+        }
+        Err(_) => false, // Some irrecoverable error happened, probably can't print the stack trace.
+    };
+
     if print_stack_trace {
         display_stack_trace(session, path).await?;
     }
 
-    result
+    result.map(|_| ())
 }
 
 pub async fn test(


Test patch:
diff --git a/probe-rs-tools/src/bin/probe-rs/rpc/functions/test.rs b/probe-rs-tools/src/bin/probe-rs/rpc/functions/test.rs
index 44fd5aa7..9c532040 100644
--- a/probe-rs-tools/src/bin/probe-rs/rpc/functions/test.rs
+++ b/probe-rs-tools/src/bin/probe-rs/rpc/functions/test.rs
@@ -175,6 +175,9 @@ fn list_tests_impl(
             version: 1,
             tests: vec![],
         }),
+        ReturnReason::LockedUp => {
+            anyhow::bail!("The target locked up while waiting for the test list.")
+        }
     }
 }
 
@@ -266,6 +269,9 @@ fn run_test_impl(
             expected_outcome, outcome
         ))),
         ReturnReason::Cancelled => Ok(TestResult::Cancelled),
+        ReturnReason::LockedUp => {
+            anyhow::bail!("The target locked up while running the test.")
+        }
     }
 }