Add a type that, together with the C function object_deinit, ensures the
correct drop order for QOM objects relative to their superclasses.
Right now it is not possible to implement the Drop trait for QOM classes
that are defined in Rust, as the drop() function would not be called when
the object goes away; instead what is called is ObjectImpl::INSTANCE_FINALIZE.
It would be nice for INSTANCE_FINALIZE to just drop the object, but this has
a problem: suppose you have
pub struct MySuperclass {
parent: DeviceState,
field: Box<MyData>,
...
}
impl Drop for MySuperclass {
...
}
pub struct MySubclass {
parent: MySuperclass,
...
}
and an instance_finalize implementation that is like
unsafe extern "C" fn drop_object<T: ObjectImpl>(obj: *mut Object) {
unsafe { std::ptr::drop_in_place(obj.cast::<T>()) }
}
When instance_finalize is called for MySubclass, it will walk the struct's
list of fields and call the drop method for MySuperclass. Then, object_deinit
recurses to the superclass and calls the same drop method again. This
will cause double-freeing of the Box<Data>.
What's happening here is that QOM wants to control the drop order of
MySuperclass and MySubclass's fields. To do so, the parent field must
be marked ManuallyDrop<>, which is quite ugly. Instead, add a wrapper
type ParentField<> that is specific to QOM. This hides the implementation
detail of *what* is special about the ParentField, and will also be easy
to check in the #[derive(Object)] macro.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 6 ++--
rust/qemu-api/src/qom.rs | 56 +++++++++++++++++++++++++++++---
rust/qemu-api/tests/tests.rs | 4 +--
3 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 18cc122951d..689202f4550 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -14,7 +14,7 @@
irq::InterruptSource,
prelude::*,
qdev::DeviceImpl,
- qom::ObjectImpl,
+ qom::{ObjectImpl, ParentField},
};
use crate::{
@@ -86,7 +86,7 @@ fn index(&self, idx: u32) -> &Self::Output {
#[derive(Debug, qemu_api_macros::Object, qemu_api_macros::offsets)]
/// PL011 Device Model in QEMU
pub struct PL011State {
- pub parent_obj: SysBusDevice,
+ pub parent_obj: ParentField<SysBusDevice>,
pub iomem: MemoryRegion,
#[doc(alias = "fr")]
pub flags: registers::Flags,
@@ -645,7 +645,7 @@ pub fn post_load(&mut self, _version_id: u32) -> Result<(), ()> {
#[derive(Debug, qemu_api_macros::Object)]
/// PL011 Luminary device model.
pub struct PL011Luminary {
- parent_obj: PL011State,
+ parent_obj: ParentField<PL011State>,
}
impl PL011Luminary {
diff --git a/rust/qemu-api/src/qom.rs b/rust/qemu-api/src/qom.rs
index 7d5fbef1e17..1341a173893 100644
--- a/rust/qemu-api/src/qom.rs
+++ b/rust/qemu-api/src/qom.rs
@@ -55,6 +55,7 @@
use std::{
ffi::CStr,
+ fmt,
ops::{Deref, DerefMut},
os::raw::c_void,
};
@@ -105,6 +106,52 @@ fn as_ref(&self) -> &$parent {
};
}
+/// This is the same as [`ManuallyDrop<T>`](std::mem::ManuallyDrop), though
+/// it hides the standard methods of `ManuallyDrop`.
+///
+/// The first field of an `ObjectType` must be of type `ParentField<T>`.
+/// (Technically, this is only necessary if there is at least one Rust
+/// superclass in the hierarchy). This is to ensure that the parent field is
+/// dropped after the subclass; this drop order is enforced by the C
+/// `object_deinit` function.
+///
+/// # Examples
+///
+/// ```ignore
+/// #[repr(C)]
+/// #[derive(qemu_api_macros::Object)]
+/// pub struct MyDevice {
+/// parent: ParentField<DeviceState>,
+/// ...
+/// }
+/// ```
+#[derive(Debug)]
+#[repr(transparent)]
+pub struct ParentField<T: ObjectType>(std::mem::ManuallyDrop<T>);
+
+impl<T: ObjectType> Deref for ParentField<T> {
+ type Target = T;
+
+ #[inline(always)]
+ fn deref(&self) -> &Self::Target {
+ &self.0
+ }
+}
+
+impl<T: ObjectType> DerefMut for ParentField<T> {
+ #[inline(always)]
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ &mut self.0
+ }
+}
+
+impl<T: fmt::Display + ObjectType> fmt::Display for ParentField<T> {
+ #[inline(always)]
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+ self.0.fmt(f)
+ }
+}
+
unsafe extern "C" fn rust_instance_init<T: ObjectImpl>(obj: *mut Object) {
// SAFETY: obj is an instance of T, since rust_instance_init<T>
// is called from QOM core as the instance_init function
@@ -151,8 +198,9 @@ fn as_ref(&self) -> &$parent {
///
/// - the struct must be `#[repr(C)]`;
///
-/// - the first field of the struct must be of the instance struct corresponding
-/// to the superclass, which is `ObjectImpl::ParentType`
+/// - the first field of the struct must be of type
+/// [`ParentField<T>`](ParentField), where `T` is the parent type
+/// [`ObjectImpl::ParentType`]
///
/// - likewise, the first field of the `Class` must be of the class struct
/// corresponding to the superclass, which is `ObjectImpl::ParentType::Class`.
@@ -384,8 +432,8 @@ impl<T: ObjectType> ObjectCastMut for &mut T {}
/// Trait a type must implement to be registered with QEMU.
pub trait ObjectImpl: ObjectType + ClassInitImpl<Self::Class> {
- /// The parent of the type. This should match the first field of
- /// the struct that implements `ObjectImpl`:
+ /// The parent of the type. This should match the first field of the
+ /// struct that implements `ObjectImpl`, minus the `ParentField<_>` wrapper.
type ParentType: ObjectType;
/// Whether the object can be instantiated
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 41ea4026b83..3b3cf793ae3 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -15,7 +15,7 @@
c_str, declare_properties, define_property,
prelude::*,
qdev::{DeviceImpl, DeviceState, Property},
- qom::ObjectImpl,
+ qom::{ObjectImpl, ParentField},
vmstate::VMStateDescription,
zeroable::Zeroable,
};
@@ -31,7 +31,7 @@
#[repr(C)]
#[derive(qemu_api_macros::Object)]
pub struct DummyState {
- parent: DeviceState,
+ parent: ParentField<DeviceState>,
migrate_clock: bool,
}
--
2.47.1