Move registers out of HubrisArchive

Author: mkeeter

humility-cli/src/lib.rs

@@ -278,10 +278,7 @@ impl Cli {
         validate: Option<HubrisValidate>,
     ) -> Result<Box<dyn Core>> {
         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<Box<dyn Core>> {
+    pub fn attach_dump(&self) -> Result<Box<dyn Core>> {
         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");
         };

humility-core/src/core.rs

@@ -129,11 +129,8 @@ pub enum NetAgent {
     Hiffy,
 }
 
-pub fn attach_dump(
-    dump: &str,
-    hubris: &HubrisArchive,
-) -> Result<Box<dyn Core>> {
-    let core = DumpCore::new(dump, hubris)?;
+pub fn attach_dump(dump: &str) -> Result<Box<dyn Core>> {
+    let core = DumpCore::new(dump)?;
     crate::msg!("attached to dump");
     Ok(Box::new(core))
 }

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<u8>,
     regions: BTreeMap<u32, (u32, usize)>,
-    registers: HashMap<ARMRegister, u32>,
+    registers: Option<HashMap<ARMRegister, u32>>,
 }
 
 impl DumpCore {
-    pub(crate) fn new(dump: &str, hubris: &HubrisArchive) -> Result<DumpCore> {
+    pub(crate) fn new(dump: &str) -> Result<DumpCore> {
         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<HashMap<ARMRegister, u32>> {
+    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<String>) {
@@ -114,10 +163,15 @@ impl Core for DumpCore {
     }
 
     fn read_reg(&mut self, reg: ARMRegister) -> Result<u32> {
-        if let Some(val) = self.registers.get(&reg) {
-            Ok(*val)
-        } else {
-            bail!("register {} not found in dump", reg);
+        match &self.registers {
+            Some(regs) => {
+                if let Some(val) = regs.get(&reg) {
+                    Ok(*val)
+                } else {
+                    bail!("register {} not found in dump", reg);
+                }
+            }
+            None => bail!("dump does not include register info"),
         }
     }
 

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;
 
@@ -743,9 +743,6 @@ pub struct HubrisArchive {
     // Manual stack pushes before a syscall
     syscall_pushes: HashMap<u32, Option<Vec<ARMRegister>>>,
 
-    // Current registers (if a dump)
-    registers: HashMap<ARMRegister, u32>,
-
     // Modules: text address to module
     modules: BTreeMap<u32, HubrisModule>,
 
@@ -838,7 +835,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(),
@@ -1784,38 +1780,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<u8> {
         self.archive
@@ -1850,9 +1814,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, _)) => {
@@ -1866,6 +1827,9 @@ impl HubrisArchive {
                                     }
                                 }
                             }
+                            OXIDE_NT_HUBRIS_REGISTERS => {
+                                // unused when building the archive, but valid
+                            }
                             _ => {
                                 bail!("unrecognized note 0x{:x}", note.n_type);
                             }
@@ -2852,10 +2816,6 @@ impl HubrisArchive {
         Ok(regions)
     }
 
-    pub fn dump_registers(&self) -> HashMap<ARMRegister, u32> {
-        self.registers.clone()
-    }
-
     pub fn registers(
         &self,
         core: &mut dyn crate::core::Core,