From ba7b156c2387e3326db371d54245a0a8aea0dbd2 Mon Sep 17 00:00:00 2001 From: Garret Kelly Date: Sun, 10 May 2020 15:42:50 -0400 Subject: [PATCH 1/2] Enforce register value constraints 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. --- src/error.rs | 63 +++++++++++++++++++++++++++++++++++ src/svd/registerproperties.rs | 1 + 2 files changed, 64 insertions(+) diff --git a/src/error.rs b/src/error.rs index 0ac94300..630539bd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,6 +74,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 = Lazy::new(|| Regex::new("^[_A-Za-z0-9]*$").unwrap()); if PATTERN.is_match(name) { @@ -93,3 +103,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, + value: Option, + mask: Option, +) -> 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" + ); + } +} diff --git a/src/svd/registerproperties.rs b/src/svd/registerproperties.rs index 37bf6c95..e3ae10ab 100644 --- a/src/svd/registerproperties.rs +++ b/src/svd/registerproperties.rs @@ -51,6 +51,7 @@ impl Parse for RegisterProperties { p.reset_value = parse::optional::("resetValue", tree)?; p.reset_mask = parse::optional::("resetMask", tree)?; p.access = parse::optional::("access", tree)?; + check_reset_value(p.size, p.reset_value, p.reset_mask)?; Ok(p) } } From f23e29da02e7220c03171e9258fc79e0fd206f63 Mon Sep 17 00:00:00 2001 From: Garret Kelly Date: Tue, 12 May 2020 10:11:12 -0400 Subject: [PATCH 2/2] Add missing use statement and fix a test --- src/error.rs | 1 + src/svd/registerproperties.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 630539bd..0088dec2 100644 --- a/src/error.rs +++ b/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; diff --git a/src/svd/registerproperties.rs b/src/svd/registerproperties.rs index e3ae10ab..8b4f679d 100644 --- a/src/svd/registerproperties.rs +++ b/src/svd/registerproperties.rs @@ -95,7 +95,7 @@ mod tests { 0xaabbccdd 0x11223344 - 0x00000000 + 0xffffffff read-only ", @@ -104,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();