From 2059901800467e78bf5aa661764e9445e22bfbf9 Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Fri, 1 May 2026 13:33:09 -0400 Subject: [PATCH] Move registers out of HubrisArchive --- humility-cli/src/lib.rs | 9 ++--- humility-core/src/core.rs | 7 ++-- humility-core/src/dump.rs | 72 ++++++++++++++++++++++++++++++++----- humility-core/src/hubris.rs | 56 +++++------------------------ 4 files changed, 76 insertions(+), 68 deletions(-) diff --git a/humility-cli/src/lib.rs b/humility-cli/src/lib.rs index deaaba711..16a61dc61 100644 --- a/humility-cli/src/lib.rs +++ b/humility-cli/src/lib.rs @@ -278,10 +278,7 @@ impl Cli { validate: Option, ) -> Result> { let mut core = if let Some(dump) = &self.dump { - let Some(hubris) = hubris else { - bail!("cannot load dump without archive"); - }; - humility::core::attach_dump(dump, hubris)? + humility::core::attach_dump(dump)? } else if let Some(ip) = &self.ip { let Some(hubris) = hubris else { bail!("cannot connect over the network without archive"); @@ -328,9 +325,9 @@ impl Cli { /// Attaches to a dump /// /// Reads from the `--dump` argument to pick a target file - pub fn attach_dump(&self, hubris: &HubrisArchive) -> Result> { + pub fn attach_dump(&self) -> Result> { let core = if let Some(dump) = &self.dump { - humility::core::attach_dump(dump, hubris)? + humility::core::attach_dump(dump)? } else { bail!("must be run against a dump"); }; diff --git a/humility-core/src/core.rs b/humility-core/src/core.rs index 98a21cc3d..883ef9b03 100644 --- a/humility-core/src/core.rs +++ b/humility-core/src/core.rs @@ -129,11 +129,8 @@ pub enum NetAgent { Hiffy, } -pub fn attach_dump( - dump: &str, - hubris: &HubrisArchive, -) -> Result> { - let core = DumpCore::new(dump, hubris)?; +pub fn attach_dump(dump: &str) -> Result> { + let core = DumpCore::new(dump)?; crate::msg!("attached to dump"); Ok(Box::new(core)) } diff --git a/humility-core/src/dump.rs b/humility-core/src/dump.rs index 06790c4bc..261292376 100644 --- a/humility-core/src/dump.rs +++ b/humility-core/src/dump.rs @@ -3,10 +3,11 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use crate::core::Core; -use crate::hubris::HubrisArchive; -use anyhow::{Result, anyhow, bail}; +use crate::hubris::OXIDE_NT_HUBRIS_REGISTERS; +use anyhow::{Context, Result, anyhow, bail}; use goblin::elf::Elf; use humility_arch_arm::ARMRegister; +use num_traits::FromPrimitive; use std::collections::{BTreeMap, HashMap}; use std::fs::File; use std::io::Read; @@ -15,11 +16,11 @@ use std::path::Path; pub struct DumpCore { contents: Vec, regions: BTreeMap, - registers: HashMap, + registers: Option>, } impl DumpCore { - pub(crate) fn new(dump: &str, hubris: &HubrisArchive) -> Result { + pub(crate) fn new(dump: &str) -> Result { let mut file = File::open(dump)?; let mut regions = BTreeMap::new(); @@ -41,7 +42,23 @@ impl DumpCore { ); } - Ok(Self { contents, regions, registers: hubris.dump_registers() }) + let mut registers = None; + if let Some(notes) = elf.iter_note_headers(&contents) { + for note in notes { + let note = note.context("failed to parse note")?; + if note.n_type == OXIDE_NT_HUBRIS_REGISTERS { + if registers.is_some() { + bail!( + "multiple copies of OXIDE_NT_HUBRIS_REGISTERS \ + found in dump" + ); + } + registers = Some(load_registers(note.desc)?); + } + } + } + + Ok(Self { contents, regions, registers }) } fn check_offset(&self, addr: u32, rsize: usize, offs: usize) -> Result<()> { @@ -67,6 +84,38 @@ impl DumpCore { } } +fn load_registers(r: &[u8]) -> Result> { + if !r.len().is_multiple_of(8) { + bail!("bad length {} in registers note", r.len()); + } + let mut registers = HashMap::new(); + for (i, chunk) in r.chunks_exact(8).enumerate() { + let (id, val) = chunk.split_at(4); + // We unwrap here because it can only fail if the length is wrong, + // but we've explicitly broken a chunk of 8 into two chunks of 4, + // so a failure here would mean this code has been changed. + let id = u32::from_le_bytes(id.try_into().unwrap()); + let val = u32::from_le_bytes(val.try_into().unwrap()); + + let reg = match ARMRegister::from_u32(id) { + Some(r) => r, + None => { + // This can totally happen if we encounter a future coredump + // where we decided to store, say, additional MSRs or a + // floating point register. Since this version of Humility + // doesn't understand them, we'll just skip it. + continue; + } + }; + + if registers.insert(reg, val).is_some() { + bail!("duplicate register {} ({}) at offset {}", reg, id, i * 8); + } + } + + Ok(registers) +} + #[rustfmt::skip::macros(bail)] impl Core for DumpCore { fn info(&self) -> (String, Option) { @@ -114,10 +163,15 @@ impl Core for DumpCore { } fn read_reg(&mut self, reg: ARMRegister) -> Result { - if let Some(val) = self.registers.get(®) { - Ok(*val) - } else { - bail!("register {} not found in dump", reg); + match &self.registers { + Some(regs) => { + if let Some(val) = regs.get(®) { + Ok(*val) + } else { + bail!("register {} not found in dump", reg); + } + } + None => bail!("dump does not include register info"), } } diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index 1b0139b1b..ccb0ee80c 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -33,11 +33,11 @@ use rustc_demangle::demangle; use scroll::{IOwrite, Pwrite}; use zerocopy::{FromBytes, IntoBytes}; -const OXIDE_NT_NAME: &str = "Oxide Computer Company"; -const OXIDE_NT_BASE: u32 = 0x1de << 20; -const OXIDE_NT_HUBRIS_ARCHIVE: u32 = OXIDE_NT_BASE + 1; -const OXIDE_NT_HUBRIS_REGISTERS: u32 = OXIDE_NT_BASE + 2; -const OXIDE_NT_HUBRIS_TASK: u32 = OXIDE_NT_BASE + 3; +pub const OXIDE_NT_NAME: &str = "Oxide Computer Company"; +pub const OXIDE_NT_BASE: u32 = 0x1de << 20; +pub const OXIDE_NT_HUBRIS_ARCHIVE: u32 = OXIDE_NT_BASE + 1; +pub const OXIDE_NT_HUBRIS_REGISTERS: u32 = OXIDE_NT_BASE + 2; +pub const OXIDE_NT_HUBRIS_TASK: u32 = OXIDE_NT_BASE + 3; const MAX_HUBRIS_VERSION: u32 = 11; @@ -715,9 +715,6 @@ pub struct HubrisArchive { // Manual stack pushes before a syscall syscall_pushes: HashMap>>, - // Current registers (if a dump) - registers: HashMap, - // Modules: text address to module modules: BTreeMap, @@ -810,7 +807,6 @@ impl HubrisArchive { task_dump: None, instrs: HashMap::new(), syscall_pushes: HashMap::new(), - registers: HashMap::new(), modules: BTreeMap::new(), tasks: HashMap::new(), frames: HashMap::new(), @@ -1658,38 +1654,6 @@ impl HubrisArchive { flash.chip } - fn load_registers(&mut self, r: &[u8]) -> Result<()> { - if !r.len().is_multiple_of(8) { - bail!("bad length {} in registers note", r.len()); - } - - for (i, chunk) in r.chunks_exact(8).enumerate() { - let (id, val) = chunk.split_at(4); - // We unwrap here because it can only fail if the length is wrong, - // but we've explicitly broken a chunk of 8 into two chunks of 4, - // so a failure here would mean this code has been changed. - let id = u32::from_le_bytes(id.try_into().unwrap()); - let val = u32::from_le_bytes(val.try_into().unwrap()); - - let reg = match ARMRegister::from_u32(id) { - Some(r) => r, - None => { - // This can totally happen if we encounter a future coredump - // where we decided to store, say, additional MSRs or a - // floating point register. Since this version of Humility - // doesn't understand them, we'll just skip it. - continue; - } - }; - - if self.registers.insert(reg, val).is_some() { - bail!("duplicate register {} ({}) at offset {}", reg, id, i * 8); - } - } - - Ok(()) - } - /// Destroys the `HubrisArchive`, returning the raw archive data pub fn take_raw_archive(self) -> Vec { self.archive @@ -1724,9 +1688,6 @@ impl HubrisArchive { self.archive = note.desc.to_vec(); } - OXIDE_NT_HUBRIS_REGISTERS => { - self.load_registers(note.desc)?; - } OXIDE_NT_HUBRIS_TASK => { match DumpTask::read_from_prefix(note.desc) { Ok((task, _)) => { @@ -1740,6 +1701,9 @@ impl HubrisArchive { } } } + OXIDE_NT_HUBRIS_REGISTERS => { + // unused when building the archive, but valid + } _ => { bail!("unrecognized note 0x{:x}", note.n_type); } @@ -2726,10 +2690,6 @@ impl HubrisArchive { Ok(regions) } - pub fn dump_registers(&self) -> HashMap { - self.registers.clone() - } - pub fn registers( &self, core: &mut dyn crate::core::Core,