aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Jorge Aparicio <jorge@japaric.io> 2018-08-31 22:02:29 +0200
committerGravatar Jorge Aparicio <jorge@japaric.io> 2018-08-31 22:02:29 +0200
commitf68e65af8fa0bf915209ae7b3d75303c317ea109 (patch)
tree736cdd0a577040a0fd9ac3f26951305dabd2f43d
parentf2a155a0715cb99cbace2eca7ab5fcfa93d106d2 (diff)
downloadcortex-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.rs8
-rw-r--r--cortex-m-rt/examples/state.rs6
-rw-r--r--cortex-m-rt/macros/Cargo.toml2
-rw-r--r--cortex-m-rt/macros/src/lib.rs185
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()
}
}
}