From bbe787c1aaf6f450b33bd4102acbfd7cbd525bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 13 Feb 2020 00:24:13 +0200 Subject: [PATCH 1/3] Don't store subclass impl/private data in an Option We don't know the exact memory layout of the Option in relation to T so can't easily go from a *const T to the surrounding *const Option. 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 https://github.com/rust-lang/rust/issues/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. --- src/subclass/object.rs | 11 +++++++++++ src/subclass/types.rs | 22 +++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/subclass/object.rs b/src/subclass/object.rs index 909dc30e..e30f6eaa 100644 --- a/src/subclass/object.rs +++ b/src/subclass/object.rs @@ -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(); diff --git a/src/subclass/types.rs b/src/subclass/types.rs index adfb096b..482c9add 100644 --- a/src/subclass/types.rs +++ b/src/subclass/types.rs @@ -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; + let imp = priv_ptr as *const Self::Type; - (*imp).as_ref().expect("No private struct") + &*imp } } @@ -377,7 +377,7 @@ unsafe extern "C" fn class_init( // We have to update the private struct offset once the class is actually // being initialized. - { + if mem::size_of::() != 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; @@ -417,13 +417,13 @@ unsafe extern "C" fn instance_init( 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; + 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(obj: *mut gobject_sys::GObject) { @@ -433,9 +433,9 @@ unsafe extern "C" fn finalize(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; + 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. @@ -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::>()); + + let private_offset = if mem::size_of::() == 0 { + 0 + } else { + gobject_sys::g_type_add_instance_private(type_.to_glib(), mem::size_of::()) + }; (*data.as_mut()).private_offset = private_offset as isize; T::type_init(&mut InitializingType::(type_, marker::PhantomData)); From 85daeabc4a3cebf28be5918436f3785a81cdd9c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 13 Feb 2020 00:38:47 +0200 Subject: [PATCH 2/3] Use ptr::drop_in_place() for dropping impl/private data of subclasses Instead of first reading it on the stack and the dropping it. --- src/subclass/types.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/subclass/types.rs b/src/subclass/types.rs index 482c9add..112f857c 100644 --- a/src/subclass/types.rs +++ b/src/subclass/types.rs @@ -427,16 +427,13 @@ unsafe extern "C" fn instance_init( } unsafe extern "C" fn finalize(obj: *mut gobject_sys::GObject) { - // Retrieve the private struct, take it out of its storage and - // drop it for freeing all associated memory. + // Retrieve the private struct and drop it for freeing all associated memory. let mut data = T::type_data(); 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 T; - - let imp = ptr::read(imp_storage); - drop(imp); + ptr::drop_in_place(imp_storage); // Chain up to the parent class' finalize implementation, if any. let parent_class = &*(data.as_ref().get_parent_class() as *const gobject_sys::GObjectClass); From 155ea119a54ee6cc54db2b763734636557cda757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 13 Feb 2020 06:57:44 +0200 Subject: [PATCH 3/3] Don't allow using types with bigger alignment than two usize as private/impl data for subclasses GLib aligns the type private data to two gsizes so we can't safely store any type there that requires a bigger alignment. --- src/subclass/types.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/subclass/types.rs b/src/subclass/types.rs index 112f857c..2f239d64 100644 --- a/src/subclass/types.rs +++ b/src/subclass/types.rs @@ -454,6 +454,16 @@ pub fn register_type() -> Type where <::ParentType as ObjectType>::RustClassType: IsSubclassable, { + // GLib aligns the type private data to two gsizes so we can't safely store any type there that + // requires a bigger alignment. + if mem::align_of::() > 2 * mem::size_of::() { + panic!( + "Alignment {} of type not supported, bigger than {}", + mem::align_of::(), + 2 * mem::size_of::(), + ); + } + unsafe { use std::ffi::CString;