Bug 1467632 - Make AsmJSGlobal's pod field be POD. r=luke, r=waldo
authorLars T Hansen <lhansen@mozilla.com>
Mon, 20 Aug 2018 17:14:19 +0200
changeset 433032 a0883c99c897122286c6e1711cada83d9889e093
parent 433031 7e7cfbc863e6dd839f31c8d1b4cca3c04b254f5c
child 433033 aa3d1b33b0e94e6dae117d2e32e5cd007debd36a
push id34499
push usercsabou@mozilla.com
push dateThu, 23 Aug 2018 21:40:51 +0000
treeherdermozilla-central@49b70f7e6817 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke, waldo
bugs1467632
milestone63.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 1467632 - Make AsmJSGlobal's pod field be POD. r=luke, r=waldo The pod member needs to be POD but has members that have evolved no longer to be POD - a ValType and a LitVal. We work around the problem locally by using ValType's representation type PackedTypeCode to represent types, and by specializing LitVal as LitValPOD for use in this structure.
js/src/wasm/AsmJS.cpp
js/src/wasm/WasmTypes.h
--- a/js/src/wasm/AsmJS.cpp
+++ b/js/src/wasm/AsmJS.cpp
@@ -101,44 +101,82 @@ enum AsmJSMathBuiltinFunction
     AsmJSMathBuiltin_asin, AsmJSMathBuiltin_acos, AsmJSMathBuiltin_atan,
     AsmJSMathBuiltin_ceil, AsmJSMathBuiltin_floor, AsmJSMathBuiltin_exp,
     AsmJSMathBuiltin_log, AsmJSMathBuiltin_pow, AsmJSMathBuiltin_sqrt,
     AsmJSMathBuiltin_abs, AsmJSMathBuiltin_atan2, AsmJSMathBuiltin_imul,
     AsmJSMathBuiltin_fround, AsmJSMathBuiltin_min, AsmJSMathBuiltin_max,
     AsmJSMathBuiltin_clz32
 };
 
+// LitValPOD is a restricted version of LitVal suitable for asm.js that is
+// always POD.
+
+struct LitValPOD
+{
+    PackedTypeCode valType_;
+    union U {
+        uint32_t  u32_;
+        uint64_t  u64_;
+        float     f32_;
+        double    f64_;
+    } u;
+
+    LitValPOD() = default;
+
+    explicit LitValPOD(uint32_t u32) : valType_(ValType(ValType::I32).packed()) { u.u32_ = u32; }
+    explicit LitValPOD(uint64_t u64) : valType_(ValType(ValType::I64).packed()) { u.u64_ = u64; }
+
+    explicit LitValPOD(float f32) : valType_(ValType(ValType::F32).packed()) { u.f32_ = f32; }
+    explicit LitValPOD(double f64) : valType_(ValType(ValType::F64).packed()) { u.f64_ = f64; }
+
+    LitVal asLitVal() const {
+        switch (UnpackTypeCodeType(valType_)) {
+          case TypeCode::I32:
+            return LitVal(u.u32_);
+          case TypeCode::I64:
+            return LitVal(u.u64_);
+          case TypeCode::F32:
+            return LitVal(u.f32_);
+          case TypeCode::F64:
+            return LitVal(u.f64_);
+          default:
+            MOZ_CRASH("Can't happen");
+        }
+    }
+};
+
+static_assert(std::is_pod<LitValPOD>::value,
+              "must be POD to be simply serialized/deserialized");
+
 // An AsmJSGlobal represents a JS global variable in the asm.js module function.
 class AsmJSGlobal
 {
   public:
     enum Which { Variable, FFI, ArrayView, ArrayViewCtor, MathBuiltinFunction, Constant };
     enum VarInitKind { InitConstant, InitImport };
     enum ConstantKind { GlobalConstant, MathConstant };
 
   private:
     struct CacheablePod {
         Which which_;
         union V {
             struct {
                 VarInitKind initKind_;
                 union U {
-                    ValType importType_;
-                    LitVal val_;
-                    U() : val_(LitVal()) {}
+                    PackedTypeCode importValType_;
+                    LitValPOD val_;
                 } u;
             } var;
             uint32_t ffiIndex_;
             Scalar::Type viewType_;
             AsmJSMathBuiltinFunction mathBuiltinFunc_;
             struct {
                 ConstantKind kind_;
                 double value_;
             } constant;
-            V() : ffiIndex_(0) {}
         } u;
     } pod;
     CacheableChars field_;
 
     friend class ModuleValidator;
 
   public:
     AsmJSGlobal() = default;
@@ -152,25 +190,25 @@ class AsmJSGlobal
     }
     Which which() const {
         return pod.which_;
     }
     VarInitKind varInitKind() const {
         MOZ_ASSERT(pod.which_ == Variable);
         return pod.u.var.initKind_;
     }
-    LitVal varInitVal() const {
+    LitValPOD varInitVal() const {
         MOZ_ASSERT(pod.which_ == Variable);
         MOZ_ASSERT(pod.u.var.initKind_ == InitConstant);
         return pod.u.var.u.val_;
     }
     ValType varInitImportType() const {
         MOZ_ASSERT(pod.which_ == Variable);
         MOZ_ASSERT(pod.u.var.initKind_ == InitImport);
-        return pod.u.var.u.importType_;
+        return ValType(pod.u.var.u.importValType_);
     }
     uint32_t ffiIndex() const {
         MOZ_ASSERT(pod.which_ == FFI);
         return pod.u.ffiIndex_;
     }
     // When a view is created from an imported constructor:
     //   var I32 = stdlib.Int32Array;
     //   var i32 = new I32(buffer);
@@ -846,26 +884,26 @@ class NumLit
           case NumLit::Float:
             return IsPositiveZero(toFloat());
           case NumLit::OutOfRangeInt:
             MOZ_CRASH("can't be here because of valid() check above");
         }
         return false;
     }
 
-    LitVal value() const {
+    LitValPOD value() const {
         switch (which_) {
           case NumLit::Fixnum:
           case NumLit::NegativeInt:
           case NumLit::BigUnsigned:
-            return LitVal(toUint32());
+            return LitValPOD(toUint32());
           case NumLit::Float:
-            return LitVal(toFloat());
+            return LitValPOD(toFloat());
           case NumLit::Double:
-            return LitVal(toDouble());
+            return LitValPOD(toDouble());
           case NumLit::OutOfRangeInt:;
         }
         MOZ_CRASH("bad literal");
     }
 };
 
 // Represents the type of a general asm.js expression.
 //
@@ -1686,17 +1724,17 @@ class MOZ_STACK_CLASS JS_HAZ_ROOTED Modu
         if (!global)
             return false;
         new (&global->u.varOrConst) Global::U::VarOrConst(index, type.which());
         if (!globalMap_.putNew(var, global))
             return false;
 
         AsmJSGlobal g(AsmJSGlobal::Variable, std::move(fieldChars));
         g.pod.u.var.initKind_ = AsmJSGlobal::InitImport;
-        g.pod.u.var.u.importType_ = valType;
+        g.pod.u.var.u.importValType_ = valType.packed();
         return asmJSMetadata_->asmJSGlobals.append(std::move(g));
     }
     bool addArrayView(PropertyName* var, Scalar::Type vt, PropertyName* maybeField) {
         UniqueChars fieldChars;
         if (maybeField) {
             fieldChars = StringToNewUTF8CharsZ(cx_, *maybeField);
             if (!fieldChars)
                 return false;
@@ -5786,17 +5824,17 @@ HasPureCoercion(JSContext* cx, HandleVal
     {
         return true;
     }
     return false;
 }
 
 static bool
 ValidateGlobalVariable(JSContext* cx, const AsmJSGlobal& global, HandleValue importVal,
-                       Maybe<LitVal>* val)
+                       Maybe<LitValPOD>* val)
 {
     switch (global.varInitKind()) {
       case AsmJSGlobal::InitConstant:
         val->emplace(global.varInitVal());
         return true;
 
       case AsmJSGlobal::InitImport: {
         RootedValue v(cx);
@@ -6008,20 +6046,20 @@ GetImports(JSContext* cx, const AsmJSMet
 {
     Rooted<FunctionVector> ffis(cx, FunctionVector(cx));
     if (!ffis.resize(metadata.numFFIs))
         return false;
 
     for (const AsmJSGlobal& global : metadata.asmJSGlobals) {
         switch (global.which()) {
           case AsmJSGlobal::Variable: {
-            Maybe<LitVal> litVal;
+            Maybe<LitValPOD> litVal;
             if (!ValidateGlobalVariable(cx, global, importVal, &litVal))
                 return false;
-            if (!valImports.append(Val(*litVal)))
+            if (!valImports.append(Val(litVal->asLitVal())))
                 return false;
             break;
           }
           case AsmJSGlobal::FFI:
             if (!ValidateFFI(cx, global, importVal, &ffis))
                 return false;
             break;
           case AsmJSGlobal::ArrayView:
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -163,16 +163,19 @@ struct ShareableBase : AtomicRefCounted<
 //
 // PackedTypeCode is an enum class, as opposed to the more natural
 // struct-with-bitfields, because bitfields would make it non-POD.
 //
 // DO NOT use PackedTypeCode as a cast.  ALWAYS go via PackTypeCode().
 
 enum class PackedTypeCode : uint32_t {};
 
+static_assert(std::is_pod<PackedTypeCode>::value,
+              "must be POD to be simply serialized/deserialized");
+
 const uint32_t NoTypeCode     = 0xFF;      // Only use these
 const uint32_t NoRefTypeIndex = 0xFFFFFF;  //   with PackedTypeCode
 
 static inline PackedTypeCode
 InvalidPackedTypeCode()
 {
     return PackedTypeCode((NoRefTypeIndex << 8) | NoTypeCode);
 }