From 2a896c44f6ff696e9dd0bce40a6a48d6ae25c89a Mon Sep 17 00:00:00 2001 From: matejcik Date: Thu, 27 Jun 2024 21:07:37 +0200 Subject: [PATCH] feat(core/rust): introduce GcBox --- core/embed/rust/build.rs | 1 + core/embed/rust/src/micropython/gc.rs | 228 +++++++++++++++++++++++- core/embed/rust/src/micropython/list.rs | 30 ++-- 3 files changed, 245 insertions(+), 14 deletions(-) diff --git a/core/embed/rust/build.rs b/core/embed/rust/build.rs index 9d664cd788..6f2706e555 100644 --- a/core/embed/rust/build.rs +++ b/core/embed/rust/build.rs @@ -235,6 +235,7 @@ fn generate_micropython_bindings() { .allowlist_var("mp_type_fun_builtin_var") // gc .allowlist_function("gc_alloc") + .allowlist_function("gc_free") .allowlist_var("GC_ALLOC_FLAG_HAS_FINALISER") // iter .allowlist_type("mp_obj_iter_buf_t") diff --git a/core/embed/rust/src/micropython/gc.rs b/core/embed/rust/src/micropython/gc.rs index e283cbd7db..8a9d103498 100644 --- a/core/embed/rust/src/micropython/gc.rs +++ b/core/embed/rust/src/micropython/gc.rs @@ -1,6 +1,6 @@ use core::{ alloc::Layout, - ops::Deref, + ops::{Deref, DerefMut}, ptr::{self, NonNull}, }; @@ -33,6 +33,10 @@ impl Gc { /// that have a base as their first element unsafe fn alloc(v: T, flags: u32) -> Result { let layout = Layout::for_value(&v); + debug_assert!( + layout.size() > 0, + "Zero-sized allocations are not supported" + ); // TODO: Assert that `layout.align()` is the same as the GC alignment. // SAFETY: // - Unfortunately we cannot respect `layout.align()` as MicroPython GC does @@ -161,3 +165,225 @@ impl Deref for Gc { unsafe { self.0.as_ref() } } } + +/// Box-like allocation on the GC heap. +/// +/// Values allocated using GcBox are guaranteed to be only owned by that +/// particular GcBox instance. This makes them safe to mutate, and they run +/// destructors when dropped. +/// +/// Suitable for use in cases where you would normally use Rust's native Box +/// type -- i.e., typically for storing sub-values in a struct, possibly also +/// for returning values from functions. +/// +/// While general unsizing is not available, you can use the `coerce!` macro to +/// safely cast the box to a trait object. +/// +/// # Safety and usage notes +/// +/// One caveat of using GcBox is that it still always needs to be visible to the +/// GC -- that is, stored in a struct which is allocated on the GC heap, on the +/// call stack, or reachable from one of the GC roots. +/// +/// Specifically, it is generally unsafe to store a GcBox in a global variable. +/// +/// When a GcBox is stored in a struct, which itself is allocated via raw GC, +/// the containing struct might get GC'd, which will cause GcBox not to get +/// dropped, which in turn will prevent GcBox's contents from getting dropped. +pub struct GcBox(Gc); + +impl GcBox { + /// Allocate memory on the heap managed by the MicroPython GC and then place + /// `value` into it. + /// + /// `value` _will_ get its Drop implementation called when the GcBox is + /// dropped. + pub fn new(value: T) -> Result { + Ok(Self(Gc::new(value)?)) + } +} + +impl GcBox { + /// Leak contents of the box as a pointer. + /// + /// # Safety + /// + /// The value will not be dropped. If necessary, the caller is responsible + /// for dropping it manually, e.g., via `ptr::drop_in_place`. + pub fn into_raw(this: Self) -> *mut T { + let result = Gc::into_raw(this.0); + core::mem::forget(this); + result + } + + /// Construct a `GcBox` from a raw pointer. + /// + /// # Safety + /// + /// This is only safe for pointers _allocated on the MicroPython GC heap_ + /// via `gc_alloc()`. Specifically, unlike `Gc::from_raw`, it is unsafe + /// to construct a GcBox from ROM values, even those that are trackable + /// by the GC. + /// + /// This is because the Drop implementation of GcBox calls `gc_free()` on + /// the pointer. + /// + /// In addition, the caller must ensure that it is safe to apply box-like + /// semantics to the value, namely that: + /// * nobody else has the pointer to the value, so that it is safe to + /// create mutable references to it + /// * the value is going to be dropped when the GcBox is dropped. + pub unsafe fn from_raw(ptr: *mut T) -> Self { + // SAFETY: just a wrapper around Gc::from_raw + Self(unsafe { Gc::from_raw(ptr) }) + } + + /// Leak contents of the box as a regular Gc allocation. + /// + /// This gives up the unique ownership. It is no longer possible to safely + /// mutably borrow the value, and its destructor will not be called when + /// it is dropped. In exchange, it is possible to return Gc instance to + /// MicroPython. + pub fn leak(self) -> Gc { + let inner = self.0; + core::mem::forget(self); + inner + } +} + +/// Type-cast GcBox contents to a `dyn Trait` object. +macro_rules! coerce { + ($t:path, $v:expr) => { + // SAFETY: we are just re-wrapping the pointer, so all safety requirements + // of `GcBox::from_raw` are upheld. + // Rust type system will not allow us to cast to a trait object that is not + // implemented by the type. + unsafe { GcBox::from_raw(GcBox::into_raw($v) as *mut dyn $t) } + }; +} + +pub(crate) use coerce; + +impl Deref for GcBox { + type Target = T; + + fn deref(&self) -> &Self::Target { + self.0.deref() + } +} + +impl DerefMut for GcBox { + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: We are the sole owner of the allocated value, and we are borrowed + // mutably. + unsafe { Gc::as_mut(&mut self.0) } + } +} + +impl Drop for GcBox { + fn drop(&mut self) { + let ptr = Gc::into_raw(self.0); + // SAFETY: We are the sole owner of the allocated value, and we are being + // dropped. + unsafe { + ptr::drop_in_place(ptr); + ffi::gc_free(ptr.cast()); + } + } +} + +#[cfg(test)] +mod test { + use core::cell::Cell; + + use crate::micropython::testutil::mpy_init; + + use super::*; + + struct SignalDrop<'a>(&'a Cell); + + impl Drop for SignalDrop<'_> { + fn drop(&mut self) { + self.0.set(true); + } + } + + trait Foo { + fn foo(&self) -> i32; + } + + impl Foo for SignalDrop<'_> { + fn foo(&self) -> i32 { + 42 + } + } + + #[test] + fn gc_nodrop() { + unsafe { mpy_init() }; + + let drop_signalled = Cell::new(false); + { + let _gc = Gc::new(SignalDrop(&drop_signalled)).unwrap(); + } + assert!(!drop_signalled.get()); + } + + #[test] + fn gcbox_drop() { + unsafe { mpy_init() }; + + let drop_signalled = Cell::new(false); + { + let _gcbox = GcBox::new(SignalDrop(&drop_signalled)).unwrap(); + } + assert!(drop_signalled.get()); + } + + #[test] + fn gc_raw_roundtrip() { + unsafe { mpy_init() }; + + let gc = Gc::new(42).unwrap(); + let ptr = Gc::into_raw(gc); + let wrapped = unsafe { Gc::from_raw(ptr) }; + let retrieved = Gc::into_raw(wrapped); + assert_eq!(ptr, retrieved); + } + + #[test] + fn gcbox_raw_roundtrip() { + unsafe { mpy_init() }; + + let drop_signalled = Cell::new(false); + + { + let gcbox = GcBox::new(SignalDrop(&drop_signalled)).unwrap(); + assert!(!drop_signalled.get()); + let ptr = GcBox::into_raw(gcbox); + assert!(!drop_signalled.get()); + let wrapped = unsafe { GcBox::from_raw(ptr) }; + assert!(!drop_signalled.get()); + let retrieved = GcBox::into_raw(wrapped); + assert!(!drop_signalled.get()); + assert_eq!(ptr, retrieved); + + let _rewrapped = unsafe { GcBox::from_raw(ptr) }; + } + assert!(drop_signalled.get()); + } + + #[test] + fn test_coerce() { + unsafe { mpy_init() }; + + let drop_signalled = Cell::new(false); + { + let gcbox = GcBox::new(SignalDrop(&drop_signalled)).unwrap(); + let coerced: GcBox = coerce!(Foo, gcbox); + assert!(!drop_signalled.get()); + assert_eq!(coerced.foo(), 42); + } + assert!(drop_signalled.get()); + } +} diff --git a/core/embed/rust/src/micropython/list.rs b/core/embed/rust/src/micropython/list.rs index 42da263139..3073adbc72 100644 --- a/core/embed/rust/src/micropython/list.rs +++ b/core/embed/rust/src/micropython/list.rs @@ -2,7 +2,12 @@ use core::{convert::TryFrom, ptr}; use crate::error::Error; -use super::{ffi, gc::Gc, obj::Obj, runtime::catch_exception}; +use super::{ + ffi, + gc::{Gc, GcBox}, + obj::Obj, + runtime::catch_exception, +}; pub type List = ffi::mp_obj_list_t; @@ -17,29 +22,29 @@ impl List { }) } - pub fn with_capacity(capacity: usize) -> Result, Error> { + pub fn with_capacity(capacity: usize) -> Result, Error> { // EXCEPTION: Will raise if allocation fails. catch_exception(|| unsafe { let list = ffi::mp_obj_new_list(capacity, ptr::null_mut()); // By default, the new list will have its len set to n. We want to preallocate // to a specific size and then use append() to add items, so we reset len to 0. ffi::mp_obj_list_set_len(list, 0); - Gc::from_raw(list.as_ptr().cast()) + // SAFETY: list is freshly allocated so we are still its unique owner. + GcBox::from_raw(list.as_ptr().cast()) }) } - pub fn from_iter(iter: impl Iterator) -> Result, Error> + pub fn from_iter(iter: impl Iterator) -> Result, Error> where T: TryInto, Error: From, { let max_size = iter.size_hint().1.unwrap_or(0); - let mut gc_list = List::with_capacity(max_size)?; - let list = unsafe { Gc::as_mut(&mut gc_list) }; + let mut list = List::with_capacity(max_size)?; for value in iter { list.append(value.try_into()?)?; } - Ok(gc_list) + Ok(list) } // Internal helper to get the `Obj` variant of this. @@ -155,7 +160,7 @@ mod tests { // create an upy list of 5 elements let vec: Vec = (0..5).collect(); - let list: Obj = List::from_iter(vec.iter().copied()).unwrap().into(); + let list: Obj = List::from_iter(vec.iter().copied()).unwrap().leak().into(); // collect the elements into a Vec of maximum length 10, through an iterator let retrieved_vec: Vec = IterBuf::new() @@ -181,8 +186,7 @@ mod tests { unsafe { mpy_init() }; let vec: Vec = (0..17).collect(); - let mut gc_list = List::from_iter(vec.iter().copied()).unwrap(); - let list = unsafe { Gc::as_mut(&mut gc_list) }; + let mut list = List::from_iter(vec.iter().copied()).unwrap(); for (i, value) in vec.iter().copied().enumerate() { assert_eq!( @@ -197,7 +201,7 @@ mod tests { } let retrieved_vec: Vec = IterBuf::new() - .try_iterate(gc_list.into()) + .try_iterate(list.leak().into()) .unwrap() .map(TryInto::try_into) .collect::, Error>>() @@ -229,7 +233,7 @@ mod tests { let vec: Vec = (0..5).collect(); let mut list = List::from_iter(vec.iter().copied()).unwrap(); - let slice = unsafe { Gc::as_mut(&mut list).as_mut_slice() }; + let slice = unsafe { list.as_mut_slice() }; assert_eq!(slice.len(), vec.len()); assert_eq!(vec[0], TryInto::::try_into(slice[0]).unwrap()); @@ -238,7 +242,7 @@ mod tests { } let retrieved_vec: Vec = IterBuf::new() - .try_iterate(list.into()) + .try_iterate(list.leak().into()) .unwrap() .map(TryInto::try_into) .collect::, Error>>()