Merge #120

120: Enforce register value constraints r=burrbull a=gkelly

Enforce register size, reset, and mask value agreement. If the provided
reset value or mask extend past the register size, or the provided reset
value would conflict with the mask, then fail parsing.

---

I saw that there are some tests from CMSIS that are disabled because of constraint violations due to a check like this. There are also a couple of ATSAMD51 tests (at least) that will fail because of this that are currently enabled.

Co-authored-by: Garret Kelly <[email protected]>

Author: bors[bot]

src/error.rs

@@ -2,6 +2,7 @@
 //! This module defines error types and messages for SVD parsing and encoding
 
 pub use anyhow::{Context, Result};
+use core::u32;
 use once_cell::sync::Lazy;
 use regex::Regex;
 use xmltree::Element;
@@ -74,6 +75,16 @@ pub enum NameError {
     Invalid(String, String),
 }
 
+#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
+pub enum ResetValueError {
+    #[error("Reset value 0x{0:x} doesn't fit in {1} bits")]
+    ValueTooLarge(u32, u32),
+    #[error("Reset value 0x{0:x} conflicts with mask '{1}'")]
+    MaskConflict(u32, u32),
+    #[error("Mask value 0x{0:x} doesn't fit in {1} bits")]
+    MaskTooLarge(u32, u32),
+}
+
 pub(crate) fn check_name(name: &str, tag: &str) -> Result<()> {
     static PATTERN: Lazy<Regex> = Lazy::new(|| Regex::new("^[_A-Za-z0-9]*$").unwrap());
     if PATTERN.is_match(name) {
@@ -93,3 +104,56 @@ pub(crate) fn check_dimable_name(name: &str, tag: &str) -> Result<()> {
         Err(NameError::Invalid(name.to_string(), tag.to_string()).into())
     }
 }
+
+pub(crate) fn check_reset_value(
+    size: Option<u32>,
+    value: Option<u32>,
+    mask: Option<u32>,
+) -> Result<()> {
+    const MAX_BITS: u32 = u32::MAX.count_ones();
+
+    if let (Some(size), Some(value)) = (size, value) {
+        if MAX_BITS - value.leading_zeros() > size {
+            return Err(ResetValueError::ValueTooLarge(value, size).into());
+        }
+    }
+    if let (Some(size), Some(mask)) = (size, mask) {
+        if MAX_BITS - mask.leading_zeros() > size {
+            return Err(ResetValueError::MaskTooLarge(mask, size).into());
+        }
+    }
+    if let (Some(value), Some(mask)) = (value, mask) {
+        if value & mask != value {
+            return Err(ResetValueError::MaskConflict(value, mask).into());
+        }
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::error::check_reset_value;
+
+    #[test]
+    fn test_check_reset_value() {
+        check_reset_value(None, None, None).unwrap();
+        check_reset_value(Some(8), None, None).unwrap();
+        check_reset_value(Some(8), None, Some(0xff)).unwrap();
+        check_reset_value(Some(32), Some(0xfaceface), None).unwrap();
+        check_reset_value(Some(32), Some(0xfaceface), Some(0xffffffff)).unwrap();
+
+        assert!(
+            check_reset_value(Some(8), None, Some(0x100)).is_err(),
+            "mask shouldn't fit in size"
+        );
+        assert!(
+            check_reset_value(Some(1), Some(0x02), None).is_err(),
+            "reset value shouldn't fit in field"
+        );
+        assert!(
+            check_reset_value(Some(8), Some(0x80), Some(0x01)).is_err(),
+            "value should conflict with mask"
+        );
+    }
+}

src/svd/registerproperties.rs

@@ -51,6 +51,7 @@ impl Parse for RegisterProperties {
         p.reset_value = parse::optional::<u32>("resetValue", tree)?;
         p.reset_mask = parse::optional::<u32>("resetMask", tree)?;
         p.access = parse::optional::<Access>("access", tree)?;
+        check_reset_value(p.size, p.reset_value, p.reset_mask)?;
         Ok(p)
     }
 }
@@ -94,7 +95,7 @@ mod tests {
             <mock>
                 <size>0xaabbccdd</size>
                 <resetValue>0x11223344</resetValue>
-                <resetMask>0x00000000</resetMask>
+                <resetMask>0xffffffff</resetMask>
                 <access>read-only</access>
             </mock>
         ",
@@ -103,7 +104,7 @@ mod tests {
         let mut expected = RegisterProperties::default();
         expected.size = Some(0xaabbccdd);
         expected.reset_value = Some(0x11223344);
-        expected.reset_mask = Some(0x00000000);
+        expected.reset_mask = Some(0xffffffff);
         expected.access = Some(Access::ReadOnly);
 
         let tree1 = Element::parse(example.as_bytes()).unwrap();