Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

Commit

Permalink
glib: Don't store subclass impl/private data in an Option<T>
Browse files Browse the repository at this point in the history
We don't know the exact memory layout of the Option<T> in relation to T
so can't easily go from a *const T to the surrounding *const Option<T>.

Since nightly from 2020-02-12 using both pointer types interchangeably
causes segmentation faults as their memory layouts are not actually
compatible and never were, but due to some compiler changes this
actually causes crashes at runtime now.

See rust-lang/rust#69102 for details.

As an Option<_> was only used for some paranoid runtime checks that are
not really needed, simply store T directly as impl/private data of
subclasses.

This has the side-effect of having zero-sized impl/private data types to
actually occupy zero bytes of memory, and in some other cases to save a
few bytes of the allocation.
  • Loading branch information
sdroege committed Feb 12, 2020
1 parent 4947f85 commit f9f17d6
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
11 changes: 11 additions & 0 deletions src/subclass/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,17 @@ mod test {
assert!(weak.upgrade().is_none());
}

#[test]
fn test_create_child_object() {
let type_ = ChildObject::get_type();
let obj = Object::new(type_, &[]).unwrap();

// ChildObject is a zero-sized type and we map that to the same pointer as the object
// itself. No private/impl data is allocated for zero-sized types.
let imp = ChildObject::from_instance(&obj);
assert_eq!(imp as *const _ as *const (), obj.as_ptr() as *const _);
}

#[test]
fn test_set_properties() {
let obj = Object::new(SimpleObject::get_type(), &[]).unwrap();
Expand Down
22 changes: 13 additions & 9 deletions src/subclass/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ pub unsafe trait InstanceStruct: Sized + 'static {
let private_offset = data.as_ref().private_offset;
let ptr: *const u8 = self as *const _ as *const u8;
let priv_ptr = ptr.offset(private_offset);
let imp = priv_ptr as *const Option<Self::Type>;
let imp = priv_ptr as *const Self::Type;

(*imp).as_ref().expect("No private struct")
&*imp
}
}

Expand Down Expand Up @@ -377,7 +377,7 @@ unsafe extern "C" fn class_init<T: ObjectSubclass>(

// We have to update the private struct offset once the class is actually
// being initialized.
{
if mem::size_of::<T>() != 0 {
let mut private_offset = data.as_ref().private_offset as i32;
gobject_sys::g_type_class_adjust_private_offset(klass, &mut private_offset);
(*data.as_mut()).private_offset = private_offset as isize;
Expand Down Expand Up @@ -417,13 +417,13 @@ unsafe extern "C" fn instance_init<T: ObjectSubclass>(
let private_offset = (*data.as_mut()).private_offset;
let ptr: *mut u8 = obj as *mut _ as *mut u8;
let priv_ptr = ptr.offset(private_offset);
let imp_storage = priv_ptr as *mut Option<T>;
let imp_storage = priv_ptr as *mut T;

let klass = &*(klass as *const T::Class);

let imp = T::new_with_class(klass);

ptr::write(imp_storage, Some(imp));
ptr::write(imp_storage, imp);
}

unsafe extern "C" fn finalize<T: ObjectSubclass>(obj: *mut gobject_sys::GObject) {
Expand All @@ -433,9 +433,9 @@ unsafe extern "C" fn finalize<T: ObjectSubclass>(obj: *mut gobject_sys::GObject)
let private_offset = (*data.as_mut()).private_offset;
let ptr: *mut u8 = obj as *mut _ as *mut u8;
let priv_ptr = ptr.offset(private_offset);
let imp_storage = priv_ptr as *mut Option<T>;
let imp_storage = priv_ptr as *mut T;

let imp = (*imp_storage).take().expect("No private struct");
let imp = ptr::read(imp_storage);
drop(imp);

// Chain up to the parent class' finalize implementation, if any.
Expand Down Expand Up @@ -494,8 +494,12 @@ where

let mut data = T::type_data();
(*data.as_mut()).type_ = type_;
let private_offset =
gobject_sys::g_type_add_instance_private(type_.to_glib(), mem::size_of::<Option<T>>());

let private_offset = if mem::size_of::<T>() == 0 {
0
} else {
gobject_sys::g_type_add_instance_private(type_.to_glib(), mem::size_of::<T>())
};
(*data.as_mut()).private_offset = private_offset as isize;

T::type_init(&mut InitializingType::<T>(type_, marker::PhantomData));
Expand Down

0 comments on commit f9f17d6

Please sign in to comment.