From 6207008884c6ae8e465933a445cca796a43d1b53 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Oct 2019 19:11:35 -0500 Subject: document the limitations of cyccnt::{Instant,Duration} --- src/cyccnt.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'src/cyccnt.rs') diff --git a/src/cyccnt.rs b/src/cyccnt.rs index c8a1b7e..1c21560 100644 --- a/src/cyccnt.rs +++ b/src/cyccnt.rs @@ -1,4 +1,4 @@ -//! Data Watchpoint Trace (DWT) unit's CYCle CouNTer +//! Data Watchpoint Trace (DWT) unit's CYCle CouNTer (CYCCNT) use core::{ cmp::Ordering, @@ -16,9 +16,15 @@ use crate::Fraction; /// /// This data type is only available on ARMv7-M /// -/// Note that this value is tied to the CYCCNT of one core and that sending it a different core -/// makes it lose its meaning -- each Cortex-M core has its own CYCCNT counter and these are usually -/// unsynchronized and they may even be running at different frequencies. +/// # Correctness +/// +/// Adding or subtracting a `Duration` of more than `(1 << 31)` cycles to an `Instant` effectively +/// makes it "wrap around" and creates an incorrect value. This is also true if the operation is +/// done in steps, e.g. `(instant + dur) + dur` where `dur` is `(1 << 30)` ticks. +/// +/// In multi-core contexts: this value is tied to the CYCCNT of *one* core so sending it a different +/// core makes it lose its meaning -- each Cortex-M core has its own CYCCNT counter and these are +/// usually unsynchronized and may even be running at different frequencies. #[derive(Clone, Copy, Eq, PartialEq)] pub struct Instant { inner: i32, @@ -61,6 +67,8 @@ impl fmt::Debug for Instant { impl ops::AddAssign for Instant { fn add_assign(&mut self, dur: Duration) { + // NOTE this is a debug assertion because there's no foolproof way to detect a wrap around; + // the user may write `(instant + dur) + dur` where `dur` is `(1<<31)-1` ticks. debug_assert!(dur.inner < (1 << 31)); self.inner = self.inner.wrapping_add(dur.inner as i32); } @@ -77,7 +85,7 @@ impl ops::Add for Instant { impl ops::SubAssign for Instant { fn sub_assign(&mut self, dur: Duration) { - // XXX should this be a non-debug assertion? + // NOTE see the NOTE in `>::add_assign` debug_assert!(dur.inner < (1 << 31)); self.inner = self.inner.wrapping_sub(dur.inner as i32); } @@ -115,6 +123,12 @@ impl PartialOrd for Instant { /// A `Duration` type to represent a span of time. /// /// This data type is only available on ARMv7-M +/// +/// # Correctness +/// +/// This type is *not* appropriate for representing time spans in the order of, or larger than, +/// seconds because it can hold a maximum of `(1 << 31)` "ticks" where each tick is the inverse of +/// the CPU frequency, which usually is dozens of MHz. #[derive(Clone, Copy, Default, Eq, Ord, PartialEq, PartialOrd)] pub struct Duration { inner: u32, -- cgit v1.2.3 From a458a0703153f17ae9170ea88a38348ebd290388 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 15 Oct 2019 19:12:05 -0500 Subject: cyccnt::Instant: simplify the Send / Sync impl originally the type was made `!Send` because it loses its meaning when send from one core to another but that was an incorrect use of the `Send` bound (the send operation makes the value incorrect but that doesn't cause memory unsafety on its own). So later the type was (explicitly) made `Send` again resulting in a convoluted implementation -- this commit simplifies things. --- src/cyccnt.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) (limited to 'src/cyccnt.rs') diff --git a/src/cyccnt.rs b/src/cyccnt.rs index 1c21560..86969cb 100644 --- a/src/cyccnt.rs +++ b/src/cyccnt.rs @@ -3,9 +3,7 @@ use core::{ cmp::Ordering, convert::{Infallible, TryInto}, - fmt, - marker::PhantomData, - ops, + fmt, ops, }; use cortex_m::peripheral::DWT; @@ -28,19 +26,13 @@ use crate::Fraction; #[derive(Clone, Copy, Eq, PartialEq)] pub struct Instant { inner: i32, - _not_send_or_sync: PhantomData<*mut ()>, } -unsafe impl Sync for Instant {} - -unsafe impl Send for Instant {} - impl Instant { /// Returns an instant corresponding to "now" pub fn now() -> Self { Instant { inner: DWT::get_cycle_count() as i32, - _not_send_or_sync: PhantomData, } } @@ -222,9 +214,6 @@ impl crate::Monotonic for CYCCNT { } fn zero() -> Instant { - Instant { - inner: 0, - _not_send_or_sync: PhantomData, - } + Instant { inner: 0 } } } -- cgit v1.2.3