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 481303 a0883c99c897122286c6e1711cada83d9889e093
parent 481302 7e7cfbc863e6dd839f31c8d1b4cca3c04b254f5c
child 481304 aa3d1b33b0e94e6dae117d2e32e5cd007debd36a
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersluke, waldo
bugs1467632
milestone63.0a1
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);
 }