Merge #140

140: fix soundness issue: forbid early returns in init r=japaric a=japaric

TL;DR

1. v0.4.1 will be published once this PR lands

2. v0.4.0 will be yanked once v0.4.1 is out

3. v0.4.1 will reject code that contains early returns in `init` *and* contains
   late resources. Yes, this is a breaking change but such code is unsound /
   has undefined behavior.

4. as of v0.4.1 users are encouraged to use `fn init() -> init::LateResources`
   instead of `fn init()` when they make use of late resources.

---

This PR fixes a soundness issue reported by @RalfJung. Basically, early
returning from `init` leaves *late resources* (runtime initialized statics)
uninitialized, and this produces undefined behavior as tasks rely on those
statics being initialized. The example below showcases a program that runs into
this soundness issue.

``` rust
 #[rtfm::app(device = lm3s6965)]
const APP: () = {
    // this is actually `static mut UNINITIALIZED: MaybeUninit<bool> = ..`
    static mut UNINITIALIZED: bool = ();

    #[init]
    fn init() {
        // early return
        return;

        // this is translated into `UNINITIALIZED.set(true)`
        UNINITIALIZED = true; // the DSL forces you to write this at the end
    }

    #[interrupt(resources = [UNINITIALIZED])]
    fn UART0() {
        // `resources.UNINITIALIZED` is basically `UNINITIALIZED.get_mut()`

        if resources.UNINITIALIZED {
            // undefined behavior
        }
    }
};
```

The fix consists of two parts. The first part is producing a compiler error
whenever the `app` procedural macro finds a `return` expression in `init`. This
covers most cases, except for macros (e.g. `ret!()` expands into `return`) which
cannot be instrospected by procedural macros. This fix is technically a
breaking change (though unlikely to affect real code out there) but as per our
SemVer policy (which follows rust-lang/rust's) we are allowed to make breaking
changes to fix soundness bugs.

The second part of the fix consists of extending the `init` syntax to let the
user return the initial values of late resources in a struct. Namely, `fn() ->
init::LateResources` will become a valid signature for `init` (we allowed this
signature back in v0.3.x). Thus the problematic code shown above can be
rewritten as:

``` rust
 #[rtfm::app(device = lm3s6965)]
const APP: () = {
    static mut UNINITIALIZED: bool = ();

    #[init]
    fn init() -> init::LateResources {
        // rejected by the compiler
        // return; //~ ERROR expected `init::LateResources`, found `()`

        // initialize late resources
        init::LateResources {
            UNINITIALIZED: true,
        }
    }

    #[interrupt(resources = [UNINITIALIZED])]
    fn UART0() {
        if resources.UNINITIALIZED {
            // OK
        }
    }
};
```

Attempting to early return without giving the initial values for late resources
will produce a compiler error.

~~Additionally, we'll emit warnings if the `init: fn()` signature is used to
encourage users to switch to the alternative `init: fn() -> init::LateResources`
signature.~~ Turns out we can't do this on stable. Bummer.

The book and examples have been updated to make use of `init::LateResources`.

In the next minor version release we'll reject `fn init()` if late resources
are declared. `fn init() -> init::LateResources` will become the only way to
initialize late resources.

This PR also prepares release v0.4.1. Once that version is published the unsound
version v0.4.0 will be yanked.


Co-authored-by: Jorge Aparicio <jorge@japaric.io>

7 files changed, 70 insertions, 5 deletions

diff --git a/tests/cfail/early-return-2.rs b/tests/cfail/early-return-2.rs
new file mode 100644
index 0000000..bf867e0
--- /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 0000000..fb695aa
--- /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
+        }
+    }
+};
diff --git a/tests/cfail/init-divergent.rs b/tests/cfail/init-divergent.rs
index 400c805..54813d4 100644
--- a/tests/cfail/init-divergent.rs
+++ b/tests/cfail/init-divergent.rs
@@ -11,7 +11,7 @@ use rtfm::app;
 const APP: () = {
     #[init]
     fn init() -> ! {
-        //~^ ERROR `init` must have type signature `[unsafe] fn()`
+        //~^ ERROR `init` must have type signature `[unsafe] fn() [-> init::LateResources]`
         loop {}
     }
 };
diff --git a/tests/cfail/init-input.rs b/tests/cfail/init-input.rs
index fa79099..3bf0cad 100644
--- a/tests/cfail/init-input.rs
+++ b/tests/cfail/init-input.rs
@@ -11,6 +11,6 @@ use rtfm::app;
 const APP: () = {
     #[init]
     fn init(undef: u32) {
-        //~^ ERROR `init` must have type signature `[unsafe] fn()`
+        //~^ ERROR `init` must have type signature `[unsafe] fn() [-> init::LateResources]`
     }
 };
diff --git a/tests/cfail/init-output.rs b/tests/cfail/init-output.rs
index 1200aca..414a35a 100644
--- a/tests/cfail/init-output.rs
+++ b/tests/cfail/init-output.rs
@@ -11,7 +11,7 @@ use rtfm::app;
 const APP: () = {
     #[init]
     fn init() -> u32 {
-        //~^ ERROR `init` must have type signature `[unsafe] fn()`
+        //~^ ERROR `init` must have type signature `[unsafe] fn() [-> init::LateResources]`
         0
     }
 };
diff --git a/tests/cfail/late-not-send.rs b/tests/cfail/late-not-send.rs
index b9180fe..eb3048d 100644
--- a/tests/cfail/late-not-send.rs
+++ b/tests/cfail/late-not-send.rs
@@ -22,8 +22,10 @@ const APP: () = {
     static mut X: NotSend = ();
 
     #[init]
-    fn init() {
-        X = NotSend { _0: PhantomData };
+    fn init() -> init::LateResources {
+        init::LateResources {
+            X: NotSend { _0: PhantomData },
+        }
     }
 
     #[interrupt(resources = [X])]
diff --git a/tests/cfail/late-uninit.rs b/tests/cfail/late-uninit.rs
index eeb9bd4..55122ed 100644
--- a/tests/cfail/late-uninit.rs
+++ b/tests/cfail/late-uninit.rs
@@ -1,3 +1,5 @@
+// TODO remove in v0.5.x
+
 #![no_main]
 #![no_std]