Bug 1500362 - Make GkAtoms opaque to avoid lld-link.exe errors r=emilio
authorCameron McCormack <cam@mcc.id.au>
Mon, 07 Jan 2019 09:50:25 +0000
changeset 509870 b7e4f2bbfe5705e81877fdbe2170445f5979a65c
parent 509869 f4151dfd56e567676cd32ea74a46626550a78763
child 509871 612d05e15b6f99c7f5efb688ea9513f21bfa5491
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1500362
milestone66.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 1500362 - Make GkAtoms opaque to avoid lld-link.exe errors r=emilio Depends on D15078 Differential Revision: https://phabricator.services.mozilla.com/D15800
layout/style/ServoBindings.toml
servo/components/style/gecko_string_cache/mod.rs
xpcom/ds/nsGkAtoms.h
--- a/layout/style/ServoBindings.toml
+++ b/layout/style/ServoBindings.toml
@@ -162,16 +162,17 @@ whitelist-vars = [
     "CSS_PSEUDO_ELEMENT_.*",
     "SERVO_CSS_PSEUDO_ELEMENT_FLAGS_.*",
     "kNameSpaceID_.*",
     "kGenericFont_.*",
     "kPresContext_.*",
     "nsContentUtils_.*",
     "GECKO_IS_NIGHTLY",
     "mozilla::detail::gGkAtoms",
+    "mozilla::detail::kGkAtomsArrayOffset",
 ]
 whitelist-types = [
     "RawGecko.*",
     "RawServoAnimationValueMap",  # TODO(heycam): Handle this elsewhere.
     "mozilla::DeclarationBlockMutationClosure",
     "mozilla::AnimationPropertySegment",
     "mozilla::AnonymousCounterStyle",
     "mozilla::AtomArray",
@@ -372,16 +373,17 @@ opaque-types = [
     "mozilla::Maybe",
     "gfxSize",  # <- union { struct { T width; T height; }; T components[2] };
     "gfxSize_Super",  # Ditto.
     "mozilla::StyleAnimationValue",
     "StyleAnimationValue", # pulls in a whole bunch of stuff we don't need in the bindings
     "mozilla::dom::.*Callback", # Pulls in ErrorResult and other things that
                                 # don't align properly on Linux 32-bit
     "mozilla::SchedulerGroup", # Non-standard-layout packing of field into superclass
+    "mozilla::detail::GkAtoms", # https://bugzilla.mozilla.org/show_bug.cgi?id=1517685
 ]
 
 # All cbindgen-types are in mod "structs::root::mozilla".
 cbindgen-types = [
     { gecko = "StyleAppearance", servo = "values::specified::Appearance" },
     { gecko = "StyleComputedFontStretchRange", servo = "font_face::ComputedFontStretchRange" },
     { gecko = "StyleComputedFontStyleDescriptor", servo = "font_face::ComputedFontStyleDescriptor" },
     { gecko = "StyleComputedFontWeightRange", servo = "font_face::ComputedFontWeightRange" },
--- a/servo/components/style/gecko_string_cache/mod.rs
+++ b/servo/components/style/gecko_string_cache/mod.rs
@@ -10,17 +10,19 @@
 
 //! A drop-in replacement for string_cache, but backed by Gecko `nsAtom`s.
 
 use crate::gecko_bindings::bindings::Gecko_AddRefAtom;
 use crate::gecko_bindings::bindings::Gecko_Atomize;
 use crate::gecko_bindings::bindings::Gecko_Atomize16;
 use crate::gecko_bindings::bindings::Gecko_ReleaseAtom;
 use crate::gecko_bindings::structs::{nsAtom, nsDynamicAtom, nsStaticAtom};
+use crate::gecko_bindings::structs::root::mozilla::detail::GkAtoms_Atoms_AtomsCount;
 use crate::gecko_bindings::structs::root::mozilla::detail::gGkAtoms;
+use crate::gecko_bindings::structs::root::mozilla::detail::kGkAtomsArrayOffset;
 use nsstring::{nsAString, nsStr};
 use precomputed_hash::PrecomputedHash;
 use std::borrow::{Borrow, Cow};
 use std::char::{self, DecodeUtf16};
 use std::fmt::{self, Write};
 use std::hash::{Hash, Hasher};
 use std::iter::Cloned;
 use std::ops::Deref;
@@ -52,21 +54,42 @@ macro_rules! local_name {
 pub struct Atom(usize);
 
 /// An atom *without* a strong reference.
 ///
 /// Only usable as `&'a WeakAtom`,
 /// where `'a` is the lifetime of something that holds a strong reference to that atom.
 pub struct WeakAtom(nsAtom);
 
+/// The number of static atoms we have.
+const STATIC_ATOM_COUNT: usize = GkAtoms_Atoms_AtomsCount as usize;
+
+/// Returns the Gecko static atom array.
+///
+/// We have this rather than use rust-bindgen to generate
+/// mozilla::detail::gGkAtoms and then just reference gGkAtoms.mAtoms, so we
+/// avoid a problem with lld-link.exe on Windows.
+///
+/// https://bugzilla.mozilla.org/show_bug.cgi?id=1517685
+#[inline]
+fn static_atoms() -> &'static [nsStaticAtom; STATIC_ATOM_COUNT] {
+    unsafe {
+        let addr = &gGkAtoms as *const _ as usize + kGkAtomsArrayOffset as usize;
+        &*(addr as *const _)
+    }
+}
+
+/// Returns whether the specified address points to one of the nsStaticAtom
+/// objects in the Gecko static atom array.
 #[inline]
 fn valid_static_atom_addr(addr: usize) -> bool {
     unsafe {
-        let start = gGkAtoms.mAtoms.get_unchecked(0) as *const _;
-        let end = gGkAtoms.mAtoms.get_unchecked(gGkAtoms.mAtoms.len()) as *const _;
+        let atoms = static_atoms();
+        let start = atoms.get_unchecked(0) as *const _;
+        let end = atoms.get_unchecked(STATIC_ATOM_COUNT) as *const _;
         let in_range = addr >= start as usize && addr < end as usize;
         let aligned = addr % mem::align_of::<nsStaticAtom>() == 0;
         in_range && aligned
     }
 }
 
 impl Deref for Atom {
     type Target = WeakAtom;
@@ -336,17 +359,17 @@ impl Atom {
         mem::forget(atom);
         ret
     }
 
     /// Creates a static atom from its index in the static atom table, without
     /// checking in release builds.
     #[inline]
     pub unsafe fn from_index(index: u16) -> Self {
-        let ptr = gGkAtoms.mAtoms.get_unchecked(index as usize) as *const _;
+        let ptr = static_atoms().get_unchecked(index as usize) as *const _;
         let handle = make_static_handle(ptr);
         let atom = Atom(handle);
         debug_assert!(valid_static_atom_addr(ptr as usize));
         debug_assert!(atom.is_static());
         debug_assert!((*atom).is_static());
         debug_assert!(handle == make_handle(atom.as_ptr()));
         atom
     }
--- a/xpcom/ds/nsGkAtoms.h
+++ b/xpcom/ds/nsGkAtoms.h
@@ -104,16 +104,24 @@ struct GkAtoms {
 #include "nsGkAtomList.h"
 #undef GK_ATOM
     AtomsCount
   };
 
   const nsStaticAtom mAtoms[static_cast<size_t>(Atoms::AtomsCount)];
 };
 
+// The offset from the start of the GkAtoms object to the start of the
+// nsStaticAtom array inside it.  This is used in Rust to avoid problems
+// with lld-link.exe on Windows when rust-bindgen generates a non-opaque
+// version of GkAtoms.
+//
+// https://bugzilla.mozilla.org/show_bug.cgi?id=1517685
+const ptrdiff_t kGkAtomsArrayOffset = offsetof(GkAtoms, mAtoms);
+
 // The GkAtoms instance is `extern const` so it can be defined in a .cpp file.
 //
 // XXX: The NS_EXTERNAL_VIS is necessary to work around an apparent GCC bug:
 // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87494
 #if defined(__GNUC__) && !defined(__clang__)
 extern NS_EXTERNAL_VIS const GkAtoms gGkAtoms;
 #else
 extern const GkAtoms gGkAtoms;