From 8065d741aceb96ea06e70afce05408e334a977b5 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Tue, 2 Nov 2021 13:41:12 +0100 Subject: [PATCH 1/5] Fixed aliasing issue due to RacyCell implementation --- macros/src/codegen.rs | 2 +- macros/src/codegen/dispatchers.rs | 8 ++-- macros/src/codegen/local_resources_struct.rs | 4 +- macros/src/codegen/module.rs | 42 +++++++++---------- macros/src/codegen/post_init.rs | 12 +++--- macros/src/codegen/pre_init.rs | 2 +- macros/src/codegen/shared_resources.rs | 2 +- macros/src/codegen/shared_resources_struct.rs | 4 +- macros/src/codegen/timer_queue.rs | 8 ++-- src/lib.rs | 12 +++--- 10 files changed, 47 insertions(+), 49 deletions(-) diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index 63a4e3cadc..f07326ba47 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -135,7 +135,7 @@ pub fn app(app: &App, analysis: &Analysis, extra: &Extra) -> TokenStream2 { rtic::export::interrupt::free(|_| { use rtic::Monotonic as _; use rtic::time::Clock as _; - if let Some(m) = unsafe{ super::super::#ident.get_mut_unchecked() } { + if let Some(m) = unsafe{ &mut *super::super::#ident.get_mut() } { if let Ok(v) = m.try_now() { v } else { diff --git a/macros/src/codegen/dispatchers.rs b/macros/src/codegen/dispatchers.rs index 57103acd0b..a90a97c773 100644 --- a/macros/src/codegen/dispatchers.rs +++ b/macros/src/codegen/dispatchers.rs @@ -78,12 +78,12 @@ pub fn codegen(app: &App, analysis: &Analysis, _extra: &Extra) -> Vec { let #tupled = - #inputs - .get_unchecked() + (&*#inputs + .get()) .get_unchecked(usize::from(index)) .as_ptr() .read(); - #fq.get_mut_unchecked().split().0.enqueue_unchecked(index); + (&mut *#fq.get_mut()).split().0.enqueue_unchecked(index); let priority = &rtic::export::Priority::new(PRIORITY); #name( #name::Context::new(priority) @@ -95,7 +95,7 @@ pub fn codegen(app: &App, analysis: &Analysis, _extra: &Extra) -> Vec>(); stmts.push(quote!( - while let Some((task, index)) = #rq.get_mut_unchecked().split().1.dequeue() { + while let Some((task, index)) = (&mut *#rq.get_mut()).split().1.dequeue() { match task { #(#arms)* } diff --git a/macros/src/codegen/local_resources_struct.rs b/macros/src/codegen/local_resources_struct.rs index cdbfcccac9..10bde5fefb 100644 --- a/macros/src/codegen/local_resources_struct.rs +++ b/macros/src/codegen/local_resources_struct.rs @@ -57,9 +57,9 @@ pub fn codegen(ctxt: Context, needs_lt: &mut bool, app: &App) -> (TokenStream2, let expr = if is_declared { // If the local resources is already initialized, we only need to access its value and // not go through an `MaybeUninit` - quote!(#mangled_name.get_mut_unchecked()) + quote!(&mut *#mangled_name.get_mut()) } else { - quote!(&mut *#mangled_name.get_mut_unchecked().as_mut_ptr()) + quote!(&mut *(&mut *#mangled_name.get_mut()).as_mut_ptr()) }; values.push(quote!( diff --git a/macros/src/codegen/module.rs b/macros/src/codegen/module.rs index 5e0827ca9d..c59a814c6e 100644 --- a/macros/src/codegen/module.rs +++ b/macros/src/codegen/module.rs @@ -232,15 +232,15 @@ pub fn codegen( let input = #tupled; unsafe { - if let Some(index) = rtic::export::interrupt::free(|_| #fq.get_mut_unchecked().dequeue()) { - #inputs - .get_mut_unchecked() + if let Some(index) = rtic::export::interrupt::free(|_| (&mut *#fq.get_mut()).dequeue()) { + (&mut *#inputs + .get_mut()) .get_unchecked_mut(usize::from(index)) .as_mut_ptr() .write(input); rtic::export::interrupt::free(|_| { - #rq.get_mut_unchecked().enqueue_unchecked((#t::#name, index)); + (&mut *#rq.get_mut()).enqueue_unchecked((#t::#name, index)); }); rtic::pend(#device::#enum_::#interrupt); @@ -330,16 +330,16 @@ pub fn codegen( impl #internal_spawn_handle_ident { pub fn cancel(self) -> Result<#ty, ()> { rtic::export::interrupt::free(|_| unsafe { - let tq = #tq.get_mut_unchecked(); + let tq = &mut *#tq.get_mut(); if let Some((_task, index)) = tq.cancel_marker(self.marker) { // Get the message - let msg = #inputs - .get_unchecked() + let msg = (&*#inputs + .get()) .get_unchecked(usize::from(index)) .as_ptr() .read(); // Return the index to the free queue - #fq.get_mut_unchecked().split().0.enqueue_unchecked(index); + (&mut *#fq.get_mut()).split().0.enqueue_unchecked(index); Ok(msg) } else { @@ -359,10 +359,10 @@ pub fn codegen( pub fn reschedule_at(self, instant: rtic::time::Instant<#mono_type>) -> Result { rtic::export::interrupt::free(|_| unsafe { - let marker = *#tq_marker.get_mut_unchecked(); - *#tq_marker.get_mut_unchecked() = #tq_marker.get_mut_unchecked().wrapping_add(1); + let marker = #tq_marker.get().read(); + #tq_marker.get_mut().write(marker.wrapping_add(1)); - let tq = #tq.get_mut_unchecked(); + let tq = (&mut *#tq.get_mut()); tq.update_marker(self.marker, marker, instant, || #pend).map(|_| #name::#m::SpawnHandle { marker }) }) @@ -383,7 +383,7 @@ pub fn codegen( D::T: Into<<#mono_type as rtic::time::Clock>::T>, { - let instant = if rtic::export::interrupt::free(|_| unsafe { #m_ident.get_mut_unchecked().is_none() }) { + let instant = if rtic::export::interrupt::free(|_| unsafe { (&*#m_ident.get()).is_none() }) { rtic::time::Instant::new(0) } else { monotonics::#m::now() @@ -401,21 +401,21 @@ pub fn codegen( ) -> Result<#name::#m::SpawnHandle, #ty> { unsafe { let input = #tupled; - if let Some(index) = rtic::export::interrupt::free(|_| #fq.get_mut_unchecked().dequeue()) { - #inputs - .get_mut_unchecked() + if let Some(index) = rtic::export::interrupt::free(|_| (&mut *#fq.get_mut()).dequeue()) { + (&mut *#inputs + .get_mut()) .get_unchecked_mut(usize::from(index)) .as_mut_ptr() .write(input); - #instants - .get_mut_unchecked() + (&mut *#instants + .get_mut()) .get_unchecked_mut(usize::from(index)) .as_mut_ptr() .write(instant); rtic::export::interrupt::free(|_| { - let marker = *#tq_marker.get_mut_unchecked(); + let marker = #tq_marker.get().read(); let nr = rtic::export::NotReady { instant, index, @@ -423,15 +423,15 @@ pub fn codegen( marker, }; - *#tq_marker.get_mut_unchecked() = #tq_marker.get_mut_unchecked().wrapping_add(1); + #tq_marker.get_mut().write(#tq_marker.get().read().wrapping_add(1)); - let tq = #tq.get_mut_unchecked(); + let tq = &mut *#tq.get_mut(); tq.enqueue_unchecked( nr, || #enable_interrupt, || #pend, - #m_ident.get_mut_unchecked().as_mut()); + (&mut *#m_ident.get_mut()).as_mut()); Ok(#name::#m::SpawnHandle { marker }) }) diff --git a/macros/src/codegen/post_init.rs b/macros/src/codegen/post_init.rs index 5624b20a72..b3359ab468 100644 --- a/macros/src/codegen/post_init.rs +++ b/macros/src/codegen/post_init.rs @@ -19,10 +19,9 @@ pub fn codegen(app: &App, analysis: &Analysis) -> Vec { // We include the cfgs #(#cfgs)* // Resource is a RacyCell> - // - `get_mut_unchecked` to obtain `MaybeUninit` - // - `as_mut_ptr` to obtain a raw pointer to `MaybeUninit` + // - `get_mut` to obtain a raw pointer to `MaybeUninit` // - `write` the defined value for the late resource T - #mangled_name.get_mut_unchecked().as_mut_ptr().write(shared_resources.#name); + (&mut *#mangled_name.get_mut()).as_mut_ptr().write(shared_resources.#name); )); } } @@ -37,10 +36,9 @@ pub fn codegen(app: &App, analysis: &Analysis) -> Vec { // We include the cfgs #(#cfgs)* // Resource is a RacyCell> - // - `get_mut_unchecked` to obtain `MaybeUninit` - // - `as_mut_ptr` to obtain a raw pointer to `MaybeUninit` + // - `get_mut` to obtain a raw pointer to `MaybeUninit` // - `write` the defined value for the late resource T - #mangled_name.get_mut_unchecked().as_mut_ptr().write(local_resources.#name); + (&mut *#mangled_name.get_mut()).as_mut_ptr().write(local_resources.#name); )); } } @@ -58,7 +56,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> Vec { // Store the monotonic let name = util::monotonic_ident(&monotonic.to_string()); - stmts.push(quote!(*#name.get_mut_unchecked() = Some(monotonics.#idx);)); + stmts.push(quote!(#name.get_mut().write(Some(monotonics.#idx));)); } // Enable the interrupts -- this completes the `init`-ialization phase diff --git a/macros/src/codegen/pre_init.rs b/macros/src/codegen/pre_init.rs index 3017c08e3d..42cc055298 100644 --- a/macros/src/codegen/pre_init.rs +++ b/macros/src/codegen/pre_init.rs @@ -19,7 +19,7 @@ pub fn codegen(app: &App, analysis: &Analysis, extra: &Extra) -> Vec (TokenStream2, } let expr = if access.is_exclusive() { - quote!(&mut *#mangled_name.get_mut_unchecked().as_mut_ptr()) + quote!(&mut *(&mut *#mangled_name.get_mut()).as_mut_ptr()) } else { - quote!(&*#mangled_name.get_unchecked().as_ptr()) + quote!(&*(&*#mangled_name.get()).as_ptr()) }; values.push(quote!( diff --git a/macros/src/codegen/timer_queue.rs b/macros/src/codegen/timer_queue.rs index 896b3a83f6..2a344d256b 100644 --- a/macros/src/codegen/timer_queue.rs +++ b/macros/src/codegen/timer_queue.rs @@ -117,7 +117,7 @@ pub fn codegen(app: &App, analysis: &Analysis, _extra: &Extra) -> Vec { - rtic::export::interrupt::free(|_| #rq.get_mut_unchecked().split().0.enqueue_unchecked((#rqt::#name, index))); + rtic::export::interrupt::free(|_| (&mut *#rq.get_mut()).split().0.enqueue_unchecked((#rqt::#name, index))); #pend } @@ -137,8 +137,8 @@ pub fn codegen(app: &App, analysis: &Analysis, _extra: &Extra) -> Vec Vec RacyCell { RacyCell(UnsafeCell::new(value)) } - /// Get `&mut T` + /// Get `*mut T` #[inline(always)] - pub unsafe fn get_mut_unchecked(&self) -> &mut T { - &mut *self.0.get() + pub unsafe fn get_mut(&self) -> *mut T { + self.0.get() } - /// Get `&T` + /// Get `*const T` #[inline(always)] - pub unsafe fn get_unchecked(&self) -> &T { - &*self.0.get() + pub unsafe fn get(&self) -> *const T { + self.0.get() } } From 1e2ab78a3fad8f383e5949dd0a11aaaaf03467f1 Mon Sep 17 00:00:00 2001 From: Per Lindgren Date: Tue, 2 Nov 2021 19:47:14 +0100 Subject: [PATCH 2/5] added doc for RacyCell --- src/lib.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index d5505b03cf..8463442acf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,6 +59,27 @@ where use core::cell::UnsafeCell; /// Internal replacement for `static mut T` +/// +/// Used to represent RTIC Resources +/// +/// Soundness: +/// 1) Unsafe API for internal use only +/// 2) get_mut(&self) -> *mut T +/// returns a raw mutable pointer to the inner T +/// casting to &mut T is under control of RTIC +/// RTIC ensures &mut T to be unique under Rust aliasing rules. +/// +/// Implementation uses the underlying UnsafeCell +/// self.0.get() -> *mut T +/// +/// 3) get(&self) -> *const T +/// returns a raw immutable (const) pointer to the inner T +/// casting to &T is under control of RTIC +/// RTIC ensures &T to be shared under Rust aliasing rules. +/// +/// Implementation uses the underlying UnsafeCell +/// self.0.get() -> *mut T, demoted to *const T +/// #[repr(transparent)] pub struct RacyCell(UnsafeCell); From d3d66c97aef5eeb06ea57dac8200c43f6720e1c1 Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Wed, 3 Nov 2021 08:26:45 +0100 Subject: [PATCH 3/5] Cleanup of resource initialization, no need to dereference --- macros/src/codegen/post_init.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/macros/src/codegen/post_init.rs b/macros/src/codegen/post_init.rs index b3359ab468..07fbd03c9c 100644 --- a/macros/src/codegen/post_init.rs +++ b/macros/src/codegen/post_init.rs @@ -21,7 +21,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> Vec { // Resource is a RacyCell> // - `get_mut` to obtain a raw pointer to `MaybeUninit` // - `write` the defined value for the late resource T - (&mut *#mangled_name.get_mut()).as_mut_ptr().write(shared_resources.#name); + #mangled_name.get_mut().write(core::mem::MaybeUninit::new(shared_resources.#name)); )); } } @@ -38,7 +38,7 @@ pub fn codegen(app: &App, analysis: &Analysis) -> Vec { // Resource is a RacyCell> // - `get_mut` to obtain a raw pointer to `MaybeUninit` // - `write` the defined value for the late resource T - (&mut *#mangled_name.get_mut()).as_mut_ptr().write(local_resources.#name); + #mangled_name.get_mut().write(core::mem::MaybeUninit::new(local_resources.#name)); )); } } From 50017b96f04ff095e143fd19bb8487e86adae28f Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Wed, 3 Nov 2021 08:27:05 +0100 Subject: [PATCH 4/5] Fixed aliasing in lock impl --- macros/src/codegen/shared_resources.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/macros/src/codegen/shared_resources.rs b/macros/src/codegen/shared_resources.rs index aa50dccb6e..b27c827c23 100644 --- a/macros/src/codegen/shared_resources.rs +++ b/macros/src/codegen/shared_resources.rs @@ -69,7 +69,7 @@ pub fn codegen( let ptr = quote!( #(#cfgs)* - (&mut *#mangled_name.get_mut()).as_mut_ptr() + #mangled_name.get_mut() as *mut _ ); let ceiling = match analysis.ownerships.get(name) { From 9e24fcbbd90609a25b9d985f9292900b476fe5ea Mon Sep 17 00:00:00 2001 From: Emil Fresk Date: Wed, 3 Nov 2021 08:54:18 +0100 Subject: [PATCH 5/5] Fix CI --- examples/cfg-whole-task.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/examples/cfg-whole-task.rs b/examples/cfg-whole-task.rs index 213fe13f92..f41866db47 100644 --- a/examples/cfg-whole-task.rs +++ b/examples/cfg-whole-task.rs @@ -15,7 +15,6 @@ mod app { #[shared] struct Shared { - #[cfg(debug_assertions)] // <- `true` when using the `dev` profile count: u32, #[cfg(never)] unused: u32, @@ -31,7 +30,6 @@ mod app { ( Shared { - #[cfg(debug_assertions)] count: 0, #[cfg(never)] unused: 1,