Comments (12)
Status update:
There is now a corresponding POC:
https://github.com/rtic-rs/cortex-m-rtic/tree/lockall
There is still an outstanding issue regarding the resource proxy for the lock-all API. It currently exposes the priority (Priority type). At some point, we deemed that to be risky, I don't recall now exactly how it can potentially be exploited. In any case that problem should be solvable if we decide to include the lockall API in the RTIC 0.6 release.
Please go ahead an try it. The examples/lockall*.rs should give you an idea how it can be used.
from rfcs.
Status update:
The Priority leakage has been fixed in:
https://github.com/rtic-rs/cortex-m-rtic/tree/lockall
from rfcs.
Status update:
Added a comparison examples/multilock_vs_lockall
. The correct field types are inferred by current rust-analyzer
.
from rfcs.
Status update:
The current multi-lock moves the resource proxies into the lock
. While this is sound it blocks further locks, so even outside/after the lock
closure there is no way to recover the proxies. See e.g., examples/multilock_problem.rs
.
from rfcs.
Status update:
A workaround to the above mentioned problem is to pass mutable references to proxies instead of moving the proxies.
See, e.g. examples/multilock_problem_workaround.rs
.
from rfcs.
@perlindgren the internal use of &'static mut _
references strikes me as 'Undefined-Behavior wrong' / 'semantically wrong'. Not that I can show you how to make it explode today but I would put it under the category 'some future change in rustc/LLVM codegen may make this Undefined Behavior in the future'
I would suggest creating / using a separate trait to achieve the right lifetime constraints:
NOTE: this is a sketch & syntax/API from memory; I didn't try to compile it so it may be wrong in a few places
/* input RTIC code */
struct Shared {
x: i32,
y: i64,
z: bool,
}
#[task(shared = [x, y])]
fn f(cx: f::Context) {
// (the move is NOT need; this is just to show the type name)
let mut shared: f::SharedResources = cx.shared;
cx.shared.lock(|shared: f::SharedMut| { // <- SharedMutex trait
*shared.x *= 1;
*shared.y *= 1;
});
// resource proxies can still be moved out and used independently
// (again, the move is just to show the type name)
let mut x: resources::X = shared.x;
x.lock(|x| { // <- Mutex trait
*x += 1;
});
}
/* generated code */
// this is the existing codegen (simplified for brevity)
mod resources {
// proxies
struct X;
struct Y;
impl Mutex for X { /* .. */ }
impl Mutex for Y { /* .. */ }
}
mod f {
struct SharedResources {
x: crate::resources::X,
y: crate::resources::Y,
}
// this is the new codegen
// NO `'static` lifetimes
struct SharedMut<'a> {
x: &'a mut i32,
y: &'a mut i64,
}
impl<'a> SharedMutex for &'a mut SharedResources {
// ^^ != 'static ^^^^^^^ reference, not plain type
//
type T = SharedMut<'a>;
// ^^ connect lifetime to closure argument
fn lock<R>(self, f: impl FnOnce(SharedMut<'a>) -> R) -> R {
// ^^^^ Self is &'a mut SharedResources
// do locking here
}
}
}
// this is the new trait
trait SharedMutex {
type T;
fn lock<R>(self, f: impl FnOnce(Self::T) -> R) -> R;
// ^^^^ NOTE by value, not by reference (`&mut self`)
}
from rfcs.
@japaric, I tried something similar before residing to &'static mut
in the current API, but got into a problem with lifetimes. I did not use a separate trait with move semantics though. That would also make it a bit asymmetric to the current lock. (Where you would not be able to re-lock resources unless you move the struct back, and I'm not sure how that would work, as we are not giving the proxy as a parameter to the closure (just the locked struct).
I'm not 100% sure what in Rust's semantics that actually saves us of from unsoundness in the current proposal, but it seems that when lending out an &'a mut Shared { a: &static mut T }
the inner &'static mut
will be demoted to `a. If this is defined somewhere, the current proposal should be fine (and it is the behavior of the current compiler).
https://doc.rust-lang.org/reference/subtyping.html
Is not conclusive to this point, (under what lifetime the &`static field will be accessible, but the only thing that makes sense is to assume the stricter lifetime).
from rfcs.
By the way, I verified that at least for trivial cases the abstraction is zero cost and the lock would be optimized out. We rely on Rust+LLVM to evaluate the priority at compile time. In the general case this might become too complex for Rust+LLVM to figure out, and the lock would not be optimized out, but I haven't seen that in practice in my small experiments.
from rfcs.
After some additional testing I haven't found a way to break the proposed API.
In examples/lockall_soundness3.rs
an attempt to move out the locked struct is rejected (due to lifetime error). Essentially, together with the prior soundness tests (1/2) this should cover the possibilities to break the API.
Regarding performance, to more examples were added (examples/lock_cost.rs
and examples/lockall_cost.rs
). The features:
[features]
inline-asm = ["cortex-m/inline-asm"]
linker-plugin-lto = ["cortex-m/linker-plugin-lto"]
were added to Cargo.toml
(not sure we want them there finally, maybe there is some possibility to have these features available only for building examples...?).
I added two examples lock_cost
and lockall_cost
for comparison. Code for further analysis are generated by:
cargo objdump --example lock_cost --target thumbv7m-none-eabi --release --features inline-asm -- --disassemble > lock_cost.objdump
When comparing the lock
and lockall
they generate exactly the same code, so performance should be Ok (zero cost). In both cases a taken lock will generate:
...
#[task(binds = GPIOA, shared = [shared])]
fn low(mut cx: low::Context) {
cx.shared.shared.lock(|shared| *shared += 1);
}
#[task(binds = GPIOB, priority = 2, shared = [shared])]
fn high(mut cx: high::Context) {
cx.shared.shared.lock(|shared| *shared += 1);
}
// 0000016c <GPIOA>:
// 16c: 80 b5 push {r7, lr}
// 16e: 6f 46 mov r7, sp
// 170: c0 20 movs r0, #192
// 172: 80 f3 11 88 msr basepri, r0
// 176: 40 f2 00 00 movw r0, #0
// 17a: c2 f2 00 00 movt r0, #8192
// 17e: 01 68 ldr r1, [r0]
// 180: 01 31 adds r1, #1
// 182: 01 60 str r1, [r0]
// 184: e0 20 movs r0, #224
// 186: 80 f3 11 88 msr basepri, r0
// 18a: 00 20 movs r0, #0
// 18c: 80 f3 11 88 msr basepri, r0
// 190: 80 bd pop {r7, pc}
In the above case the lock is taken (low prio).
// 00000192 <GPIOB>:
// 192: 80 b5 push {r7, lr}
// 194: 6f 46 mov r7, sp
// 196: 40 f2 00 01 movw r1, #0
// 19a: ef f3 11 80 mrs r0, basepri
// 19e: c2 f2 00 01 movt r1, #8192
// 1a2: 0a 68 ldr r2, [r1]
// 1a4: 01 32 adds r2, #1
// 1a6: 0a 60 str r2, [r1]
// 1a8: 80 f3 11 88 msr basepri, r0
// 1ac: 80 bd pop {r7, pc}
In the above the lock is optimized out.
The very same code is generated for lock
and lock_all
. In both cases the code could be further optimized according to 19, but the improvement is minor (we are already "zero cost" in the big picture).
In any case the proposed API does not introduce any additional OH, at least not for simple examples.
from rfcs.
(revisiting this thread after some vacation away from the computer)
I'm not 100% sure what in Rust's semantics that actually saves us of from unsoundness in the current proposal, but it seems that when lending out an &'a mut Shared { a: &static mut T } the inner &'static mut will be demoted to `a
I'd be more concerned about (LLVM) codegen than the borrow checker or other type check not working adequately. A future version of rustc could produce LLVM IR in such a way the creation of a static reference (&'static mut T
) in more than one execution path during the lifetime of the program is Undefined Behavior. Limiting the creation of &'static mut T
to at most once in the lifetime of the program strikes me as valid (a valid optimization). Your use of static lifetimes creates a &'static mut T
on each lock, which could happen more than once in the lifetime of the program.
Yes, you create a static reference and then reborrow it with a non-static lifetime before you hand it to the user but the intermediate step is hypothetically / potentially problematic. This is a bit like how you are not allowed to create a reference to an uninitialized struct field and then create a raw pointer from it &mut x as *mut T
because that's Undefined Behavior; instead you should use ptr::addr_of_mut!
to directly create a raw pointer (*mut T
) without the intermediate reference (&mut T
).
In any case, this is all speculation because I don't think creation of static references (&'static mut
) is well specified in the language or covered by the unsafe code guidelines at the moment.
I did not use a separate trait with move semantics though.
Note that the trait uses self
but does not consume the caller because it's implemented on a mutable reference so the usual re-borrowing rules apply (in non-generic context) when calling the trait method. If you want to use a method with a &mut self
receiver instead of a self
receiver you would need to use the GAT (Generic Associated Types) feature (which has not been implemented yet) to adequately constrain the lifetimes without using the intermediate 'static
lifetime.
from rfcs.
Thanks for your input japaric. I made some further experimentation, the current version does not use &'static mut
, but rather &'a mut
, however it may still suffer from unsoundness due to the points you mentioned.
The whole idea creating a structure reflecting the accessible shared resources as fields is not the only way though.
Maybe it would be better to create a tuple (like done now with the macro). The advantage over the current macro based multi-lock, would be ergonomics (we can lock all without splitting), and secondly performance (guaranteed to have a single critical section). This would also allow us to use the current way of dealing out the resources to the closure (so symmetric). Should not be too complex to implement either.
from rfcs.
Thinking twice on the problem, tuples are indeed treated as structs in Rust, so I'm not sure it will make any difference to the soundness. However, the idea is not "bad", instead of giving an array of shared resources (for a task) the syntax could be changed to a tuple (and the order of elements would be the same for the declaration and the lockall api). This way the struct name would not need to be defined, so a bit less "magic" overall.
from rfcs.
Related Issues (20)
- Memory Lane - Zero Cost Logging with deferred formatting HOT 4
- #[init()] functions should provide a CriticalSection HOT 5
- [tracking issue] 1.0 (previously 0.6) push HOT 7
- Resource proxy mutabability, mutex Trait and closure captures (async fn) HOT 7
- SeqCst around critical sections.
- Spawn and message queues. HOT 1
- Synchroneous task
- Emulation of Monotonic Timers
- Resource handling take 2 HOT 1
- RTIC on RISC-V - roadmap / tasks HOT 1
- Tracing RTIC tasks over ITM HOT 4
- Async idle
- Exporting app description during compliation for tracing purposes? HOT 2
- Add support for the async functions HOT 1
- Soundness of RTIC
- Fine grained concurrency on thumbv6m (no BASEPRI). HOT 1
- Modular RTIC HOT 2
- Type based Layout for Rust RTIC
- rtic-sync: DynSender and DynReceiver HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rfcs.