Repository: chipsalliance/caliptra-sw
Pull request #92 (2023-03-22 07:35:20)

Add test to ensure ROM doesn't link to the panic handler.
It should not be possible for the ROM to panic. Enforce this as part of continuous integration.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 9ab9891b..5bf7729e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -322,6 +322,7 @@ dependencies = [
 name = "caliptra-rom"
 version = "0.1.0"
 dependencies = [
+ "caliptra-builder",
  "caliptra-image-types",
  "caliptra-image-verify",
  "caliptra-kat",
diff --git a/builder/src/elf_symbols.rs b/builder/src/elf_symbols.rs
new file mode 100644
index 00000000..b69ef1cd
--- /dev/null
+++ b/builder/src/elf_symbols.rs
@@ -0,0 +1,183 @@
+// Licensed under the Apache-2.0 license
+
+use std::io;
+
+use elf::endian::LittleEndian;
+
+use super::other_err;
+
+pub fn elf_symbols(elf_bytes: &[u8]) -> io::Result<Vec<Symbol>> {
+    let elf = elf::ElfBytes::<LittleEndian>::minimal_parse(elf_bytes).map_err(other_err)?;
+    let Some((symbols, strings)) = elf.symbol_table().map_err(other_err)? else {
+        return Ok(vec![]);
+    };
+    let mut result = vec![];
+    for sym in symbols.iter() {
+        let sym_name = strings.get(sym.st_name as usize).map_err(|e| {
+            other_err(format!(
+                "Could not parse symbol string at index {}: {e}",
+                sym.st_name
+            ))
+        })?;
+        result.push(Symbol {
+            name: sym_name,
+            size: sym.st_size,
+            value: sym.st_value,
+
+            // Unwrap cannot panic because st_vis is only 2 bits
+            visibility: SymbolVisibility::try_from(sym.st_vis()).unwrap(),
+
+            // Unwrap cannot panic because st_symtype is only 4 bits
+            ty: SymbolType::try_from(sym.st_symtype()).unwrap(),
+
+            // Unwrap cannot panic because st_bind is only 4 bits
+            bind: SymbolBind::try_from(sym.st_bind()).unwrap(),
+        });
+    }
+    Ok(result)
+}
+
+#[derive(Debug, Eq, PartialEq)]
+pub struct Symbol<'a> {
+    pub name: &'a str,
+    pub size: u64,
+    pub value: u64,
+    pub ty: SymbolType,
+    pub visibility: SymbolVisibility,
+    pub bind: SymbolBind,
+}
+
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum SymbolType {
+    None = 0,
+    Object = 1,
+    Func = 2,
+    Section = 3,
+    File = 4,
+    Common = 5,
+    Tls = 6,
+    Reserved7 = 7,
+    Reserved8 = 8,
+    Reserved9 = 9,
+    Os10 = 10,
+    Os11 = 11,
+    Os12 = 12,
+    Proc13 = 13,
+    Proc14 = 14,
+    Proc15 = 15,
+}
+impl TryFrom<u8> for SymbolType {
+    type Error = ();
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(Self::None),
+            1 => Ok(Self::Object),
+            2 => Ok(Self::Func),
+            3 => Ok(Self::Section),
+            4 => Ok(Self::File),
+            5 => Ok(Self::Common),
+            6 => Ok(Self::Tls),
+            7 => Ok(Self::Reserved7),
+            8 => Ok(Self::Reserved8),
+            9 => Ok(Self::Reserved9),
+            10 => Ok(Self::Os10),
+            11 => Ok(Self::Os11),
+            12 => Ok(Self::Os12),
+            13 => Ok(Self::Proc13),
+            14 => Ok(Self::Proc14),
+            15 => Ok(Self::Proc15),
+            _ => Err(()),
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum SymbolVisibility {
+    Default = 0,
+    Internal = 1,
+    Hidden = 2,
+    Protected = 3,
+}
+impl TryFrom<u8> for SymbolVisibility {
+    type Error = ();
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(Self::Default),
+            1 => Ok(Self::Internal),
+            2 => Ok(Self::Hidden),
+            3 => Ok(Self::Protected),
+            _ => Err(()),
+        }
+    }
+}
+
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum SymbolBind {
+    Local = 0,
+    Global = 1,
+    Weak = 2,
+    Reserved3 = 3,
+    Reserved4 = 4,
+    Reserved5 = 5,
+    Reserved6 = 6,
+    Reserved7 = 7,
+    Reserved8 = 8,
+    Reserved9 = 9,
+    Os10 = 10,
+    Os11 = 11,
+    Os12 = 12,
+    Proc13 = 13,
+    Proc14 = 14,
+    Proc15 = 15,
+}
+impl TryFrom<u8> for SymbolBind {
+    type Error = ();
+
+    fn try_from(value: u8) -> Result<Self, Self::Error> {
+        match value {
+            0 => Ok(Self::Local),
+            1 => Ok(Self::Global),
+            2 => Ok(Self::Weak),
+            3 => Ok(Self::Reserved3),
+            4 => Ok(Self::Reserved4),
+            5 => Ok(Self::Reserved5),
+            6 => Ok(Self::Reserved6),
+            7 => Ok(Self::Reserved7),
+            8 => Ok(Self::Reserved8),
+            9 => Ok(Self::Reserved9),
+            10 => Ok(Self::Os10),
+            11 => Ok(Self::Os11),
+            12 => Ok(Self::Os12),
+            13 => Ok(Self::Proc13),
+            14 => Ok(Self::Proc14),
+            15 => Ok(Self::Proc15),
+            _ => Err(()),
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use crate::{self as caliptra_builder, SymbolBind, SymbolType, SymbolVisibility};
+
+    #[test]
+    fn test_elf_symbols() {
+        let symbols =
+            caliptra_builder::elf_symbols(include_bytes!("testdata/example.elf")).unwrap();
+        let bss_start = symbols.iter().find(|s| s.name == "BSS_START");
+        assert_eq!(
+            bss_start,
+            Some(&caliptra_builder::elf_symbols::Symbol {
+                name: "BSS_START",
+                size: 0,
+                value: 0x50000000,
+                ty: SymbolType::None,
+                visibility: SymbolVisibility::Default,
+                bind: SymbolBind::Global,
+            })
+        );
+        println!("{:?}", symbols);
+    }
+}
diff --git a/builder/src/lib.rs b/builder/src/lib.rs
index 4a2a9ffd..05da2dd8 100644
--- a/builder/src/lib.rs
+++ b/builder/src/lib.rs
@@ -7,6 +7,10 @@ use std::process::Command;
 
 use elf::endian::LittleEndian;
 
+mod elf_symbols;
+
+pub use elf_symbols::{elf_symbols, Symbol, SymbolBind, SymbolType, SymbolVisibility};
+
 fn other_err(e: impl Into<Box<dyn std::error::Error + Send + Sync>>) -> io::Error {
     io::Error::new(ErrorKind::Other, e)
 }
@@ -42,6 +46,7 @@ pub fn build_firmware_elf(fw_crate_name: &str, bin_name: &str) -> io::Result<Vec
             .arg("--target")
             .arg(TARGET)
             .arg("--features=emu,riscv")
+            .arg("--no-default-features")
             .arg("--profile")
             .arg(PROFILE)
             .arg("-p")
diff --git a/rom/dev/Cargo.toml b/rom/dev/Cargo.toml
index 4e13c446..4a6eacf0 100644
--- a/rom/dev/Cargo.toml
+++ b/rom/dev/Cargo.toml
@@ -18,7 +18,11 @@ zerocopy = "0.6.1"
 [build-dependencies]
 cfg-if = "1.0.0"
 
+[dev-dependencies]
+caliptra-builder = { path = "../../builder" }
+
 [features]
+riscv = []
 default = ["std"]
 emu = ["caliptra-lib/emu"]
 std = [
diff --git a/rom/dev/src/main.rs b/rom/dev/src/main.rs
index 6defa58b..6b1b5209 100644
--- a/rom/dev/src/main.rs
+++ b/rom/dev/src/main.rs
@@ -15,6 +15,8 @@ Abstract:
 #![cfg_attr(not(feature = "std"), no_main)]
 
 use crate::lock::lock_registers;
+use core::hint::black_box;
+
 use caliptra_lib::report_fw_error_non_fatal;
 use rom_env::RomEnv;
 
@@ -145,6 +147,7 @@ extern "C" fn nmi_handler(exception: &exception::ExceptionRecord) {
 #[cfg(not(feature = "std"))]
 fn rom_panic(_: &core::panic::PanicInfo) -> ! {
     cprintln!("Panic!!");
+    panic_is_possible();
 
     // TODO: Signal non-fatal error to SOC
 
@@ -158,3 +161,11 @@ fn report_error(code: u32) -> ! {
 
     loop {}
 }
+
+#[no_mangle]
+#[inline(never)]
+fn panic_is_possible() {
+    black_box(());
+    // The existence of this symbol is used to inform test_panic_missing
+    // that panics are possible. Do not remove or rename this symbol.
+}


Test patch:
diff --git a/rom/dev/tests/test_panic_missing.rs b/rom/dev/tests/test_panic_missing.rs
new file mode 100644
index 00000000..7eb6fa47
--- /dev/null
+++ b/rom/dev/tests/test_panic_missing.rs
@@ -0,0 +1,13 @@
+// Licensed under the Apache-2.0 license
+
+#[test]
+fn test_panic_missing() {
+    let rom_elf = caliptra_builder::build_firmware_elf("caliptra-rom", "caliptra-rom").unwrap();
+    let symbols = caliptra_builder::elf_symbols(&rom_elf).unwrap();
+    if symbols.iter().any(|s| s.name.contains("panic_is_possible")) {
+        panic!(
+            "The caliptra ROM contains the panic_is_possible symbol, which is not allowed. \
+                Please remove any code that might panic."
+        )
+    }
+}