From b138cc163148ba307cc9548e66efbf90a6482d25 Mon Sep 17 00:00:00 2001 From: Per Lindgren Date: Wed, 3 Nov 2021 20:58:21 +0100 Subject: [PATCH] wip tests do pass, MutexStruct based --- macros/src/codegen/shared_resources_struct.rs | 56 +++++++++++-------- macros/src/codegen/util.rs | 2 +- src/export.rs | 37 ++++++++++-- src/lib.rs | 2 +- ui/lockall_lifetime.stderr | 4 +- ui/lockall_lifetime_destruct_field.rs | 5 -- ui/lockall_lifetime_destruct_field.stderr | 8 +-- ui/lockall_lifetime_reborrow.stderr | 2 +- 8 files changed, 76 insertions(+), 40 deletions(-) diff --git a/macros/src/codegen/shared_resources_struct.rs b/macros/src/codegen/shared_resources_struct.rs index 6ca9710bd6..ababbd8a1b 100644 --- a/macros/src/codegen/shared_resources_struct.rs +++ b/macros/src/codegen/shared_resources_struct.rs @@ -143,22 +143,15 @@ pub fn codegen( ); let ident_mut = util::shared_resources_ident_mut(ctxt, app); - let item = quote!( + let mut items = vec![]; + items.push(quote!( #[allow(non_snake_case)] #[allow(non_camel_case_types)] #[doc = #doc] pub struct #ident<#lt> { #(#fields,)* } - - // Used by the lock-all API - #[allow(non_snake_case)] - #[allow(non_camel_case_types)] - #[doc = #doc_mut] - pub struct #ident_mut<#lt> { - #(#fields_mut,)* - } - ); + )); let arg = if ctxt.is_init() { None @@ -166,7 +159,16 @@ pub fn codegen( Some(quote!(priority: &#lt rtic::export::Priority)) }; - let (lock_all, get_prio) = if let Some(name) = field_get_prio { + let (lock_all, new_struct, get_prio) = if let Some(name) = field_get_prio { + items.push(quote!( + // Used by the lock-all API + #[allow(non_snake_case)] + #[allow(non_camel_case_types)] + #[doc = #doc_mut] + pub struct #ident_mut<#lt> { + #(#fields_mut,)* + } + )); ( util::impl_mutex_struct( extra, @@ -177,6 +179,17 @@ pub fn codegen( quote!(self.priority()), quote!(|| { #ident_mut::new() }), ), + quote!( + // Used by the lock-all API + impl<#lt> #ident_mut<#lt> { + #[inline(always)] + pub unsafe fn new() -> Self { + #ident_mut { + #(#values_mut,)* + } + } + } + ), quote!( // Used by the lock-all API #[inline(always)] @@ -186,7 +199,14 @@ pub fn codegen( ), ) } else { - (quote!(), quote!()) + items.push(quote!( + // Used by the lock-all API + #[allow(non_snake_case)] + #[allow(non_camel_case_types)] + #[doc = #doc_mut] + pub struct #ident_mut {} + )); + (quote!(), quote!(), quote!()) }; let implementations = quote!( @@ -201,18 +221,10 @@ pub fn codegen( #get_prio } - // Used by the lock-all API - impl<#lt> #ident_mut<#lt> { - #[inline(always)] - pub unsafe fn new() -> Self { - #ident_mut { - #(#values_mut,)* - } - } - } + #new_struct #lock_all ); - (item, implementations) + (quote!(#(#items)*), implementations) } diff --git a/macros/src/codegen/util.rs b/macros/src/codegen/util.rs index 671f839a04..7a4c58ce32 100644 --- a/macros/src/codegen/util.rs +++ b/macros/src/codegen/util.rs @@ -71,7 +71,7 @@ pub fn impl_mutex_struct( type T = #ty; #[inline(always)] - fn lock(&mut self, f: impl FnOnce(#ty) -> RTIC_INTERNAL_R) -> RTIC_INTERNAL_R { + fn lock(&mut self, f: impl FnOnce(&mut #ty) -> RTIC_INTERNAL_R) -> RTIC_INTERNAL_R { /// Priority ceiling const CEILING: u8 = #ceiling; diff --git a/src/export.rs b/src/export.rs index c10b340ebc..07a41d73d5 100644 --- a/src/export.rs +++ b/src/export.rs @@ -175,26 +175,27 @@ pub unsafe fn lock_struct( priority: &Priority, ceiling: u8, nvic_prio_bits: u8, - f: impl FnOnce(T) -> R, + f: impl FnOnce(&mut T) -> R, ) -> R { let current = priority.get(); if current < ceiling { if ceiling == (1 << nvic_prio_bits) { priority.set(u8::max_value()); - let r = interrupt::free(|_| f(ptr())); + let r = interrupt::free(|_| f(&mut ptr())); priority.set(current); r } else { priority.set(ceiling); basepri::write(logical2hw(ceiling, nvic_prio_bits)); - let r = f(ptr()); // inside of lock + let r = f(&mut ptr()); // inside of lock + cortex_m::asm::nop(); basepri::write(logical2hw(current, nvic_prio_bits)); priority.set(current); r } } else { - f(ptr()) + f(&mut ptr()) } } @@ -226,6 +227,34 @@ pub unsafe fn lock( } } +/// Lock the resource proxy by setting the PRIMASK +/// and running the closure with interrupt::free +/// +/// # Safety +/// +/// Writing to the PRIMASK +/// Dereferencing a raw pointer +#[cfg(not(armv7m))] +#[inline(always)] +pub unsafe fn lock_struct( + ptr: impl Fn() -> T, + priority: &Priority, + ceiling: u8, + _nvic_prio_bits: u8, + f: impl FnOnce(&mut T) -> R, +) -> R { + let current = priority.get(); + + if current < ceiling { + priority.set(u8::max_value()); + let r = interrupt::free(|_| f(&mut ptr())); + priority.set(current); + r + } else { + f(&mut ptr()) + } +} + #[inline] pub fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 { ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits) diff --git a/src/lib.rs b/src/lib.rs index a78114b165..bb331e9fa8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,5 +111,5 @@ pub trait MutexStruct { type T; /// Creates a critical section and grants temporary access to the protected data - fn lock(&mut self, f: impl FnOnce(Self::T) -> R) -> R; + fn lock(&mut self, f: impl FnOnce(&mut Self::T) -> R) -> R; } diff --git a/ui/lockall_lifetime.stderr b/ui/lockall_lifetime.stderr index d42b5ef5c3..2c1d9ac8cb 100644 --- a/ui/lockall_lifetime.stderr +++ b/ui/lockall_lifetime.stderr @@ -4,5 +4,5 @@ error: lifetime may not live long enough 20 | let _ = c.shared.lock(|s| s); // lifetime | -- ^ returning this value requires that `'1` must outlive `'2` | || - | |return type of closure is &'2 mut __rtic_internal_fooShared - | has type `&'1 mut __rtic_internal_fooShared` + | |return type of closure is &'2 mut __rtic_internal_fooShared<'_> + | has type `&'1 mut __rtic_internal_fooShared<'_>` diff --git a/ui/lockall_lifetime_destruct_field.rs b/ui/lockall_lifetime_destruct_field.rs index 3eb56150ed..3ebbfb107e 100644 --- a/ui/lockall_lifetime_destruct_field.rs +++ b/ui/lockall_lifetime_destruct_field.rs @@ -1,9 +1,4 @@ -//! examples/lockall_soundness.rs - -#![deny(unsafe_code)] -#![deny(warnings)] #![no_main] -#![no_std] use panic_semihosting as _; diff --git a/ui/lockall_lifetime_destruct_field.stderr b/ui/lockall_lifetime_destruct_field.stderr index 67e73d5c28..0a8e4dcb1c 100644 --- a/ui/lockall_lifetime_destruct_field.stderr +++ b/ui/lockall_lifetime_destruct_field.stderr @@ -1,9 +1,9 @@ error: lifetime may not live long enough - --> ui/lockall_lifetime_destruct_field.rs:28:13 + --> ui/lockall_lifetime_destruct_field.rs:23:13 | -27 | let _ = c.shared.lock(|foo::Shared { a }| { +22 | let _ = c.shared.lock(|foo::Shared { a }| { | ------------------ return type of closure is &'2 mut &mut u32 | | - | has type `&'1 mut __rtic_internal_fooShared` -28 | a // lifetime + | has type `&'1 mut __rtic_internal_fooShared<'_>` +23 | a // lifetime | ^ returning this value requires that `'1` must outlive `'2` diff --git a/ui/lockall_lifetime_reborrow.stderr b/ui/lockall_lifetime_reborrow.stderr index b2980c5819..5eac90e5c8 100644 --- a/ui/lockall_lifetime_reborrow.stderr +++ b/ui/lockall_lifetime_reborrow.stderr @@ -5,4 +5,4 @@ error: lifetime may not live long enough | -- ^^^^^^^^^ returning this value requires that `'1` must outlive `'2` | || | |return type of closure is &'2 mut u32 - | has type `&'1 mut __rtic_internal_fooShared` + | has type `&'1 mut __rtic_internal_fooShared<'_>`