From 83be1e4877d7a51728a34dcb678aeff29266bb3a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 28 Jul 2024 12:34:04 +0000 Subject: [PATCH] make the Views type miri-safe and add more comments --- src/views.rs | 115 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/src/views.rs b/src/views.rs index 19798737..5fd08c2f 100644 --- a/src/views.rs +++ b/src/views.rs @@ -7,18 +7,82 @@ use orx_concurrent_vec::ConcurrentVec; use crate::Database; +/// A `Views` struct is associated with some specific database type +/// (a `DatabaseImpl` for some existential `U`). It contains functions +/// to downcast from that type to `dyn DbView` for various traits `DbView`. +/// None of these types are known at compilation time, they are all checked +/// dynamically through `TypeId` magic. +/// +/// You can think of the struct as looking like: +/// +/// ```rust,ignore +/// struct Views { +/// source_type_id: TypeId, // `TypeId` for `Db` +/// view_casters: Arc { +/// ViewCaster +/// }>>, +/// } +/// ``` #[derive(Clone)] pub struct Views { source_type_id: TypeId, view_casters: Arc>, } +/// A ViewCaster contains a trait object that can cast from the +/// (ghost) `Db` type of `Views` to some (ghost) `DbView` type. +/// +/// You can think of the struct as looking like: +/// +/// ```rust,ignore +/// struct ViewCaster { +/// target_type_id: TypeId, // TypeId of DbView +/// type_name: &'static str, // type name of DbView +/// cast_to: OpaqueBoxDyn, // a `Box>` that expects a `Db` +/// free_box: Box, // the same box as above, but upcast to `dyn Free` +/// } +/// ``` +/// +/// As you can see, we have to work very hard to manage things +/// in a way that miri is happy with. What is going on here? +/// +/// * The `cast_to` is the cast object, but we can't actually name its type, so +/// we transmute it into some opaque bytes. We can transmute it back once we +/// are in a function monormophized over some function `T` that has the same type-id +/// as `target_type_id`. +/// * The problem is that dropping `cast_to` has no effect and we need +/// to free the box! To do that, we *also* upcast the box to a `Box`. +/// This trait has no purpose but to carry a destructor. struct ViewCaster { + /// The id of the target type `DbView` that we can cast to. target_type_id: TypeId, + + /// The name of the target type `DbView` that we can cast to. type_name: &'static str, - func: fn(&Dummy) -> &Dummy, + + /// A "type-obscured" `Box>`, where `DbView` + /// is the type whose id is encoded in `target_type_id`. + cast_to: OpaqueBoxDyn, + + /// An upcasted version of `cast_to`; the only purpose of this field is + /// to be dropped in the destructor, see `ViewCaster` comment. + #[allow(dead_code)] + free_box: Box, } +type OpaqueBoxDyn = [u8; std::mem::size_of::>>()]; + +trait CastTo: Free { + /// # Safety requirement + /// + /// `db` must have a data pointer whose type is the `Db` type for `Self` + unsafe fn cast<'db>(&self, db: &'db dyn Database) -> &'db DbView; + + fn into_box_free(self: Box) -> Box; +} + +trait Free: Send + Sync {} + #[allow(dead_code)] enum Dummy {} @@ -45,10 +109,21 @@ impl Views { return; } + let cast_to: Box> = Box::new(func); + let cast_to: OpaqueBoxDyn = + unsafe { std::mem::transmute::>, OpaqueBoxDyn>(cast_to) }; + + // Create a second copy of `cast_to` (which is now `Copy`) and upcast it to a `Box`. + // We will drop this box to run the destructor. + let free_box: Box = unsafe { + std::mem::transmute::>>(cast_to).into_box_free() + }; + self.view_casters.push(ViewCaster { target_type_id, type_name: std::any::type_name::(), - func: unsafe { std::mem::transmute:: &DbView, fn(&Dummy) -> &Dummy>(func) }, + cast_to, + free_box, }); } @@ -73,8 +148,12 @@ impl Views { // While the compiler doesn't know what `X` is at this point, we know it's the // same as the true type of `db_data_ptr`, and the memory representation for `()` // and `&X` are the same (since `X` is `Sized`). - let func: fn(&()) -> &DbView = unsafe { std::mem::transmute(caster.func) }; - return Some(func(data_ptr(db))); + let cast_to: &OpaqueBoxDyn = &caster.cast_to; + unsafe { + let cast_to = + std::mem::transmute::<&OpaqueBoxDyn, &Box>>(cast_to); + return Some(cast_to.cast(db)); + }; } } @@ -98,8 +177,32 @@ impl std::fmt::Debug for ViewCaster { /// Given a wide pointer `T`, extracts the data pointer (typed as `()`). /// This is safe because `()` gives no access to any data and has no validity requirements in particular. -fn data_ptr(t: &T) -> &() { +unsafe fn data_ptr(t: &T) -> &U { let t: *const T = t; - let u: *const () = t as *const (); + let u: *const U = t as *const U; unsafe { &*u } } + +impl CastTo for fn(&Db) -> &DbView +where + Db: Database, + DbView: ?Sized + Any, +{ + unsafe fn cast<'db>(&self, db: &'db dyn Database) -> &'db DbView { + // This tests the safety requirement: + debug_assert_eq!(db.type_id(), TypeId::of::()); + + // SAFETY: + // + // Caller guarantees that the input is of type `Db` + // (we test it in the debug-assertion above). + let db = unsafe { data_ptr::(db) }; + (*self)(db) + } + + fn into_box_free(self: Box) -> Box { + self + } +} + +impl Free for T {}