From d69a6789687c9df6b4a71f9a2cdb5e7db00e2c6d Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 13 Apr 2016 12:11:42 -0700 Subject: [PATCH] Avoid moves of Gecko style structs. --- ports/geckolib/lib.rs | 1 + ports/geckolib/properties.mako.rs | 20 ++++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/ports/geckolib/lib.rs b/ports/geckolib/lib.rs index d901e2d7aee7..5a2a468d3192 100644 --- a/ports/geckolib/lib.rs +++ b/ports/geckolib/lib.rs @@ -5,6 +5,7 @@ #![feature(as_unsafe_cell)] #![feature(box_syntax)] #![feature(ptr_as_ref)] +#![feature(custom_attribute)] #![feature(custom_derive)] #![feature(plugin)] diff --git a/ports/geckolib/properties.mako.rs b/ports/geckolib/properties.mako.rs index 4f74e767e7a3..ba797f5888c5 100644 --- a/ports/geckolib/properties.mako.rs +++ b/ports/geckolib/properties.mako.rs @@ -96,6 +96,7 @@ impl ComputedValues for GeckoComputedValues { <%def name="declare_style_struct(style_struct)"> #[derive(Clone, HeapSizeOf, Debug)] +#[no_move] % if style_struct.gecko_ffi_name: pub struct ${style_struct.gecko_struct_name} { gecko: ${style_struct.gecko_ffi_name}, @@ -108,11 +109,22 @@ pub struct ${style_struct.gecko_struct_name}; <%def name="impl_style_struct(style_struct)"> impl ${style_struct.gecko_struct_name} { #[allow(dead_code, unused_variables)] - fn initial() -> Self { + fn initial() -> Arc { % if style_struct.gecko_ffi_name: - let mut result = ${style_struct.gecko_struct_name} { gecko: unsafe { zeroed() } }; + // Some Gecko style structs have AutoTArray members, which have internal pointers and are + // thus MOZ_NON_MEMMOVABLE. Since Rust is generally a very move-happy language, we need to + // be very careful that nsStyle* structs are never moved after they are constructed. + // + // By annotating the structs [no_move], we can get the |rust-tenacious| linter to trigger + // an error on any semantic moves. But we don't have a great way of telling LLVM to + // allocate our new object directly on the heap without using a temporary. So to do that + // (and also please tenacious), we pass zeroed memory into the Arc constructor, and _then_ + // use make_mut to get a reference to pass to the Gecko constructor. Since the refcount is + // guaranteed to be 1, make_mut will always pass us a direct reference instead of taking + // the copy-on-write path. + let mut result = Arc::new(${style_struct.gecko_struct_name} { gecko: unsafe { zeroed() } }); unsafe { - Gecko_Construct_${style_struct.gecko_ffi_name}(&mut result.gecko); + Gecko_Construct_${style_struct.gecko_ffi_name}(&mut Arc::make_mut(&mut result).gecko); } result % else: @@ -224,7 +236,7 @@ ${impl_style_struct(style_struct)} lazy_static! { pub static ref INITIAL_GECKO_VALUES: GeckoComputedValues = GeckoComputedValues { % for style_struct in STYLE_STRUCTS: - ${style_struct.ident}: Arc::new(${style_struct.gecko_struct_name}::initial()), + ${style_struct.ident}: ${style_struct.gecko_struct_name}::initial(), % endfor custom_properties: None, shareable: true,