Bug 1593951 - Use MaybeUninit in style struct clone impls / constructors. r=xidorn
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 12 Nov 2019 20:30:42 +0000
changeset 501643 e2df01b709f9ceed696ba3c89667588ae81ae81f
parent 501642 38c5042a680fa46d1a76d6d3f22fc1ad5335bd40
child 501644 56f779b8961dd87cac2e20b4c91c1d6b2a3b0f7b
push id36797
push useropoprus@mozilla.com
push dateWed, 13 Nov 2019 09:55:25 +0000
treeherdermozilla-central@2f19e7b646e0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn
bugs1593951
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1593951 - Use MaybeUninit in style struct clone impls / constructors. r=xidorn Differential Revision: https://phabricator.services.mozilla.com/D51788
layout/style/nsStyleStruct.cpp
servo/components/style/properties/gecko.mako.rs
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -2680,18 +2680,20 @@ nsStyleDisplay::nsStyleDisplay(const Doc
       mOrient(StyleOrient::Inline),
       mIsolation(NS_STYLE_ISOLATION_AUTO),
       mTopLayer(NS_STYLE_TOP_LAYER_NONE),
       mTouchAction(StyleTouchAction_AUTO),
       mScrollBehavior(NS_STYLE_SCROLL_BEHAVIOR_AUTO),
       mOverscrollBehaviorX(StyleOverscrollBehavior::Auto),
       mOverscrollBehaviorY(StyleOverscrollBehavior::Auto),
       mOverflowAnchor(StyleOverflowAnchor::Auto),
-      mScrollSnapType(
-          {StyleScrollSnapAxis::Both, StyleScrollSnapStrictness::None}),
+      mScrollSnapAlign{StyleScrollSnapAlignKeyword::None,
+                       StyleScrollSnapAlignKeyword::None},
+      mScrollSnapType{StyleScrollSnapAxis::Both,
+                      StyleScrollSnapStrictness::None},
       mLineClamp(0),
       mRotate(StyleRotate::None()),
       mTranslate(StyleTranslate::None()),
       mScale(StyleScale::None()),
       mBackfaceVisibility(NS_STYLE_BACKFACE_VISIBILITY_VISIBLE),
       mTransformStyle(NS_STYLE_TRANSFORM_STYLE_FLAT),
       mTransformBox(StyleGeometryBox::BorderBox),
       mOffsetPath(StyleOffsetPath::None()),
@@ -2745,16 +2747,18 @@ nsStyleDisplay::nsStyleDisplay(const nsS
       mResize(aSource.mResize),
       mOrient(aSource.mOrient),
       mIsolation(aSource.mIsolation),
       mTopLayer(aSource.mTopLayer),
       mTouchAction(aSource.mTouchAction),
       mScrollBehavior(aSource.mScrollBehavior),
       mOverscrollBehaviorX(aSource.mOverscrollBehaviorX),
       mOverscrollBehaviorY(aSource.mOverscrollBehaviorY),
+      mOverflowAnchor(aSource.mOverflowAnchor),
+      mScrollSnapAlign(aSource.mScrollSnapAlign),
       mScrollSnapType(aSource.mScrollSnapType),
       mLineClamp(aSource.mLineClamp),
       mTransform(aSource.mTransform),
       mRotate(aSource.mRotate),
       mTranslate(aSource.mTranslate),
       mScale(aSource.mScale),
       mBackfaceVisibility(aSource.mBackfaceVisibility),
       mTransformStyle(aSource.mTransformStyle),
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -41,17 +41,17 @@ use crate::gecko::values::round_border_t
 use crate::logical_geometry::WritingMode;
 use crate::media_queries::Device;
 use crate::properties::computed_value_flags::*;
 use crate::properties::longhands;
 use crate::rule_tree::StrongRuleNode;
 use crate::selector_parser::PseudoElement;
 use servo_arc::{Arc, RawOffsetArc, UniqueArc};
 use std::marker::PhantomData;
-use std::mem::{forget, zeroed, ManuallyDrop};
+use std::mem::{forget, MaybeUninit};
 use std::{cmp, ops, ptr};
 use crate::values::{self, CustomIdent, Either, KeyframesName, None_};
 use crate::values::computed::{Percentage, TransitionProperty};
 use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::BorderStyle;
 use crate::values::computed::font::FontSize;
 use crate::values::generics::column::ColumnCount;
 use crate::values::generics::image::ImageLayer;
@@ -213,17 +213,17 @@ impl ComputedValuesInner {
     ) -> Arc<ComputedValues> {
         let pseudo_ty = match pseudo {
             Some(p) => p.pseudo_type(),
             None => structs::PseudoStyleType::NotPseudo,
         };
         unsafe {
             let mut arc = UniqueArc::<ComputedValues>::new_uninit();
             bindings::Gecko_ComputedStyle_Init(
-                &mut (*arc.as_mut_ptr()).0,
+                arc.as_mut_ptr() as *mut _,
                 &self,
                 pseudo_ty,
             );
             // We're simulating move semantics by having C++ do a memcpy and then forgetting
             // it on this end.
             forget(self);
             UniqueArc::assume_init(arc).shareable()
         }
@@ -616,39 +616,45 @@ def set_gecko_property(ffi_name, expr):
 <%def name="impl_logical(name, **kwargs)">
     ${helpers.logical_setter(name)}
 </%def>
 
 <%def name="impl_style_struct(style_struct)">
 impl ${style_struct.gecko_struct_name} {
     #[allow(dead_code, unused_variables)]
     pub fn default(document: &structs::Document) -> Arc<Self> {
-        let mut result = Arc::new(${style_struct.gecko_struct_name} { gecko: ManuallyDrop::new(unsafe { zeroed() }) });
         unsafe {
+            let mut result = UniqueArc::<Self>::new_uninit();
+            // FIXME(bug 1595895): Zero the memory to keep valgrind happy, but
+            // these looks like Valgrind false-positives at a quick glance.
+            ptr::write_bytes::<Self>(result.as_mut_ptr(), 0, 1);
             Gecko_Construct_Default_${style_struct.gecko_ffi_name}(
-                &mut *Arc::get_mut(&mut result).unwrap().gecko,
+                result.as_mut_ptr() as *mut _,
                 document,
             );
+            UniqueArc::assume_init(result).shareable()
         }
-        result
     }
 }
 impl Drop for ${style_struct.gecko_struct_name} {
     fn drop(&mut self) {
         unsafe {
             Gecko_Destroy_${style_struct.gecko_ffi_name}(&mut *self.gecko);
         }
     }
 }
 impl Clone for ${style_struct.gecko_struct_name} {
     fn clone(&self) -> Self {
         unsafe {
-            let mut result = ${style_struct.gecko_struct_name} { gecko: ManuallyDrop::new(zeroed()) };
-            Gecko_CopyConstruct_${style_struct.gecko_ffi_name}(&mut *result.gecko, &*self.gecko);
-            result
+            let mut result = MaybeUninit::<Self>::uninit();
+            // FIXME(bug 1595895): Zero the memory to keep valgrind happy, but
+            // these looks like Valgrind false-positives at a quick glance.
+            ptr::write_bytes::<Self>(result.as_mut_ptr(), 0, 1);
+            Gecko_CopyConstruct_${style_struct.gecko_ffi_name}(result.as_mut_ptr() as *mut _, &*self.gecko);
+            result.assume_init()
         }
     }
 }
 
 </%def>
 
 <%def name="impl_simple_type_with_conversion(ident, gecko_ffi_name=None)">
     <%