Bug 1341951 - Use alignas/alignof (rather than a union that attempts to replicate the same thing) inside RInstructionStorage. r=nbp
authorJeff Walden <jwalden@mit.edu>
Thu, 09 Feb 2017 17:12:56 -0800
changeset 373786 039475e399397ce1a79a0b31f2379a43eb52b894
parent 373785 7e72aa400561c7767ca4734f23ff3903e55d4e0a
child 373787 2b2b65a1b476e98f610299100e97cd401e9b46c0
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1341951
milestone54.0a1
Bug 1341951 - Use alignas/alignof (rather than a union that attempts to replicate the same thing) inside RInstructionStorage. r=nbp
js/src/jit/Recover.h
js/src/jit/Snapshots.h
--- a/js/src/jit/Recover.h
+++ b/js/src/jit/Recover.h
@@ -109,17 +109,17 @@ namespace jit {
     _(ObjectState)                              \
     _(ArrayState)                               \
     _(AtomicIsLockFree)                         \
     _(AssertRecoveredOnBailout)
 
 class RResumePoint;
 class SnapshotIterator;
 
-class RInstruction
+class MOZ_NON_PARAM RInstruction
 {
   public:
     enum Opcode
     {
 #   define DEFINE_OPCODES_(op) Recover_##op,
         RECOVER_OPCODE_LIST(DEFINE_OPCODES_)
 #   undef DEFINE_OPCODES_
         Recover_Invalid
--- a/js/src/jit/Snapshots.h
+++ b/js/src/jit/Snapshots.h
@@ -3,16 +3,17 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef jit_Snapshot_h
 #define jit_Snapshot_h
 
 #include "mozilla/Alignment.h"
+#include "mozilla/Attributes.h"
 
 #include "jsalloc.h"
 #include "jsbytecode.h"
 
 #include "jit/CompactBuffer.h"
 #include "jit/IonTypes.h"
 #include "jit/Registers.h"
 
@@ -499,62 +500,53 @@ class SnapshotReader
     uint32_t numAllocationsRead() const {
         return allocRead_;
     }
     void resetNumAllocationsRead() {
         allocRead_ = 0;
     }
 };
 
-class RInstructionStorage
+class MOZ_NON_PARAM RInstructionStorage
 {
+    static constexpr size_t Size = 4 * sizeof(uint32_t);
+
+    // This presumes all RInstructionStorage are safely void*-alignable.
+    // RInstruction::readRecoverData asserts that no RInstruction subclass
+    // has stricter alignment requirements than RInstructionStorage.
+    static constexpr size_t Alignment = alignof(void*);
+
     /*
      * DO NOT REUSE THIS IMPLEMENTATION TACTIC.
      *
      * It is undefined behavior to bytewise-copy the bytes of one T, into a
-     * location that isn't a T -- and because the target |mBytes| often wasn't
-     * previously constructed as a T, it doesn't always count as such -- and
-     * then interpret that second location as T.  (This is *possibly* okay if T
-     * is POD or standard layout or is trivial.  None of those are true here.)
+     * location that isn't a T -- and because the target |mem| often wasn't
+     * already constructed as a T, it doesn't always count as such -- and then
+     * interpret that second location as T.  (This is *possibly* okay if T is
+     * POD or standard layout or is trivial.  None of those are true here.)
      * The C++ spec considers this a strict aliasing violation.  We've hit
      * crashes before when we've done this: for example, bug 1269319.
      *
      * This tactic is used here *only* because it was mistakenly added long
      * ago, before it was recognized to be buggy, and we haven't yet struggled
      * through the effort to remove it.
      */
-    static constexpr size_t Size = 4 * sizeof(uint32_t);
-    union U
-    {
-        char mBytes[Size];
-
-        // This presumes all RInstructionStorage are safely uint64_t-alignable.
-        // RInstruction::readRecoverData asserts that no RInstruction subclass
-        // has stricter alignment requirements than RInstructionStorage.
-        uint64_t mDummy;
-    } u;
+    alignas(Alignment) unsigned char mem[Size];
 
   public:
-    const void* addr() const { return u.mBytes; }
-    void* addr() { return u.mBytes; }
+    const void* addr() const { return mem; }
+    void* addr() { return mem; }
 
     RInstructionStorage() = default;
 
-    RInstructionStorage(const RInstructionStorage& other) {
-        // FIXME: This is a strict aliasing violation: see the comment above,
-        //        and see bug 1269319 for an instance of this idea having
-        //        caused crashes.
-        memcpy(addr(), other.addr(), Size);
-    }
-    void operator=(const RInstructionStorage& other) {
-        // FIXME: This is a strict aliasing violation: see the comment above,
-        //        and see bug 1269319 for an instance of this idea having
-        //        caused crashes.
-        memcpy(addr(), other.addr(), Size);
-    }
+    // FIXME (bug 1341951): These are strict aliasing violations: see the
+    // comment above, and see bug 1269319 for an instance of bytewise copying
+    // having caused crashes.
+    RInstructionStorage(const RInstructionStorage& other) = default;
+    RInstructionStorage& operator=(const RInstructionStorage& other) = default;
 };
 
 class RInstruction;
 
 class RecoverReader
 {
     CompactBufferReader reader_;