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>
