Bug 1276140 - Use memcpy rather than union to reinterpret in frame properties table. r?froydnj draft
authorXidorn Quan <me@upsuper.org>
Fri, 27 May 2016 14:32:18 +1000
changeset 371947 40399960c39c5e718ed9bf92c3322d18b6ec3452
parent 371874 a008708a1379e3e6b57982457fa281a486da5a67
child 522060 d4f672a6c6e5f173337448e2773f31d7140feb9e
push id19398
push userxquan@mozilla.com
push dateFri, 27 May 2016 04:32:39 +0000
reviewersfroydnj
bugs1276140
milestone49.0a1
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 reinterpret_cast<void*>(aValue);
+    }
+
+    static T* FromPointer(void* aPtr)
+    {
+      return reinterpret_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) {}