From 50c4f98cfe7227215b0a8c307092babd5775e647 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Thu, 9 Oct 2025 10:03:25 +0200 Subject: [PATCH 1/3] Some fixes and logs --- trace/src/platform/cortex_m/mod.rs | 46 +++++++++++++++++++++++------- trace/src/platform/mod.rs | 7 +++++ trace/src/variables/mod.rs | 12 ++++---- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/trace/src/platform/cortex_m/mod.rs b/trace/src/platform/cortex_m/mod.rs index 4aab057..93be5de 100644 --- a/trace/src/platform/cortex_m/mod.rs +++ b/trace/src/platform/cortex_m/mod.rs @@ -28,33 +28,54 @@ impl<'data> CortexMPlatform<'data> { device_memory: &mut DeviceMemory<>::Word>, unwind_info: UnwindTableRow, ) -> Result { - let updated = match unwind_info.cfa() { + let cfa = match unwind_info.cfa() { CfaRule::RegisterAndOffset { register, offset } => { - let new_cfa = (device_memory.register(*register)? as i64 + *offset) as u32; - let old_cfa = device_memory.register(gimli::Arm::SP)?; - let changed = new_cfa != old_cfa; - *device_memory.register_mut(gimli::Arm::SP)? = new_cfa; - changed + (device_memory.register(*register)? as i64 + *offset) as u32 } CfaRule::Expression(_) => todo!("CfaRule::Expression"), }; + log::info!("CFA: {:#010X}", cfa); + for i in -4..=0 { + log::info!( + "CFA {}: {:#010X}", + i * 4, + device_memory + .read_u32(cfa.wrapping_add_signed(i * 4) as u64, RunTimeEndian::Little) + .unwrap() + .unwrap() + ); + } + + let mut sp_updated = false; + for (reg, rule) in unwind_info.registers() { + if *reg == gimli::Arm::SP { + sp_updated = true; + } + match rule { RegisterRule::Undefined => unreachable!(), RegisterRule::Offset(offset) => { - let cfa = device_memory.register(gimli::Arm::SP)?; let addr = (i64::from(cfa) + offset) as u64; let new_value = device_memory .read_u32(addr, RunTimeEndian::Little)? .ok_or(TraceError::MissingMemory(addr))?; *device_memory.register_mut(*reg)? = new_value; + log::info!("Changed {:?} to {:#010X}", reg, new_value); } _ => unimplemented!(), } } - Ok(updated) + if !sp_updated { + if device_memory.register(gimli::Arm::SP)? != cfa { + sp_updated = true; + *device_memory.register_mut(gimli::Arm::SP)? = cfa; + } + } + + Ok(sp_updated) } fn is_last_frame( @@ -227,6 +248,8 @@ impl<'data> Platform<'data> for CortexMPlatform<'data> { } }; + log::info!("{unwind_info:#?}"); + // We can update the stackpointer and other registers to the previous frame by applying the unwind info let stack_pointer_changed = match Self::apply_unwind_info(device_memory, unwind_info) { Ok(stack_pointer_changed) => stack_pointer_changed, @@ -264,9 +287,10 @@ impl<'data> Platform<'data> for CortexMPlatform<'data> { line: None, column: None, }, - frame_type: FrameType::Corrupted( - "CFA did not change and LR and PC are equal".into(), - ), + frame_type: FrameType::Corrupted(format!( + "CFA did not change and LR and PC are equal: {:#010X}", + device_memory.register(gimli::Arm::PC)? + )), variables: Vec::new(), }), }); diff --git a/trace/src/platform/mod.rs b/trace/src/platform/mod.rs index 1d4d3af..b77c620 100644 --- a/trace/src/platform/mod.rs +++ b/trace/src/platform/mod.rs @@ -130,6 +130,13 @@ where Err(e) => return Err(e), } + log::info!( + "Current registers:\nSP: {:#010X}\nLR: {:#010X}\nPC: {:#010X}", + device_memory.register(gimli::Arm::SP).unwrap(), + device_memory.register(gimli::Arm::LR).unwrap(), + device_memory.register(gimli::Arm::PC).unwrap() + ); + // Try to unwind match platform_context.unwind(&mut device_memory, frames.last_mut())? { UnwindResult::Finished => { diff --git a/trace/src/variables/mod.rs b/trace/src/variables/mod.rs index 9372565..1bcd491 100644 --- a/trace/src/variables/mod.rs +++ b/trace/src/variables/mod.rs @@ -1252,12 +1252,12 @@ where })) } (Ok(variable_name), Err(type_error)) => { - log::info!( - "Could not read the type of variable `{}` of entry {:X?}: {}", - variable_name, - entry.offset().to_debug_info_offset(&unit.header), - type_error - ); + // log::info!( + // "Could not read the type of variable `{}` of entry {:X?}: {}", + // variable_name, + // entry.offset().to_debug_info_offset(&unit.header), + // type_error + // ); Ok(None) } (Err(name_error), _) => { From 8104357a171ea5282c4a976818c1437b8317431f Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Thu, 9 Oct 2025 15:30:49 +0200 Subject: [PATCH 2/3] Fix off-by-one issue from noreturn functions and remove added logs --- trace/src/platform/cortex_m/mod.rs | 32 ++++++++++-------------------- trace/src/platform/mod.rs | 7 ------- trace/src/variables/mod.rs | 12 +++++------ 3 files changed, 16 insertions(+), 35 deletions(-) diff --git a/trace/src/platform/cortex_m/mod.rs b/trace/src/platform/cortex_m/mod.rs index 93be5de..c8b068e 100644 --- a/trace/src/platform/cortex_m/mod.rs +++ b/trace/src/platform/cortex_m/mod.rs @@ -35,18 +35,6 @@ impl<'data> CortexMPlatform<'data> { CfaRule::Expression(_) => todo!("CfaRule::Expression"), }; - log::info!("CFA: {:#010X}", cfa); - for i in -4..=0 { - log::info!( - "CFA {}: {:#010X}", - i * 4, - device_memory - .read_u32(cfa.wrapping_add_signed(i * 4) as u64, RunTimeEndian::Little) - .unwrap() - .unwrap() - ); - } - let mut sp_updated = false; for (reg, rule) in unwind_info.registers() { @@ -62,17 +50,14 @@ impl<'data> CortexMPlatform<'data> { .read_u32(addr, RunTimeEndian::Little)? .ok_or(TraceError::MissingMemory(addr))?; *device_memory.register_mut(*reg)? = new_value; - log::info!("Changed {:?} to {:#010X}", reg, new_value); } _ => unimplemented!(), } } - if !sp_updated { - if device_memory.register(gimli::Arm::SP)? != cfa { - sp_updated = true; - *device_memory.register_mut(gimli::Arm::SP)? = cfa; - } + if !sp_updated && device_memory.register(gimli::Arm::SP)? != cfa { + sp_updated = true; + *device_memory.register_mut(gimli::Arm::SP)? = cfa; } Ok(sp_updated) @@ -248,8 +233,6 @@ impl<'data> Platform<'data> for CortexMPlatform<'data> { } }; - log::info!("{unwind_info:#?}"); - // We can update the stackpointer and other registers to the previous frame by applying the unwind info let stack_pointer_changed = match Self::apply_unwind_info(device_memory, unwind_info) { Ok(stack_pointer_changed) => stack_pointer_changed, @@ -329,8 +312,13 @@ impl<'data> Platform<'data> for CortexMPlatform<'data> { Err(e) => return Err(e), } } else { - // No exception, so follow the LR back - *device_memory.register_mut(gimli::Arm::PC)? = device_memory.register(gimli::Arm::LR)? + // No exception, so follow the LR back, but one instruction back. + // Sometimes a function will have a `bl` instruction at the end. + // An example is noreturn functions. + // LR always points to the next instruction, which means that it doesn't have to be in the same function as from where the branch originated. + // https://lkml.kernel.org/lkml/20240305175846.qnyiru7uaa7itqba@treble/ + *device_memory.register_mut(gimli::Arm::PC)? = + device_memory.register(gimli::Arm::LR)? - 2; } // Have we reached the reset vector? diff --git a/trace/src/platform/mod.rs b/trace/src/platform/mod.rs index b77c620..1d4d3af 100644 --- a/trace/src/platform/mod.rs +++ b/trace/src/platform/mod.rs @@ -130,13 +130,6 @@ where Err(e) => return Err(e), } - log::info!( - "Current registers:\nSP: {:#010X}\nLR: {:#010X}\nPC: {:#010X}", - device_memory.register(gimli::Arm::SP).unwrap(), - device_memory.register(gimli::Arm::LR).unwrap(), - device_memory.register(gimli::Arm::PC).unwrap() - ); - // Try to unwind match platform_context.unwind(&mut device_memory, frames.last_mut())? { UnwindResult::Finished => { diff --git a/trace/src/variables/mod.rs b/trace/src/variables/mod.rs index 1bcd491..dd6212b 100644 --- a/trace/src/variables/mod.rs +++ b/trace/src/variables/mod.rs @@ -1252,12 +1252,12 @@ where })) } (Ok(variable_name), Err(type_error)) => { - // log::info!( - // "Could not read the type of variable `{}` of entry {:X?}: {}", - // variable_name, - // entry.offset().to_debug_info_offset(&unit.header), - // type_error - // ); + log::debug!( + "Could not read the type of variable `{}` of entry {:X?}: {}", + variable_name, + entry.offset().to_debug_info_offset(&unit.header), + type_error + ); Ok(None) } (Err(name_error), _) => { From 69d68c2d0e45bb964b9a50f75a9322cd2115a432 Mon Sep 17 00:00:00 2001 From: Dion Dokter Date: Thu, 9 Oct 2025 15:38:28 +0200 Subject: [PATCH 3/3] Add to changelog and mark as 0.10.4 --- CHANGELOG.md | 4 ++++ Cargo.lock | 10 +++++----- Cargo.toml | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37c6b22..4a3190c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +## 0.10.4 (09-10-25) + +- Made unwinding more robust, especially around `noreturn` functions + ## 0.10.3 (01-08-25) - Fix panic where a data slice was indexed out of range diff --git a/Cargo.lock b/Cargo.lock index b300845..7f05f56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1738,7 +1738,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "stackdump-capture" -version = "0.10.3" +version = "0.10.4" dependencies = [ "arrayvec", "stackdump-core", @@ -1746,7 +1746,7 @@ dependencies = [ [[package]] name = "stackdump-capture-probe" -version = "0.10.3" +version = "0.10.4" dependencies = [ "probe-rs", "stackdump-core", @@ -1754,7 +1754,7 @@ dependencies = [ [[package]] name = "stackdump-cli" -version = "0.10.3" +version = "0.10.4" dependencies = [ "clap", "colored 3.0.0", @@ -1768,7 +1768,7 @@ dependencies = [ [[package]] name = "stackdump-core" -version = "0.10.3" +version = "0.10.4" dependencies = [ "arrayvec", "funty", @@ -1778,7 +1778,7 @@ dependencies = [ [[package]] name = "stackdump-trace" -version = "0.10.3" +version = "0.10.4" dependencies = [ "addr2line", "bitvec", diff --git a/Cargo.toml b/Cargo.toml index fce76b7..5b7bb14 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [workspace] resolver = "2" -package.version = "0.10.3" +package.version = "0.10.4" members = [ "capture",