Merge #287

287: unwind: skip FDEs with initial address of 0 r=jonathanpallant a=jonas-schievink

The [`panic` example in the app-template](https://github.com/knurling-rs/app-template/blob/09cb051ef0e6863e805a3fc65d509147b031f411/src/bin/panic.rs) can cause a broken backtrace like this:

```
stack backtrace:
   0: 0x00001450 @ HardFaultTrampoline
      <exception entry>
   1: 0x000008d4 @ lib::inline::__udf
        at ./asm/inline.rs:172:5
   2: 0x000008d4 @ __udf
        at ./asm/lib.rs:49:17
   3: 0x000001ba @ cortex_m::asm::udf
        at [...]/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/cortex-m-0.7.3/src/asm.rs:43:5
   4: 0x000001c4 @ _defmt_panic
        at [...]/src/lib.rs:13:5
   5: 0x000001b0 @ defmt::export::panic
        at [...]/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/defmt-0.3.0/src/export/mod.rs:125:14
   6: 0x000001b0 @ panic::__cortex_m_rt_main
        at [...]/src/bin/panic.rs:10:5
   7: 0x00000168 @ main
        at [...]/src/bin/panic.rs:6:1
   8: 0x00000144 @ Reset
   9: 0x00000100 @ Reset
  10: 0x52205244 @ __sheap
```

This appeared to have a different cause from #277, since the fix for that didn't fix the backtrace here.

After a lot of investigation, it turned out that what was going on is that there are multiple frame description entries (FDEs) in the debuginfo that claim to apply to some addresses, and we happened to pick the wrong one while unwinding from `Reset` in frame 8, causing us to restore registers incorrectly and then attempting to jump to a nonsense address.

This seems to be limited to FDEs describing presumably dead code, whose base address gets reset to 0 when they're included in the final ELF, so this PR fixes it by ignoring any FDEs that claim to apply to address 0.

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

src/backtrace/unwind.rs

@@ -1,7 +1,10 @@
 //! unwind target's program
 
 use anyhow::{anyhow, Context as _};
-use gimli::{BaseAddresses, DebugFrame, UninitializedUnwindContext, UnwindSection as _};
+use gimli::{
+    BaseAddresses, CieOrFde, DebugFrame, FrameDescriptionEntry, Reader, UninitializedUnwindContext,
+    UnwindSection as _,
+};
 use probe_rs::{config::RamRegion, Core};
 
 use crate::{
@@ -61,16 +64,19 @@ pub(crate) fn target(core: &mut Core, elf: &Elf, active_ram_region: &Option<RamR
 
         output.raw_frames.push(RawFrame::Subroutine { pc });
 
-        let uwt_row = unwrap_or_return_output!(elf
-            .debug_frame
+        let fde = unwrap_or_return_output!(find_fde(&elf.debug_frame, &base_addresses, pc));
+
+        let uwt_row = unwrap_or_return_output!(fde
             .unwind_info_for_address(
+                &elf.debug_frame,
                 &base_addresses,
                 &mut unwind_context,
-                pc.into(),
-                DebugFrame::cie_from_offset,
+                pc.into()
             )
             .with_context(|| missing_debug_info(pc)));
 
+        log::trace!("uwt row for pc {:#010x}: {:?}", pc, uwt_row);
+
         let cfa_changed = unwrap_or_return_output!(registers.update_cfa(uwt_row.cfa()));
 
         for (reg, rule) in uwt_row.registers() {
@@ -195,3 +201,52 @@ fn overflowed_stack(sp: u32, active_ram_region: &Option<RamRegion>) -> bool {
         false
     }
 }
+
+/// FDEs can never overlap. Unfortunately, computers. It looks like FDEs for dead code might still
+/// end up in the final ELF, but get their offset reset to 0, so there can be overlapping FDEs at
+/// low addresses.
+///
+/// This function finds the FDE that applies to `addr`, skipping any FDEs with a start address of 0.
+/// Since there's no code at address 0, this should never skip legitimate FDEs.
+fn find_fde<R: Reader>(
+    debug_frame: &DebugFrame<R>,
+    bases: &BaseAddresses,
+    addr: u32,
+) -> anyhow::Result<FrameDescriptionEntry<R>> {
+    let mut entries = debug_frame.entries(bases);
+    let mut fdes = Vec::new();
+    while let Some(entry) = entries.next()? {
+        match entry {
+            CieOrFde::Cie(_) => {}
+            CieOrFde::Fde(partial) => {
+                let fde = partial.parse(DebugFrame::cie_from_offset)?;
+                if fde.initial_address() == 0 {
+                    continue;
+                }
+
+                if fde.contains(addr.into()) {
+                    log::trace!(
+                        "{:#010x}: found FDE for {:#010x} .. {:#010x} at offset {:?}",
+                        addr,
+                        fde.initial_address(),
+                        fde.initial_address() + fde.len(),
+                        fde.offset(),
+                    );
+                    fdes.push(fde);
+                }
+            }
+        }
+    }
+
+    match fdes.len() {
+        0 => Err(anyhow!(gimli::Error::NoUnwindInfoForAddress))
+            .with_context(|| missing_debug_info(addr)),
+        1 => Ok(fdes.pop().unwrap()),
+        n => Err(anyhow!(
+            "found {} frame description entries for address {:#010x}, there should only be 1; \
+             this is likely a bug in your compiler toolchain; unwinding will stop here",
+            n,
+            addr
+        )),
+    }
+}

src/registers.rs

@@ -67,7 +67,15 @@ impl<'c, 'probe> Registers<'c, 'probe> {
             RegisterRule::Offset(offset) => {
                 let cfa = self.get(SP)?;
                 let addr = (cfa as i64 + offset) as u32;
-                self.cache.insert(reg.0, self.core.read_word_32(addr)?);
+                let value = self.core.read_word_32(addr)?;
+                log::trace!(
+                    "update reg={:?}, rule={:?}, abs={:#010x} -> value={:#010x}",
+                    reg,
+                    rule,
+                    addr,
+                    value
+                );
+                self.cache.insert(reg.0, value);
             }
             RegisterRule::Undefined => unreachable!(),
             _ => unimplemented!(),