Re: [PATCH 4/5] rust: qom: make ParentInit lifetime-invariant

[Zhao Liu <zhao1.liu@intel.com>]

On Mon, Jun 09, 2025 at 05:44:22PM +0200, Paolo Bonzini wrote:
> Date: Mon,  9 Jun 2025 17:44:22 +0200
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 4/5] rust: qom: make ParentInit lifetime-invariant
> X-Mailer: git-send-email 2.49.0
> 
> This is the trick that allows the parent-field initializer to be used
> only for the object that it's meant to be initialized.  This way,
> the owner of a MemoryRegion must be the object that embeds it.
> 
> More information is in the comments; it's best explained with a simplified
> example.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  rust/qemu-api/src/qom.rs | 88 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 84 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
> index 21c271cd2f9..1481ef20f0c 100644
> --- a/rust/qemu-api/src/qom.rs
> +++ b/rust/qemu-api/src/qom.rs
> @@ -95,6 +95,7 @@
>  use std::{
>      ffi::{c_void, CStr},
>      fmt,
> +    marker::PhantomData,
>      mem::{ManuallyDrop, MaybeUninit},
>      ops::{Deref, DerefMut},
>      ptr::NonNull,
> @@ -208,12 +209,91 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
>  
>  /// This struct knows that the superclasses of the object have already been
>  /// initialized.
> -pub struct ParentInit<'a, T>(&'a mut MaybeUninit<T>);
> +///
> +/// The declaration of `ParentInit` is.. *"a kind of magic"*.  It uses a
> +/// technique that is found in several crates, the main ones probably being
> +/// `ghost-cell` (in fact it was introduced by the [`GhostCell` paper](https://plv.mpi-sws.org/rustbelt/ghostcell/))
> +/// and `generativity`.

From the paper, I understand this technique should be "branded type". :-)

> +/// The `PhantomData` makes the `ParentInit` type *invariant* with respect to
> +/// the lifetime argument `'init`.  This, together with the `for<'...>` in
> +/// `[ParentInit::with]`, block any attempt of the compiler to be creative when
> +/// operating on types of type `ParentInit` and to extend their lifetimes.  In
> +/// particular, it ensures that the `ParentInit` cannot be made to outlive the
> +/// `rust_instance_init()` function that creates it, and therefore that the
> +/// `&'init T` reference is valid.
> +///
> +/// This implementation of the same concept, without the QOM baggage, can help
> +/// understanding the effect:
> +///
> +/// ```
> +/// use std::marker::PhantomData;
> +///
> +/// #[derive(PartialEq, Eq)]
> +/// pub struct Jail<'closure, T: Copy>(&'closure T, PhantomData<fn(&'closure ()) -> &'closure ()>);
> +///
> +/// impl<'closure, T: Copy> Jail<'closure, T> {
> +///     fn get(&self) -> T {
> +///         *self.0
> +///     }
> +///
> +///     #[inline]
> +///     fn with<U>(v: T, f: impl for<'id> FnOnce(Jail<'id, T>) -> U) -> U {
> +///         let parent_init = Jail(&v, PhantomData);
> +///         f(parent_init)
> +///     }
> +/// }
> +/// ```
> +///
> +/// It's impossible to escape the `Jail`; `token1` cannot be moved out of the
> +/// closure:
> +///
> +/// ```ignore
> +/// let x = 42;
> +/// let escape = Jail::with(&x, |token1| {
> +///     println!("{}", token1.get());
> +///     token1

This line will fail to compile (the below comment "// fails to compile" seems
to indicate that println! will fail):

error: lifetime may not live long enough
  --> src/main.rs:22:9
   |
20 |     let escape = Jail::with(x, |token1| {
   |                                 ------- return type of closure is Jail<'2, i32>
   |                                 |
   |                                 has type `Jail<'1, i32>`
21 |         println!("{}", token1.get());
22 |         token1
   |         ^^^^^^ returning this value requires that `'1` must outlive `'2`


Referring to GhostToken::new() [*], it said:

        // Return the result of running `f`.  Note that the `GhostToken` itself
        // cannot be returned, because `R` cannot mention the lifetime `'id`, so
        // the `GhostToken` only exists within its scope.

So this example is good, I think just need to optimize the location of the error hint.

[*]: https://gitlab.mpi-sws.org/FP/ghostcell/-/blob/master/ghostcell/src/lib.rs#L128

> +/// });
> +/// // fails to compile:
> +/// println!("{}", escape.get());
> +/// ```
> +///
> +/// Likewise, in the QOM case the `ParentInit` cannot be moved out of
> +/// `instance_init()`. Without this trick it would be possible to stash a
> +/// `ParentInit` and use it later to access uninitialized memory.
> +///
> +/// Here is another example, showing how separately-created "identities" stay
> +/// isolated:
> +///
> +/// ```ignore
> +/// impl<'closure, T: Copy> Clone for Jail<'closure, T> {
> +///     fn clone(&self) -> Jail<'closure, T> {
> +///         Jail(self.0, PhantomData)
> +///     }
> +/// }
> +///
> +/// fn main() {
> +///     Jail::with(42, |token1| {
> +///         // this works and returns true: the clone has the same "identity"
> +///         println!("{}", token1 == token1.clone());
> +///         Jail::with(42, |token2| {
> +///             // here the outer token remains accessible...
> +///             println!("{}", token1.get());
> +///             // ... but the two are separate: this fails to compile:
> +///             println!("{}", token1 == token2);
> +///         });
> +///     });
> +/// }
> +/// ```
> +pub struct ParentInit<'init, T>(
> +    &'init mut MaybeUninit<T>,
> +    PhantomData<fn(&'init ()) -> &'init ()>,
> +);
>  
> -impl<'a, T> ParentInit<'a, T> {
> +impl<'init, T> ParentInit<'init, T> {
>      #[inline]
> -    pub fn with(obj: &'a mut MaybeUninit<T>, f: impl FnOnce(ParentInit<'a, T>)) {
> -        let parent_init = ParentInit(obj);
> +    pub fn with(obj: &'init mut MaybeUninit<T>, f: impl for<'id> FnOnce(ParentInit<'id, T>)) {
> +        let parent_init = ParentInit(obj, PhantomData);

I think it's also valuable to add the similar comment as GhostToken did,
mentioning this `f` can't reture ParentInit itself.

>          f(parent_init)
>      }
>  }
> -- 
> 2.49.0
> 

Nice comment and nice reference (learned a lot).

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>