diff options
author | 2018-08-31 22:02:29 +0200 | |
---|---|---|
committer | 2018-08-31 22:02:29 +0200 | |
commit | f68e65af8fa0bf915209ae7b3d75303c317ea109 (patch) | |
tree | 736cdd0a577040a0fd9ac3f26951305dabd2f43d | |
parent | f2a155a0715cb99cbace2eca7ab5fcfa93d106d2 (diff) | |
download | cortex-m-f68e65af8fa0bf915209ae7b3d75303c317ea109.tar.gz cortex-m-f68e65af8fa0bf915209ae7b3d75303c317ea109.tar.zst cortex-m-f68e65af8fa0bf915209ae7b3d75303c317ea109.zip |
fix soundness issue; change the syntax of the `exception` attribute
-rw-r--r-- | cortex-m-rt/examples/override-exception.rs | 8 | ||||
-rw-r--r-- | cortex-m-rt/examples/state.rs | 6 | ||||
-rw-r--r-- | cortex-m-rt/macros/Cargo.toml | 2 | ||||
-rw-r--r-- | cortex-m-rt/macros/src/lib.rs | 185 |
4 files changed, 121 insertions, 80 deletions
diff --git a/cortex-m-rt/examples/override-exception.rs b/cortex-m-rt/examples/override-exception.rs index dca31e6..3e0af25 100644 --- a/cortex-m-rt/examples/override-exception.rs +++ b/cortex-m-rt/examples/override-exception.rs @@ -17,13 +17,13 @@ fn main() -> ! { loop {} } -#[exception(DefaultHandler)] -fn default_handler(_irqn: i16) { +#[exception] +fn DefaultHandler(_irqn: i16) { asm::bkpt(); } -#[exception(HardFault)] -fn hard_fault(_ef: &ExceptionFrame) -> ! { +#[exception] +fn HardFault(_ef: &ExceptionFrame) -> ! { asm::bkpt(); loop {} diff --git a/cortex-m-rt/examples/state.rs b/cortex-m-rt/examples/state.rs index 72ca194..573914f 100644 --- a/cortex-m-rt/examples/state.rs +++ b/cortex-m-rt/examples/state.rs @@ -16,7 +16,9 @@ fn main() -> ! { } // exception handler with state -#[exception(SysTick, static STATE: u32 = 0)] -fn sys_tick() { +#[exception] +fn SysTick() { + static mut STATE: u32 = 0; + *STATE += 1; } diff --git a/cortex-m-rt/macros/Cargo.toml b/cortex-m-rt/macros/Cargo.toml index 85e8cd1..d0644ee 100644 --- a/cortex-m-rt/macros/Cargo.toml +++ b/cortex-m-rt/macros/Cargo.toml @@ -8,6 +8,8 @@ proc-macro = true [dependencies] quote = "0.6.6" +rand = "0.5.5" +proc-macro2 = "0.4.15" [dependencies.syn] features = ["extra-traits", "full"] diff --git a/cortex-m-rt/macros/src/lib.rs b/cortex-m-rt/macros/src/lib.rs index 3aa494d..5cba510 100644 --- a/cortex-m-rt/macros/src/lib.rs +++ b/cortex-m-rt/macros/src/lib.rs @@ -1,14 +1,19 @@ -#![deny(warnings)] +// #![deny(warnings)] +#![allow(warnings)] extern crate proc_macro; +extern crate rand; #[macro_use] extern crate quote; #[macro_use] extern crate syn; +extern crate proc_macro2; +use proc_macro2::Span; +use rand::Rng; use syn::synom::Synom; use syn::token::{Colon, Comma, Eq, Static}; -use syn::{Expr, FnArg, Ident, ItemFn, ReturnType, Type, Visibility}; +use syn::{Expr, FnArg, Ident, Item, ItemFn, ReturnType, Stmt, Type, Visibility}; use proc_macro::TokenStream; @@ -124,15 +129,15 @@ impl Synom for State { /// /// ``` /// # use cortex_m_rt_macros::exception; -/// #[exception(SysTick, static COUNT: u32 = 0)] -/// fn handler() { +/// #[exception] +/// fn SysTick() { /// // .. /// } /// /// # fn main() {} /// ``` /// -/// where the first argument can be one of: +/// where the name of the function must be one of: /// /// - `DefaultHandler` /// - `NonMaskableInt` @@ -146,25 +151,29 @@ impl Synom for State { /// - `PendSV` /// - `SysTick` /// -/// and the second is optional. -/// /// (a) Not available on Cortex-M0 variants (`thumbv6m-none-eabi`) /// /// (b) Only available on ARMv8-M /// /// # Usage /// -/// `#[exception(HardFault)]` sets the hard fault handler. The handler must have signature +/// `#[exception] fn HardFault(..` sets the hard fault handler. The handler must have signature /// `fn(&ExceptionFrame) -> !`. This handler is not allowed to return as that can cause undefined /// behavior. /// -/// `#[exception(DefaultHandler)]` sets the *default* handler. All exceptions which have not been -/// assigned a handler will be serviced by this handler. This handler must have signature `fn(irqn: -/// i16)`. `irqn` is the IRQ number (See CMSIS); `irqn` will be a negative number when the handler -/// is servicing a core exception; `irqn` will be a positive number when the handler is servicing a -/// device specific exception (interrupt). +/// `#[exception] fn DefaultHandler(..` sets the *default* handler. All exceptions which have not +/// been assigned a handler will be serviced by this handler. This handler must have signature +/// `fn(irqn: i16)`. `irqn` is the IRQ number (See CMSIS); `irqn` will be a negative number when the +/// handler is servicing a core exception; `irqn` will be a positive number when the handler is +/// servicing a device specific exception (interrupt). +/// +/// `#[exception] fn Name(..` overrides the default handler for the exception with the given `Name`. +/// When overriding these other exception it's possible to add state to them by declaring `static +/// mut` variables at the beginning of the body of the function. These variables will be safe to +/// access from the function body. /// -/// `#[exception(Name)]` overrides the default handler for the exception with the given `Name`. +/// Exception handlers can only be called by the hardware. Other parts of the program can't refer to +/// the exception handler much less invoke them as if they were functions. /// /// # Examples /// @@ -174,8 +183,8 @@ impl Synom for State { /// # extern crate cortex_m_rt; /// # extern crate cortex_m_rt_macros; /// # use cortex_m_rt_macros::exception; -/// #[exception(HardFault)] -/// fn hard_fault(ef: &cortex_m_rt::ExceptionFrame) -> ! { +/// #[exception] +/// fn HardFault(ef: &cortex_m_rt::ExceptionFrame) -> ! { /// // prints the exception frame as a panic message /// panic!("{:#?}", ef); /// } @@ -187,8 +196,8 @@ impl Synom for State { /// /// ``` /// # use cortex_m_rt_macros::exception; -/// #[exception(DefaultHandler)] -/// fn default_handler(irqn: i16) { +/// #[exception] +/// fn DefaultHandler(irqn: i16) { /// println!("IRQn = {}", irqn); /// } /// @@ -202,8 +211,11 @@ impl Synom for State { /// /// use rt::exception; /// -/// #[exception(SysTick, static COUNT: i32 = 0)] -/// fn sys_tick() { +/// #[exception] +/// fn SysTick() { +/// static mut COUNT: i32 = 0; +/// +/// // `COUNT` is safe to access and has type `&mut i32` /// *COUNT += 1; /// /// println!("{}", COUNT); @@ -214,12 +226,14 @@ impl Synom for State { #[proc_macro_attribute] pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { let f: ItemFn = syn::parse(input).expect("`#[exception]` must be applied to a function"); - let args: ExceptionArgs = syn::parse(args).expect( - "`exception` attribute expects the exception name as its argument. \ - e.g. `#[exception(HardFault)]`", + + assert_eq!( + args.to_string(), + "", + "`exception` attribute must have no arguments" ); - let name = args.first; - let name_s = name.to_string(); + + let ident = f.ident; enum Exception { DefaultHandler, @@ -227,22 +241,32 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { Other, } - // first validation of the exception name - let exn = match &*name_s { + let ident_s = ident.to_string(); + let exn = match &*ident_s { "DefaultHandler" => Exception::DefaultHandler, "HardFault" => Exception::HardFault, // NOTE that at this point we don't check if the exception is available on the target (e.g. // MemoryManagement is not available on Cortex-M0) "NonMaskableInt" | "MemoryManagement" | "BusFault" | "UsageFault" | "SecureFault" | "SVCall" | "DebugMonitor" | "PendSV" | "SysTick" => Exception::Other, - _ => panic!("{} is not a valid exception name", name_s), + _ => panic!("{} is not a valid exception name", ident_s), }; // XXX should we blacklist other attributes? let attrs = f.attrs; - let ident = f.ident; let block = f.block; - let stmts = &block.stmts; + let stmts = block.stmts; + + let mut rng = rand::thread_rng(); + let hash = (0..16) + .map(|i| { + if i == 0 || rng.gen() { + ('a' as u8 + rng.gen::<u8>() % 25) as char + } else { + ('0' as u8 + rng.gen::<u8>() % 10) as char + } + }).collect::<String>(); + let hash = Ident::new(&hash, Span::call_site()); match exn { Exception::DefaultHandler => { @@ -262,12 +286,7 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { _ => false, }, }, - "`#[exception(DefaultHandler)]` function must have signature `fn(i16)`" - ); - - assert!( - args.second.is_none(), - "`#[exception(DefaultHandler)]` takes no additional arguments" + "`#DefaultHandler` function must have signature `fn(i16)`" ); let arg = match f.decl.inputs[0] { @@ -276,9 +295,9 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { }; quote!( - #[export_name = #name_s] + #[export_name = #ident_s] #(#attrs)* - pub fn #ident() { + pub extern "C" fn #hash() { extern crate core; const SCB_ICSR: *const u32 = 0xE000_ED04 as *const u32; @@ -318,11 +337,6 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { "`#[exception(HardFault)]` function must have signature `fn(&ExceptionFrame) -> !`" ); - assert!( - args.second.is_none(), - "`#[exception(HardFault)]` takes no additional arguments" - ); - let arg = match f.decl.inputs[0] { FnArg::Captured(ref arg) => arg, _ => unreachable!(), @@ -333,7 +347,7 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { quote!( #[export_name = "UserHardFault"] #(#attrs)* - pub unsafe extern "C" fn #ident(#arg) -> ! { + pub extern "C" fn #hash(#arg) -> ! { extern crate cortex_m_rt; // further type check of the input argument @@ -360,43 +374,66 @@ pub fn exception(args: TokenStream, input: TokenStream) -> TokenStream { _ => false, }, }, - "`#[exception]` functions must have signature `fn()`" + "`#[exception]` functions other than `DefaultHandler` and `HardFault` must \ + have signature `fn()`" ); - if let Some(second) = args.second { - let ty = second.ty; - let expr = second.expr; - let state = second.ident; - - quote!( - #[export_name = #name_s] - #(#attrs)* - pub fn #ident() { - extern crate cortex_m_rt; - - cortex_m_rt::Exception::#name; + // Collect all the `static mut` at the beginning of the function body. We'll make them + // safe + let mut istmts = stmts.into_iter(); + + let mut statics = vec![]; + let mut stmts = vec![]; + while let Some(stmt) = istmts.next() { + match stmt { + Stmt::Item(Item::Static(var)) => if var.mutability.is_some() { + statics.push(var); + } else { + stmts.push(Stmt::Item(Item::Static(var))); + }, + _ => { + stmts.push(stmt); + break; + } + } + } - static mut __STATE__: #ty = #expr; + stmts.extend(istmts); + + let vars = statics + .into_iter() + .map(|var| { + let ident = var.ident; + // `let` can't shadow a `static mut` so we must give the `static` a different + // name. We'll create a new name by appending an underscore to the original name + // of the `static`. + let mut ident_ = ident.to_string(); + ident_.push('_'); + let ident_ = Ident::new(&ident_, Span::call_site()); + let ty = var.ty; + let expr = var.expr; + + quote!( + static mut #ident_: #ty = #expr; + #[allow(non_snake_case, unsafe_code)] + let #ident: &mut #ty = unsafe { &mut #ident_ }; + ) + }).collect::<Vec<_>>(); - #[allow(non_snake_case)] - let #state: &mut #ty = unsafe { &mut __STATE__ }; + quote!( + #[export_name = #ident_s] + #(#attrs)* + pub fn #hash() { + extern crate cortex_m_rt; - #(#stmts)* - } - ).into() - } else { - quote!( - #[export_name = #name_s] - #(#attrs)* - pub fn #ident() { - extern crate cortex_m_rt; + // check that this exception actually exists + cortex_m_rt::Exception::#ident; - cortex_m_rt::Exception::#name; + #(#vars)* - #(#stmts)* - } - ).into() - } + #(#stmts)* + } + ).into() } } } |