Convert the trailing array of BindingNames at the end of the various kinds of scope data into raw unsigned chars into which those BindingNames are placement-new'd, rather than memcpy-ing non-trivial classes around and failing to comply with the C++ object model

2 files changed, 46 insertions, 13 deletions

diff --git a/js/src/vm/Scope.cpp b/js/src/vm/Scope.cpp
index 112b34586..25e389d8a 100644
--- a/js/src/vm/Scope.cpp
+++ b/js/src/vm/Scope.cpp
@@ -273,7 +273,7 @@ Scope::XDRSizedBindingNames(XDRState<mode>* xdr, Handle<ConcreteScope*> scope,
     }
 
     for (uint32_t i = 0; i < length; i++) {
-        if (!XDRBindingName(xdr, &data->names[i])) {
+        if (!XDRBindingName(xdr, &data->trailingNames[i])) {
             if (mode == XDR_DECODE) {
                 DeleteScopeData(data.get());
                 data.set(nullptr);
@@ -1250,7 +1250,7 @@ BindingIter::init(LexicalScope::Data& data, uint32_t firstFrameSlot, uint8_t fla
         init(0, 0, 0, 0, 0, 0,
              CanHaveEnvironmentSlots | flags,
              firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
-             data.names, data.length);
+             data.trailingNames.start(), data.length);
     } else {
         //            imports - [0, 0)
         // positional formals - [0, 0)
@@ -1262,7 +1262,7 @@ BindingIter::init(LexicalScope::Data& data, uint32_t firstFrameSlot, uint8_t fla
         init(0, 0, 0, 0, 0, data.constStart,
              CanHaveFrameSlots | CanHaveEnvironmentSlots | flags,
              firstFrameSlot, JSSLOT_FREE(&LexicalEnvironmentObject::class_),
-             data.names, data.length);
+             data.trailingNames.start(), data.length);
     }
 }
 
@@ -1283,7 +1283,7 @@ BindingIter::init(FunctionScope::Data& data, uint8_t flags)
     init(0, data.nonPositionalFormalStart, data.varStart, data.varStart, data.length, data.length,
          flags,
          0, JSSLOT_FREE(&CallObject::class_),
-         data.names, data.length);
+         data.trailingNames.start(), data.length);
 }
 
 void
@@ -1299,7 +1299,7 @@ BindingIter::init(VarScope::Data& data, uint32_t firstFrameSlot)
     init(0, 0, 0, 0, data.length, data.length,
          CanHaveFrameSlots | CanHaveEnvironmentSlots,
          firstFrameSlot, JSSLOT_FREE(&VarEnvironmentObject::class_),
-         data.names, data.length);
+         data.trailingNames.start(), data.length);
 }
 
 void
@@ -1343,7 +1343,7 @@ BindingIter::init(EvalScope::Data& data, bool strict)
     //             consts - [data.length, data.length)
     init(0, 0, 0, data.varStart, data.length, data.length,
          flags, firstFrameSlot, firstEnvironmentSlot,
-         data.names, data.length);
+         data.trailingNames.start(), data.length);
 }
 
 void
@@ -1359,7 +1359,7 @@ BindingIter::init(ModuleScope::Data& data)
     init(data.varStart, data.varStart, data.varStart, data.varStart, data.letStart, data.constStart,
          CanHaveFrameSlots | CanHaveEnvironmentSlots,
          0, JSSLOT_FREE(&ModuleEnvironmentObject::class_),
-         data.names, data.length);
+         data.trailingNames.start(), data.length);
 }
 
 PositionalFormalParameterIter::PositionalFormalParameterIter(JSScript* script)
diff --git a/js/src/vm/Scope.h b/js/src/vm/Scope.h
index 5304d6713..387eb30eb 100644
--- a/js/src/vm/Scope.h
+++ b/js/src/vm/Scope.h
@@ -111,6 +111,39 @@ class BindingName
     void trace(JSTracer* trc);
 };
 
+/**
+ * The various {Global,Module,...}Scope::Data classes consist of always-present
+ * bits, then a trailing array of BindingNames.  The various Data classes all
+ * end in a TrailingNamesArray that contains sized/aligned space for *one*
+ * BindingName.  Data instances that contain N BindingNames, are then allocated
+ * in sizeof(Data) + (space for (N - 1) BindingNames).  Because this class's
+ * |data_| field is properly sized/aligned, the N-BindingName array can start
+ * at |data_|.
+ *
+ * This is concededly a very low-level representation, but we want to only
+ * allocate once for data+bindings both, and this does so approximately as
+ * elegantly as C++ allows.
+ */
+class TrailingNamesArray
+{
+  private:
+    alignas(BindingName) unsigned char data_[sizeof(BindingName)];
+
+  private:
+    // Some versions of GCC treat it as a -Wstrict-aliasing violation (ergo a
+    // -Werror compile error) to reinterpret_cast<> |data_| to |T*|, even
+    // through |void*|.  Placing the latter cast in these separate functions
+    // breaks the chain such that affected GCC versions no longer warn/error.
+    void* ptr() {
+        return data_;
+    }
+
+  public:
+    BindingName* start() { return reinterpret_cast<BindingName*>(ptr()); }
+
+    BindingName& operator[](size_t i) { return start()[i]; }
+};
+
 class BindingLocation
 {
   public:
@@ -346,7 +379,7 @@ class LexicalScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };
@@ -462,7 +495,7 @@ class FunctionScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };
@@ -556,7 +589,7 @@ class VarScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };
@@ -645,7 +678,7 @@ class GlobalScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };
@@ -745,7 +778,7 @@ class EvalScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };
@@ -846,7 +879,7 @@ class ModuleScope : public Scope
 
         // The array of tagged JSAtom* names, allocated beyond the end of the
         // struct.
-        BindingName names[1];
+        TrailingNamesArray trailingNames;
 
         void trace(JSTracer* trc);
     };