Bug 1276140 - Use memcpy rather than union to reinterpret in frame properties table. r=froydnj
authorXidorn Quan <me@upsuper.org>
Mon, 30 May 2016 09:19:25 +1000
changeset 299472 bc8ea177c0d8303861f04c6009582acf40c692bb
parent 299471 a797b1192ac89f6f11049e2ea6d87dd66fb1cedc
child 299473 d36ad40cf9910c695a20eff674aa18dc89d3b269
push id30297
push usercbook@mozilla.com
push dateMon, 30 May 2016 13:29:51 +0000
treeherdermozilla-central@3435dd7ad71f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1276140
milestone49.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 1276140 - Use memcpy rather than union to reinterpret in frame properties table. r=froydnj MozReview-Commit-ID: Inrf22Ve9FQ
layout/base/FramePropertyTable.h
--- a/layout/base/FramePropertyTable.h
+++ b/layout/base/FramePropertyTable.h
@@ -163,19 +163,18 @@ public:
    * lookup (using the frame as the key) and a linear search through
    * the properties of that frame. Any existing value for the property
    * is destroyed.
    */
   template<typename T>
   void Set(const nsIFrame* aFrame, Descriptor<T> aProperty,
            PropertyType<T> aValue)
   {
-    ReinterpretHelper<T> helper{};
-    helper.value = aValue;
-    SetInternal(aFrame, aProperty, helper.ptr);
+    void* ptr = ReinterpretHelper<T>::ToPointer(aValue);
+    SetInternal(aFrame, aProperty, ptr);
   }
 
   /**
    * @return true if @aProperty is set for @aFrame. This requires one hashtable
    * lookup (using the frame as the key) and a linear search through the
    * properties of that frame.
    *
    * In most cases, this shouldn't be used outside of assertions, because if
@@ -208,19 +207,18 @@ public:
    * the frame has a value for the property. This lets callers
    * disambiguate a null result, which can mean 'no such property' or
    * 'property value is null'.
    */
   template<typename T>
   PropertyType<T> Get(const nsIFrame* aFrame, Descriptor<T> aProperty,
                       bool* aFoundResult = nullptr)
   {
-    ReinterpretHelper<T> helper;
-    helper.ptr = GetInternal(aFrame, aProperty, aFoundResult);
-    return helper.value;
+    void* ptr = GetInternal(aFrame, aProperty, aFoundResult);
+    return ReinterpretHelper<T>::FromPointer(ptr);
   }
   /**
    * Remove a property value for a frame. This requires one hashtable
    * lookup (using the frame as the key) and a linear search through
    * the properties of that frame. The old property value is returned
    * (and not destroyed). If the frame has no such property,
    * returns zero-filled result, which means null for pointers and
    * zero for integers and floating point types.
@@ -228,19 +226,18 @@ public:
    * the frame had a value for the property. This lets callers
    * disambiguate a null result, which can mean 'no such property' or
    * 'property value is null'.
    */
   template<typename T>
   PropertyType<T> Remove(const nsIFrame* aFrame, Descriptor<T> aProperty,
                          bool* aFoundResult = nullptr)
   {
-    ReinterpretHelper<T> helper;
-    helper.ptr = RemoveInternal(aFrame, aProperty, aFoundResult);
-    return helper.value;
+    void* ptr = RemoveInternal(aFrame, aProperty, aFoundResult);
+    return ReinterpretHelper<T>::FromPointer(ptr);
   }
   /**
    * Remove and destroy a property value for a frame. This requires one
    * hashtable lookup (using the frame as the key) and a linear search
    * through the properties of that frame. If the frame has no such
    * property, nothing happens.
    */
   template<typename T>
@@ -267,26 +264,49 @@ protected:
   void* GetInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty,
                     bool* aFoundResult);
 
   void* RemoveInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty,
                        bool* aFoundResult);
 
   void DeleteInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty);
 
-  // A helper union being used here rather than simple reinterpret_cast
-  // is because PropertyType<T> could be float, bool, uint8_t, etc.
-  // which do not have defined behavior for a direct cast.
   template<typename T>
-  union ReinterpretHelper
+  struct ReinterpretHelper
   {
-    void* ptr;
-    PropertyType<T> value;
     static_assert(sizeof(PropertyType<T>) <= sizeof(void*),
                   "size of the value must never be larger than a pointer");
+
+    static void* ToPointer(PropertyType<T> aValue)
+    {
+      void* ptr = nullptr;
+      memcpy(&ptr, &aValue, sizeof(aValue));
+      return ptr;
+    }
+
+    static PropertyType<T> FromPointer(void* aPtr)
+    {
+      PropertyType<T> value;
+      memcpy(&value, &aPtr, sizeof(value));
+      return value;
+    }
+  };
+
+  template<typename T>
+  struct ReinterpretHelper<T*>
+  {
+    static void* ToPointer(T* aValue)
+    {
+      return static_cast<void*>(aValue);
+    }
+
+    static T* FromPointer(void* aPtr)
+    {
+      return static_cast<T*>(aPtr);
+    }
   };
 
   /**
    * Stores a property descriptor/value pair. It can also be used to
    * store an nsTArray of PropertyValues.
    */
   struct PropertyValue {
     PropertyValue() : mProperty(nullptr), mValue(nullptr) {}