Bug 1269319 - Make AlignedStorage/AlignedStorage2 non-copyable to fix strict aliasing issues. r=Waldo a=sylvestre
☠☠ backed out by 8b7e3ca6ddb7 ☠ ☠
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 19 May 2016 20:57:36 +0200
changeset 335184 b73cfa00dab309c8b999550363df0c0456128b90
parent 335183 e4d613a6d1caf92ec1b5804cf48fabe0f30b09de
child 335185 8b7e3ca6ddb7c3d7ad1dfd8b61ab18189e6ca1f3
push id1146
push userCallek@gmail.com
push dateMon, 25 Jul 2016 16:35:44 +0000
treeherdermozilla-release@a55778f9cd5a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo, sylvestre
bugs1269319
milestone48.0a2
Bug 1269319 - Make AlignedStorage/AlignedStorage2 non-copyable to fix strict aliasing issues. r=Waldo a=sylvestre
dom/plugins/base/nsJSNPRuntime.h
js/public/HashTable.h
js/src/jit/JitCompartment.h
js/src/jit/Snapshots.h
js/src/vm/ObjectGroup.cpp
mfbt/Alignment.h
--- a/dom/plugins/base/nsJSNPRuntime.h
+++ b/dom/plugins/base/nsJSNPRuntime.h
@@ -33,18 +33,27 @@ public:
   bool operator!=(const nsJSObjWrapperKey& other) const {
     return !(*this == other);
   }
 
   void trace(JSTracer* trc) {
       JS::TraceEdge(trc, &mJSObj, "nsJSObjWrapperKey");
   }
 
+  nsJSObjWrapperKey(const nsJSObjWrapperKey& other)
+    : mJSObj(other.mJSObj),
+      mNpp(other.mNpp)
+  {}
+  void operator=(const nsJSObjWrapperKey& other) {
+    mJSObj = other.mJSObj;
+    mNpp = other.mNpp;
+  }
+
   JS::Heap<JSObject*> mJSObj;
-  const NPP mNpp;
+  NPP mNpp;
 };
 
 class nsJSObjWrapper : public NPObject
 {
 public:
   JS::Heap<JSObject *> mJSObj;
   const NPP mNpp;
   bool mDestroyPending;
--- a/js/public/HashTable.h
+++ b/js/public/HashTable.h
@@ -699,16 +699,21 @@ class HashMapEntry
         value_(mozilla::Forward<ValueInput>(v))
     {}
 
     HashMapEntry(HashMapEntry&& rhs)
       : key_(mozilla::Move(rhs.key_)),
         value_(mozilla::Move(rhs.value_))
     {}
 
+    void operator=(HashMapEntry&& rhs) {
+        key_ = mozilla::Move(rhs.key_);
+        value_ = mozilla::Move(rhs.value_);
+    }
+
     typedef Key KeyType;
     typedef Value ValueType;
 
     const Key& key() const { return key_; }
     Key& mutableKey() { return key_; }
     const Value& value() const { return value_; }
     Value& value() { return value_; }
 
@@ -769,18 +774,26 @@ class HashTableEntry
     }
 
     void destroy() {
         MOZ_ASSERT(isLive());
         mem.addr()->~T();
     }
 
     void swap(HashTableEntry* other) {
+        if (this == other)
+            return;
+        MOZ_ASSERT(isLive());
+        if (other->isLive()) {
+            mozilla::Swap(*mem.addr(), *other->mem.addr());
+        } else {
+            *other->mem.addr() = mozilla::Move(*mem.addr());
+            destroy();
+        }
         mozilla::Swap(keyHash, other->keyHash);
-        mozilla::Swap(mem, other->mem);
     }
 
     T& get() { MOZ_ASSERT(isLive()); return *mem.addr(); }
     NonConstT& getMutable() { MOZ_ASSERT(isLive()); return *mem.addr(); }
 
     bool isFree() const    { return keyHash == sFreeKey; }
     void clearLive()       { MOZ_ASSERT(isLive()); keyHash = sFreeKey; mem.addr()->~T(); }
     void clear()           { if (isLive()) mem.addr()->~T(); keyHash = sFreeKey; }
--- a/js/src/jit/JitCompartment.h
+++ b/js/src/jit/JitCompartment.h
@@ -398,16 +398,20 @@ struct CacheIRStubKey : public DefaultHa
 
     static HashNumber hash(const Lookup& l);
     static bool match(const CacheIRStubKey& entry, const Lookup& l);
 
     UniquePtr<CacheIRStubInfo, JS::FreePolicy> stubInfo;
 
     explicit CacheIRStubKey(CacheIRStubInfo* info) : stubInfo(info) {}
     CacheIRStubKey(CacheIRStubKey&& other) : stubInfo(Move(other.stubInfo)) { }
+
+    void operator=(CacheIRStubKey&& other) {
+        stubInfo = Move(other.stubInfo);
+    }
 };
 
 class JitCompartment
 {
     friend class JitActivation;
 
     template<typename Key>
     struct IcStubCodeMapGCPolicy {
--- a/js/src/jit/Snapshots.h
+++ b/js/src/jit/Snapshots.h
@@ -499,17 +499,35 @@ class SnapshotReader
     uint32_t numAllocationsRead() const {
         return allocRead_;
     }
     void resetNumAllocationsRead() {
         allocRead_ = 0;
     }
 };
 
-typedef mozilla::AlignedStorage<4 * sizeof(uint32_t)> RInstructionStorage;
+class RInstructionStorage
+{
+    static const size_t Size = 4 * sizeof(uint32_t);
+    mozilla::AlignedStorage<Size> mem;
+
+  public:
+    const void* addr() const { return mem.addr(); }
+    void* addr() { return mem.addr(); }
+
+    RInstructionStorage() = default;
+
+    RInstructionStorage(const RInstructionStorage& other) {
+        memcpy(addr(), other.addr(), Size);
+    }
+    void operator=(const RInstructionStorage& other) {
+        memcpy(addr(), other.addr(), Size);
+    }
+};
+
 class RInstruction;
 
 class RecoverReader
 {
     CompactBufferReader reader_;
 
     // Number of encoded instructions.
     uint32_t numInstructions_;
--- a/js/src/vm/ObjectGroup.cpp
+++ b/js/src/vm/ObjectGroup.cpp
@@ -1349,29 +1349,36 @@ struct ObjectGroupCompartment::Allocatio
     static const uint32_t OFFSET_LIMIT = (1 << 23);
 
     AllocationSiteKey(JSScript* script_, uint32_t offset_, JSProtoKey kind_, JSObject* proto_)
       : script(script_), offset(offset_), kind(kind_), proto(proto_)
     {
         MOZ_ASSERT(offset_ < OFFSET_LIMIT);
     }
 
+    AllocationSiteKey(const AllocationSiteKey& key)
+      : script(key.script),
+        offset(key.offset),
+        kind(key.kind),
+        proto(key.proto)
+    { }
+
     AllocationSiteKey(AllocationSiteKey&& key)
       : script(mozilla::Move(key.script)),
         offset(key.offset),
         kind(key.kind),
         proto(mozilla::Move(key.proto))
     { }
 
-    AllocationSiteKey(const AllocationSiteKey& key)
-      : script(key.script),
-        offset(key.offset),
-        kind(key.kind),
-        proto(key.proto)
-    { }
+    void operator=(AllocationSiteKey&& key) {
+        script = mozilla::Move(key.script);
+        offset = key.offset;
+        kind = key.kind;
+        proto = mozilla::Move(key.proto);
+    }
 
     static inline uint32_t hash(AllocationSiteKey key) {
         return uint32_t(size_t(key.script->offsetToPC(key.offset)) ^ key.kind ^
                MovableCellHasher<JSObject*>::hash(key.proto));
     }
 
     static inline bool match(const AllocationSiteKey& a, const AllocationSiteKey& b) {
         return DefaultHasher<JSScript*>::match(a.script, b.script) &&
--- a/mfbt/Alignment.h
+++ b/mfbt/Alignment.h
@@ -115,26 +115,40 @@ struct AlignedStorage
   union U
   {
     char mBytes[Nbytes];
     uint64_t mDummy;
   } u;
 
   const void* addr() const { return u.mBytes; }
   void* addr() { return u.mBytes; }
+
+  AlignedStorage() = default;
+
+  // AlignedStorage is non-copyable: the default copy constructor violates
+  // strict aliasing rules, per bug 1269319.
+  AlignedStorage(const AlignedStorage&) = delete;
+  void operator=(const AlignedStorage&) = delete;
 };
 
 template<typename T>
 struct MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS AlignedStorage2
 {
   union U
   {
     char mBytes[sizeof(T)];
     uint64_t mDummy;
   } u;
 
   const T* addr() const { return reinterpret_cast<const T*>(u.mBytes); }
   T* addr() { return static_cast<T*>(static_cast<void*>(u.mBytes)); }
+
+  AlignedStorage2() = default;
+
+  // AlignedStorage2 is non-copyable: the default copy constructor violates
+  // strict aliasing rules, per bug 1269319.
+  AlignedStorage2(const AlignedStorage2&) = delete;
+  void operator=(const AlignedStorage2&) = delete;
 };
 
 } /* namespace mozilla */
 
 #endif /* mozilla_Alignment_h */