From 5824434eed832b33f457cabae3f6009025ae316a Mon Sep 17 00:00:00 2001 From: Johnathan Van Why Date: Mon, 13 Jul 2020 16:12:36 -0700 Subject: [PATCH 1/2] Add the Allowed type to libtock_platform. Allowed is the type that will be returned by Platform::allow() in the successful case. It represents a memory buffer shared with the kernel. It uses raw pointers and volatile writes to perform read/write accesses to the memory to avoid encountering undefined behavior. This is the first step towards fixing #129 and #143. --- core/platform/src/allows/allow_readable.rs | 18 ++ core/platform/src/allows/allowed.rs | 85 ++++++++++ core/platform/src/allows/allowed_tests.rs | 184 +++++++++++++++++++++ core/platform/src/allows/mod.rs | 10 ++ core/platform/src/allows/test_util.rs | 37 +++++ core/platform/src/lib.rs | 4 + 6 files changed, 338 insertions(+) create mode 100644 core/platform/src/allows/allow_readable.rs create mode 100644 core/platform/src/allows/allowed.rs create mode 100644 core/platform/src/allows/allowed_tests.rs create mode 100644 core/platform/src/allows/mod.rs create mode 100644 core/platform/src/allows/test_util.rs diff --git a/core/platform/src/allows/allow_readable.rs b/core/platform/src/allows/allow_readable.rs new file mode 100644 index 00000000..c35701bf --- /dev/null +++ b/core/platform/src/allows/allow_readable.rs @@ -0,0 +1,18 @@ +/// Because the kernel may modify shared memory and place any bit pattern into +/// that memory, we can only read types out of shared memory if every bit +/// pattern is a valid value of that type. `AllowReadable` types are safe to +/// read out of a shared buffer. +pub unsafe trait AllowReadable {} + +unsafe impl AllowReadable for i8 {} +unsafe impl AllowReadable for i16 {} +unsafe impl AllowReadable for i32 {} +unsafe impl AllowReadable for i64 {} +unsafe impl AllowReadable for i128 {} +unsafe impl AllowReadable for isize {} +unsafe impl AllowReadable for u8 {} +unsafe impl AllowReadable for u16 {} +unsafe impl AllowReadable for u32 {} +unsafe impl AllowReadable for u64 {} +unsafe impl AllowReadable for u128 {} +unsafe impl AllowReadable for usize {} diff --git a/core/platform/src/allows/allowed.rs b/core/platform/src/allows/allowed.rs new file mode 100644 index 00000000..db8ba268 --- /dev/null +++ b/core/platform/src/allows/allowed.rs @@ -0,0 +1,85 @@ +/// An individual value that has been shared with the kernel using the `allow` +/// system call. +// Design note: Allowed's implementation does not directly use the 'b lifetime. +// Platform uses 'b to prevent the Allowed from accessing the buffer after the +// buffer becomes invalid. +pub struct Allowed<'b, T: 'b> { + // Safety properties: + // 1. `buffer` remains valid and usable for the lifetime of this Allowed + // instance. + // 2. Read and write accesses of `buffer`'s pointee must be performed as a + // volatile operation, as the kernel may mutate buffer's pointee at any + // time. + // 3. The value `buffer` points to may have an arbitrary bit pattern in + // it, so reading from `buffer` is only safe if the type contained + // within is AllowReadable. + buffer: core::ptr::NonNull, + + // We need to use the 'b lifetime in Allowed to prevent an "unused lifetime" + // compiler error. We use it here. Note that the phantom data must be an + // &mut rather than a shared reference in order to make Allowed invariant + // with respect to T. Invariance is required because Allowed allows us to + // mutate the value in buffer, and invariance is the required property to do + // so without allowing callers to produce dangling references. This is + // documented at https://doc.rust-lang.org/nomicon/subtyping.html. + _phantom: core::marker::PhantomData<&'b mut T>, +} + +// Allowed's API is based on that of core::cell::Cell, but removes some methods +// that are not safe for use with shared memory. +// +// Internally, Allowed performs accesses to the shared memory using volatile +// reads and writes through raw pointers. We avoid constructing references to +// shared memory because that leads to undefined behavior (there is some +// background on this in the following discussion: +// https://github.com/rust-lang/unsafe-code-guidelines/issues/33). Tock runs on +// single-threaded platforms, some of which lack atomic instructions, so we only +// need to be able to deconflict races between the kernel (which will never +// interrupt an instruction's execution) and this process. Therefore volatile +// accesses are sufficient to deconflict races. +impl<'b, T: 'b> Allowed<'b, T> { + // Allowed can only be constructed by the Platform. It is constructed after + // the `allow` system call, and as such must accept a raw pointer rather + // than a reference. The caller must make sure the following are true: + // 1. buffer points to a valid instance of type T + // 2. There are no references to buffer's pointee + // 3. buffer remains usable until the Allowed's lifetime has ended. + #[allow(dead_code)] // TODO: Remove when Platform is implemented + pub(crate) unsafe fn new(buffer: core::ptr::NonNull) -> Allowed<'b, T> { + Allowed { + buffer, + _phantom: core::marker::PhantomData, + } + } + + // Sets the value in the buffer. Note that unlike `core::cell::Cell::set`, + // `Allowed::set` will not drop the existing value. To drop the existing + // value, use `replace` and drop the return value. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write_volatile(self.buffer.as_ptr(), value); + } + } +} + +impl<'b, T: crate::AllowReadable + 'b> Allowed<'b, T> { + pub fn replace(&self, value: T) -> T { + let current = unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) }; + unsafe { + core::ptr::write_volatile(self.buffer.as_ptr(), value); + } + current + } +} + +impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> { + pub fn get(&self) -> T { + unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) } + } +} + +impl<'b, T: crate::AllowReadable + Default + 'b> Allowed<'b, T> { + pub fn take(&self) -> T { + self.replace(T::default()) + } +} diff --git a/core/platform/src/allows/allowed_tests.rs b/core/platform/src/allows/allowed_tests.rs new file mode 100644 index 00000000..ec6a3581 --- /dev/null +++ b/core/platform/src/allows/allowed_tests.rs @@ -0,0 +1,184 @@ +use crate::allows::test_util::DropCheck; +use crate::Allowed; +use core::marker::PhantomData; +use core::ptr::NonNull; + +// How do we simulate accesses to the shared buffer by the kernel? +// +// Well, a naive way would be to mutate the `buffer` variable directly. Because +// Allowed accesses the memory through a *mut pointer, such a test would compile +// and run fine with the current Rust compiler. As far as I can tell, it would +// not hit any behavior documented as undefined at +// https://doc.rust-lang.org/stable/reference/behavior-considered-undefined.html, +// nor would it cause rustc to generate LLVM bitcode that encounters undefined +// behavior. +// +// However, the naive approach will throw an "undefined behavior" error when run +// under Miri (e.g. with `cargo miri test`), which uses the stacked borrows +// model [1]. In particular, accessing the `buffer` variable directly pops the +// SharedRW off buffer's borrow stack, which prevents Allowed from using its +// *mut pointer to access `buffer` afterwards. It is likely that Rust will adopt +// the stacked borrows model as its formal model for borrow validity, and in +// that case accessing `buffer` in that manner will become undefined behavior. +// In addition, running these tests under Miri is highly valuable, as this is +// tricky code to get correct and an unsound API may be hard to fix. +// +// Instead, we explicitly refer to buffer through use of a KernelPtr that +// simulates the pointer that `allow()` would hand to the Tock kernel. As far as +// the stacked borrows model is concerned, accesses through the KernelPtr +// variable behave identically to mutations performed by the kernel. This +// pattern allows us to use `cargo miri test` to not only execute the unit +// tests, but to test whether Allowed would encounter undefined behavior when +// interacting with a real Tock kernel. +// +// [1] https://plv.mpi-sws.org/rustbelt/stacked-borrows/paper.pdf +struct KernelPtr<'b, T: 'b> { + ptr: NonNull, + + // We need to consume the 'b lifetime. This is very similar to Allowed's + // implementation. + _phantom: PhantomData<&'b mut T>, +} + +impl<'b, T: 'b> KernelPtr<'b, T> { + // The constructor for KernelPtr; simulates allow(). Returns both the + // Allowed instance the Platform would return and a KernelPtr the test can + // use to simulate a kernel. + pub fn allow(buffer: &'b mut T) -> (Allowed<'b, T>, KernelPtr<'b, T>) { + let ptr = NonNull::new(buffer).unwrap(); + // Discard buffer *without* creating a reference to it, as would be done + // if we called drop(). + let _ = buffer; + // All 3 preconditions of Allowed::new are satisfied by the fact that + // `buffer` is directly derived from a &'b mut T. + let allowed = unsafe { Allowed::new(ptr) }; + let kernel_ptr = KernelPtr { + ptr, + _phantom: PhantomData, + }; + (allowed, kernel_ptr) + } + + // Replaces the value in the buffer with a new one. Does not drop the + // existing value. + pub fn set(&self, value: T) { + unsafe { + core::ptr::write(self.ptr.as_ptr(), value); + } + } +} + +impl<'b, T: Copy + 'b> KernelPtr<'b, T> { + // Copies the contained value out of the buffer. + pub fn get(&self) -> T { + unsafe { core::ptr::read(self.ptr.as_ptr()) } + } +} + +impl<'b, 'f: 'b> KernelPtr<'b, DropCheck<'f>> { + // Retrieves the value of the contained DropCheck. + pub fn value(&self) -> usize { + let drop_check = unsafe { core::ptr::read(self.ptr.as_ptr()) }; + let value = drop_check.value; + core::mem::forget(drop_check); + value + } +} + +#[test] +fn set() { + let dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + allowed.set(DropCheck { + flag: Some(&dropped3), + value: 3, + }); + assert_eq!(kernel_ptr.value(), 3); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +} + +#[test] +fn replace() { + let dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + let returned = allowed.replace(DropCheck { + flag: Some(&dropped3), + value: 3, + }); + assert_eq!(returned.value, 2); + assert_eq!(kernel_ptr.value(), 3); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +} + +#[test] +fn get() { + // We can't use DropCheck because Drop and Copy cannot both be implemented + // on a single type. + let mut buffer = 1; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.get(), 1); + kernel_ptr.set(2); + assert_eq!(allowed.get(), 2); + assert_eq!(kernel_ptr.get(), 2); +} + +#[test] +fn take() { + let dropped1 = core::cell::Cell::new(false); + let dropped2 = core::cell::Cell::new(false); + let dropped3 = core::cell::Cell::new(false); + let mut buffer = DropCheck { + flag: Some(&dropped1), + value: 1, + }; + let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); + assert_eq!(kernel_ptr.value(), 1); + assert_eq!(dropped1.get(), false); + + // Simulate the kernel replacing the value in buffer. We don't drop the + // existing value. + kernel_ptr.set(DropCheck { + flag: Some(&dropped2), + value: 2, + }); + let returned = allowed.take(); + assert_eq!(returned.value, 2); + assert_eq!(kernel_ptr.value(), 0); + assert_eq!(dropped1.get(), false); + assert_eq!(dropped2.get(), false); + assert_eq!(dropped3.get(), false); +} diff --git a/core/platform/src/allows/mod.rs b/core/platform/src/allows/mod.rs new file mode 100644 index 00000000..410d158f --- /dev/null +++ b/core/platform/src/allows/mod.rs @@ -0,0 +1,10 @@ +mod allow_readable; +mod allowed; + +pub use allow_readable::AllowReadable; +pub use allowed::Allowed; + +#[cfg(test)] +mod allowed_tests; +#[cfg(test)] +mod test_util; diff --git a/core/platform/src/allows/test_util.rs b/core/platform/src/allows/test_util.rs new file mode 100644 index 00000000..0f63d9c3 --- /dev/null +++ b/core/platform/src/allows/test_util.rs @@ -0,0 +1,37 @@ +//! Contains testing utilities for libtock_platform::allowed. + +// A value that can be placed in an Allowed that checks whether it has been +// dropped. +#[derive(Default)] +pub(crate) struct DropCheck<'f> { + pub flag: Option<&'f core::cell::Cell>, + pub value: usize, +} + +impl<'f> Drop for DropCheck<'f> { + fn drop(&mut self) { + if let Some(flag) = self.flag { + flag.set(true); + } + } +} + +// Note: DropCheck cannot be safely used in a non-test context, as DropCheck +// does not satisfy AllowReadable's "every bit pattern is valid" requirement. +// However, in the tests we aren't sharing the buffer with a real Tock kernel, +// and instead we guarantee that only valid instances of DropCheck are loaded +// into the buffer. +unsafe impl<'f> crate::AllowReadable for DropCheck<'f> {} + +// Verify that DropCheck works, as none of the tests require it to actually set +// the drop flag. +#[test] +fn drop_check() { + let flag = core::cell::Cell::new(false); + let drop_check = DropCheck { + flag: Some(&flag), + value: 0, + }; + drop(drop_check); + assert_eq!(flag.get(), true); +} diff --git a/core/platform/src/lib.rs b/core/platform/src/lib.rs index 64d520b4..5cc7a1fe 100644 --- a/core/platform/src/lib.rs +++ b/core/platform/src/lib.rs @@ -7,3 +7,7 @@ // 2. The PlatformApi trait and Platform implementation. // 3. A system call trait so that Platform works in both real Tock apps and // unit test environments. + +mod allows; + +pub use allows::{AllowReadable, Allowed}; From 17513ebfa3a2ace2b69eeb47388691fcb67da02a Mon Sep 17 00:00:00 2001 From: Johnathan Van Why Date: Wed, 5 Aug 2020 11:38:54 -0700 Subject: [PATCH 2/2] Make Allowed require Copy and remove all the Drop-related complexity. Originally, I build Allowed so it can work with non-Copy types, which allows Drop types. The semantics of storing non-Copy and non-Drop types in shared memory, however, are somewhat nonobvious. Based on code review feedback, and being particularly cautious because Allowed uses `unsafe`, I decided to add a Copy constraint to Allowed. --- core/platform/src/allows/allowed.rs | 24 ++--- core/platform/src/allows/allowed_tests.rs | 112 +++++----------------- core/platform/src/allows/mod.rs | 2 - core/platform/src/allows/test_util.rs | 37 ------- 4 files changed, 38 insertions(+), 137 deletions(-) delete mode 100644 core/platform/src/allows/test_util.rs diff --git a/core/platform/src/allows/allowed.rs b/core/platform/src/allows/allowed.rs index db8ba268..4d66c2b2 100644 --- a/core/platform/src/allows/allowed.rs +++ b/core/platform/src/allows/allowed.rs @@ -1,9 +1,13 @@ /// An individual value that has been shared with the kernel using the `allow` /// system call. -// Design note: Allowed's implementation does not directly use the 'b lifetime. -// Platform uses 'b to prevent the Allowed from accessing the buffer after the -// buffer becomes invalid. -pub struct Allowed<'b, T: 'b> { +// Allowed's implementation does not directly use the 'b lifetime. Platform uses +// 'b to prevent the Allowed from accessing the buffer after the buffer becomes +// invalid. +// Allowed requires T to be Copy due to concerns about the semantics of +// non-copyable types in shared memory as well as concerns about unexpected +// behavior with Drop types. See the following PR discussion for more +// information: https://github.com/tock/libtock-rs/pull/222 +pub struct Allowed<'b, T: Copy + 'b> { // Safety properties: // 1. `buffer` remains valid and usable for the lifetime of this Allowed // instance. @@ -37,7 +41,7 @@ pub struct Allowed<'b, T: 'b> { // need to be able to deconflict races between the kernel (which will never // interrupt an instruction's execution) and this process. Therefore volatile // accesses are sufficient to deconflict races. -impl<'b, T: 'b> Allowed<'b, T> { +impl<'b, T: Copy + 'b> Allowed<'b, T> { // Allowed can only be constructed by the Platform. It is constructed after // the `allow` system call, and as such must accept a raw pointer rather // than a reference. The caller must make sure the following are true: @@ -52,9 +56,7 @@ impl<'b, T: 'b> Allowed<'b, T> { } } - // Sets the value in the buffer. Note that unlike `core::cell::Cell::set`, - // `Allowed::set` will not drop the existing value. To drop the existing - // value, use `replace` and drop the return value. + // Sets the value in the buffer. pub fn set(&self, value: T) { unsafe { core::ptr::write_volatile(self.buffer.as_ptr(), value); @@ -62,7 +64,7 @@ impl<'b, T: 'b> Allowed<'b, T> { } } -impl<'b, T: crate::AllowReadable + 'b> Allowed<'b, T> { +impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> { pub fn replace(&self, value: T) -> T { let current = unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) }; unsafe { @@ -70,15 +72,13 @@ impl<'b, T: crate::AllowReadable + 'b> Allowed<'b, T> { } current } -} -impl<'b, T: crate::AllowReadable + Copy + 'b> Allowed<'b, T> { pub fn get(&self) -> T { unsafe { core::ptr::read_volatile(self.buffer.as_ptr()) } } } -impl<'b, T: crate::AllowReadable + Default + 'b> Allowed<'b, T> { +impl<'b, T: crate::AllowReadable + Copy + Default + 'b> Allowed<'b, T> { pub fn take(&self) -> T { self.replace(T::default()) } diff --git a/core/platform/src/allows/allowed_tests.rs b/core/platform/src/allows/allowed_tests.rs index ec6a3581..f1f0c083 100644 --- a/core/platform/src/allows/allowed_tests.rs +++ b/core/platform/src/allows/allowed_tests.rs @@ -1,4 +1,3 @@ -use crate::allows::test_util::DropCheck; use crate::Allowed; use core::marker::PhantomData; use core::ptr::NonNull; @@ -32,7 +31,7 @@ use core::ptr::NonNull; // interacting with a real Tock kernel. // // [1] https://plv.mpi-sws.org/rustbelt/stacked-borrows/paper.pdf -struct KernelPtr<'b, T: 'b> { +struct KernelPtr<'b, T: Copy + 'b> { ptr: NonNull, // We need to consume the 'b lifetime. This is very similar to Allowed's @@ -40,7 +39,7 @@ struct KernelPtr<'b, T: 'b> { _phantom: PhantomData<&'b mut T>, } -impl<'b, T: 'b> KernelPtr<'b, T> { +impl<'b, T: Copy + 'b> KernelPtr<'b, T> { // The constructor for KernelPtr; simulates allow(). Returns both the // Allowed instance the Platform would return and a KernelPtr the test can // use to simulate a kernel. @@ -59,98 +58,53 @@ impl<'b, T: 'b> KernelPtr<'b, T> { (allowed, kernel_ptr) } - // Replaces the value in the buffer with a new one. Does not drop the - // existing value. + // Replaces the value in the buffer with a new one. pub fn set(&self, value: T) { unsafe { core::ptr::write(self.ptr.as_ptr(), value); } } -} -impl<'b, T: Copy + 'b> KernelPtr<'b, T> { // Copies the contained value out of the buffer. pub fn get(&self) -> T { unsafe { core::ptr::read(self.ptr.as_ptr()) } } } -impl<'b, 'f: 'b> KernelPtr<'b, DropCheck<'f>> { - // Retrieves the value of the contained DropCheck. - pub fn value(&self) -> usize { - let drop_check = unsafe { core::ptr::read(self.ptr.as_ptr()) }; - let value = drop_check.value; - core::mem::forget(drop_check); - value - } -} - #[test] fn set() { - let dropped1 = core::cell::Cell::new(false); - let dropped2 = core::cell::Cell::new(false); - let dropped3 = core::cell::Cell::new(false); - let mut buffer = DropCheck { - flag: Some(&dropped1), - value: 1, - }; + let mut buffer = 1; let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); - assert_eq!(kernel_ptr.value(), 1); - assert_eq!(dropped1.get(), false); + assert_eq!(kernel_ptr.get(), 1); - // Simulate the kernel replacing the value in buffer. We don't drop the - // existing value. - kernel_ptr.set(DropCheck { - flag: Some(&dropped2), - value: 2, - }); - allowed.set(DropCheck { - flag: Some(&dropped3), - value: 3, - }); - assert_eq!(kernel_ptr.value(), 3); - assert_eq!(dropped1.get(), false); - assert_eq!(dropped2.get(), false); - assert_eq!(dropped3.get(), false); + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); + allowed.set(3); + assert_eq!(kernel_ptr.get(), 3); } #[test] fn replace() { - let dropped1 = core::cell::Cell::new(false); - let dropped2 = core::cell::Cell::new(false); - let dropped3 = core::cell::Cell::new(false); - let mut buffer = DropCheck { - flag: Some(&dropped1), - value: 1, - }; + let mut buffer = 1; let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); - assert_eq!(kernel_ptr.value(), 1); - assert_eq!(dropped1.get(), false); + assert_eq!(kernel_ptr.get(), 1); - // Simulate the kernel replacing the value in buffer. We don't drop the - // existing value. - kernel_ptr.set(DropCheck { - flag: Some(&dropped2), - value: 2, - }); - let returned = allowed.replace(DropCheck { - flag: Some(&dropped3), - value: 3, - }); - assert_eq!(returned.value, 2); - assert_eq!(kernel_ptr.value(), 3); - assert_eq!(dropped1.get(), false); - assert_eq!(dropped2.get(), false); - assert_eq!(dropped3.get(), false); + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); + let returned = allowed.replace(3); + assert_eq!(returned, 2); + assert_eq!(kernel_ptr.get(), 3); } #[test] fn get() { - // We can't use DropCheck because Drop and Copy cannot both be implemented - // on a single type. let mut buffer = 1; let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); assert_eq!(kernel_ptr.get(), 1); + + assert_eq!(allowed.get(), 1); + assert_eq!(kernel_ptr.get(), 1); + kernel_ptr.set(2); assert_eq!(allowed.get(), 2); assert_eq!(kernel_ptr.get(), 2); @@ -158,27 +112,13 @@ fn get() { #[test] fn take() { - let dropped1 = core::cell::Cell::new(false); - let dropped2 = core::cell::Cell::new(false); - let dropped3 = core::cell::Cell::new(false); - let mut buffer = DropCheck { - flag: Some(&dropped1), - value: 1, - }; + let mut buffer = 1; let (allowed, kernel_ptr) = KernelPtr::allow(&mut buffer); - assert_eq!(kernel_ptr.value(), 1); - assert_eq!(dropped1.get(), false); + assert_eq!(kernel_ptr.get(), 1); - // Simulate the kernel replacing the value in buffer. We don't drop the - // existing value. - kernel_ptr.set(DropCheck { - flag: Some(&dropped2), - value: 2, - }); + // Simulate the kernel replacing the value in buffer. + kernel_ptr.set(2); let returned = allowed.take(); - assert_eq!(returned.value, 2); - assert_eq!(kernel_ptr.value(), 0); - assert_eq!(dropped1.get(), false); - assert_eq!(dropped2.get(), false); - assert_eq!(dropped3.get(), false); + assert_eq!(returned, 2); + assert_eq!(kernel_ptr.get(), 0); } diff --git a/core/platform/src/allows/mod.rs b/core/platform/src/allows/mod.rs index 410d158f..8cb21581 100644 --- a/core/platform/src/allows/mod.rs +++ b/core/platform/src/allows/mod.rs @@ -6,5 +6,3 @@ pub use allowed::Allowed; #[cfg(test)] mod allowed_tests; -#[cfg(test)] -mod test_util; diff --git a/core/platform/src/allows/test_util.rs b/core/platform/src/allows/test_util.rs deleted file mode 100644 index 0f63d9c3..00000000 --- a/core/platform/src/allows/test_util.rs +++ /dev/null @@ -1,37 +0,0 @@ -//! Contains testing utilities for libtock_platform::allowed. - -// A value that can be placed in an Allowed that checks whether it has been -// dropped. -#[derive(Default)] -pub(crate) struct DropCheck<'f> { - pub flag: Option<&'f core::cell::Cell>, - pub value: usize, -} - -impl<'f> Drop for DropCheck<'f> { - fn drop(&mut self) { - if let Some(flag) = self.flag { - flag.set(true); - } - } -} - -// Note: DropCheck cannot be safely used in a non-test context, as DropCheck -// does not satisfy AllowReadable's "every bit pattern is valid" requirement. -// However, in the tests we aren't sharing the buffer with a real Tock kernel, -// and instead we guarantee that only valid instances of DropCheck are loaded -// into the buffer. -unsafe impl<'f> crate::AllowReadable for DropCheck<'f> {} - -// Verify that DropCheck works, as none of the tests require it to actually set -// the drop flag. -#[test] -fn drop_check() { - let flag = core::cell::Cell::new(false); - let drop_check = DropCheck { - flag: Some(&flag), - value: 0, - }; - drop(drop_check); - assert_eq!(flag.get(), true); -}