From 557a51ede11b39acd37dcadb5bd3e12c4e7e7b28 Mon Sep 17 00:00:00 2001 From: Jorge Aparicio Date: Tue, 12 Feb 2019 11:07:15 +0100 Subject: [PATCH] forbid early returns in init --- macros/src/check.rs | 247 +++++++++++++++++++++++++++++++++- tests/cfail/early-return-2.rs | 29 ++++ tests/cfail/early-return.rs | 32 +++++ 3 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 tests/cfail/early-return-2.rs create mode 100644 tests/cfail/early-return.rs diff --git a/macros/src/check.rs b/macros/src/check.rs index ae2262a859..0bc31c5ff9 100644 --- a/macros/src/check.rs +++ b/macros/src/check.rs @@ -1,7 +1,7 @@ use std::{collections::HashSet, iter}; use proc_macro2::Span; -use syn::parse; +use syn::{parse, spanned::Spanned, Block, Expr, Stmt}; use crate::syntax::App; @@ -112,5 +112,250 @@ pub fn app(app: &App) -> parse::Result<()> { } } + // Check that `init` contains no early returns *if* late resources exist + if app.resources.values().any(|res| res.expr.is_none()) { + for stmt in &app.init.stmts { + noreturn_stmt(stmt)?; + } + } + + Ok(()) +} + +// checks that the given block contains no instance of `return` +fn noreturn_block(block: &Block) -> Result<(), parse::Error> { + for stmt in &block.stmts { + noreturn_stmt(stmt)?; + } + + Ok(()) +} + +// checks that the given statement contains no instance of `return` +fn noreturn_stmt(stmt: &Stmt) -> Result<(), parse::Error> { + match stmt { + // `let x = ..` -- this may contain a return in the RHS + Stmt::Local(local) => { + if let Some(ref init) = local.init { + noreturn_expr(&init.1)? + } + } + + // items have no effect on control flow + Stmt::Item(..) => {} + + Stmt::Expr(expr) => noreturn_expr(expr)?, + + Stmt::Semi(expr, ..) => noreturn_expr(expr)?, + } + + Ok(()) +} + +// checks that the given expression contains no `return` +fn noreturn_expr(expr: &Expr) -> Result<(), parse::Error> { + match expr { + Expr::Box(b) => noreturn_expr(&b.expr)?, + + Expr::InPlace(ip) => { + noreturn_expr(&ip.place)?; + noreturn_expr(&ip.value)?; + } + + Expr::Array(a) => { + for elem in &a.elems { + noreturn_expr(elem)?; + } + } + + Expr::Call(c) => { + noreturn_expr(&c.func)?; + + for arg in &c.args { + noreturn_expr(arg)?; + } + } + + Expr::MethodCall(mc) => { + noreturn_expr(&mc.receiver)?; + + for arg in &mc.args { + noreturn_expr(arg)?; + } + } + + Expr::Tuple(t) => { + for elem in &t.elems { + noreturn_expr(elem)?; + } + } + + Expr::Binary(b) => { + noreturn_expr(&b.left)?; + noreturn_expr(&b.right)?; + } + + Expr::Unary(u) => { + noreturn_expr(&u.expr)?; + } + + Expr::Lit(..) => {} + + Expr::Cast(c) => { + noreturn_expr(&c.expr)?; + } + + Expr::Type(t) => { + noreturn_expr(&t.expr)?; + } + + Expr::Let(l) => { + noreturn_expr(&l.expr)?; + } + + Expr::If(i) => { + noreturn_expr(&i.cond)?; + + noreturn_block(&i.then_branch)?; + + if let Some(ref e) = i.else_branch { + noreturn_expr(&e.1)?; + } + } + + Expr::While(w) => { + noreturn_expr(&w.cond)?; + noreturn_block(&w.body)?; + } + + Expr::ForLoop(fl) => { + noreturn_expr(&fl.expr)?; + noreturn_block(&fl.body)?; + } + + Expr::Loop(l) => { + noreturn_block(&l.body)?; + } + + Expr::Match(m) => { + noreturn_expr(&m.expr)?; + + for arm in &m.arms { + if let Some(g) = &arm.guard { + noreturn_expr(&g.1)?; + } + + noreturn_expr(&arm.body)?; + } + } + + // we don't care about `return`s inside closures + Expr::Closure(..) => {} + + Expr::Unsafe(u) => { + noreturn_block(&u.block)?; + } + + Expr::Block(b) => { + noreturn_block(&b.block)?; + } + + Expr::Assign(a) => { + noreturn_expr(&a.left)?; + noreturn_expr(&a.right)?; + } + + Expr::AssignOp(ao) => { + noreturn_expr(&ao.left)?; + noreturn_expr(&ao.right)?; + } + + Expr::Field(f) => { + noreturn_expr(&f.base)?; + } + + Expr::Index(i) => { + noreturn_expr(&i.expr)?; + noreturn_expr(&i.index)?; + } + + Expr::Range(r) => { + if let Some(ref f) = r.from { + noreturn_expr(f)?; + } + + if let Some(ref t) = r.to { + noreturn_expr(t)?; + } + } + + Expr::Path(..) => {} + + Expr::Reference(r) => { + noreturn_expr(&r.expr)?; + } + + Expr::Break(b) => { + if let Some(ref e) = b.expr { + noreturn_expr(e)?; + } + } + + Expr::Continue(..) => {} + + Expr::Return(r) => { + return Err(parse::Error::new( + r.span(), + "`init` is *not* allowed to early return", + )); + } + + // we can not analyze this + Expr::Macro(..) => {} + + Expr::Struct(s) => { + for field in &s.fields { + noreturn_expr(&field.expr)?; + } + + if let Some(ref rest) = s.rest { + noreturn_expr(rest)?; + } + } + + Expr::Repeat(r) => { + noreturn_expr(&r.expr)?; + noreturn_expr(&r.len)?; + } + + Expr::Paren(p) => { + noreturn_expr(&p.expr)?; + } + + Expr::Group(g) => { + noreturn_expr(&g.expr)?; + } + + Expr::Try(t) => { + noreturn_expr(&t.expr)?; + } + + // we don't care about `return`s inside async blocks + Expr::Async(..) => {} + + Expr::TryBlock(tb) => { + noreturn_block(&tb.block)?; + } + + Expr::Yield(y) => { + if let Some(expr) = &y.expr { + noreturn_expr(expr)?; + } + } + + // we can not analyze this + Expr::Verbatim(..) => {} + } + Ok(()) } diff --git a/tests/cfail/early-return-2.rs b/tests/cfail/early-return-2.rs new file mode 100644 index 0000000000..bf867e0720 --- /dev/null +++ b/tests/cfail/early-return-2.rs @@ -0,0 +1,29 @@ +#![no_main] +#![no_std] + +extern crate lm3s6965; +extern crate panic_halt; +extern crate rtfm; + +use rtfm::app; + +#[app(device = lm3s6965)] +const APP: () = { + static mut UNINITIALIZED: bool = (); + + #[init] + fn init() { + if false { + return; //~ ERROR `init` is *not* allowed to early return + } + + UNINITIALIZED = true; + } + + #[interrupt(resources = [UNINITIALIZED])] + fn UART0() { + if resources.UNINITIALIZED { + // UB + } + } +}; diff --git a/tests/cfail/early-return.rs b/tests/cfail/early-return.rs new file mode 100644 index 0000000000..fb695aac97 --- /dev/null +++ b/tests/cfail/early-return.rs @@ -0,0 +1,32 @@ +#![no_main] +#![no_std] + +extern crate lm3s6965; +extern crate panic_halt; +extern crate rtfm; + +use rtfm::app; + +#[app(device = lm3s6965)] +const APP: () = { + static mut UNINITIALIZED: bool = (); + + #[init] + fn init() { + let x = || { + // this is OK + return 0; + }; + + return; //~ ERROR `init` is *not* allowed to early return + + UNINITIALIZED = true; + } + + #[interrupt(resources = [UNINITIALIZED])] + fn UART0() { + if resources.UNINITIALIZED { + // UB + } + } +};