Bug 1401675 - Baldr: factor out Decoder::readValType() (r=lth)
authorLuke Wagner <luke@mozilla.com>
Tue, 12 Feb 2019 09:37:48 -0600
changeset 458708 17988b9f30d4
parent 458707 9aca1805a96a
child 458709 6ae6def2e6ed
push id35546
push userrmaries@mozilla.com
push dateWed, 13 Feb 2019 04:27:59 +0000
treeherdermozilla-central@636d2c00234d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslth
bugs1401675
milestone67.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 1401675 - Baldr: factor out Decoder::readValType() (r=lth)
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmIonCompile.cpp
js/src/wasm/WasmOpIter.h
js/src/wasm/WasmTypes.h
js/src/wasm/WasmValidate.cpp
js/src/wasm/WasmValidate.h
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -11970,18 +11970,17 @@ bool js::wasm::BaselineCompileFunctions(
     Decoder d(func.begin, func.end, func.lineOrBytecode, error);
 
     // Build the local types vector.
 
     ValTypeVector locals;
     if (!locals.appendAll(env.funcTypes[func.index]->args())) {
       return false;
     }
-    if (!DecodeLocalEntries(d, env.kind, env.types, env.gcTypesEnabled(),
-                            &locals)) {
+    if (!DecodeLocalEntries(d, env.types, env.gcTypesEnabled(), &locals)) {
       return false;
     }
 
     // One-pass baseline compilation.
 
     BaseCompiler f(env, func, locals, trapExitLayout, trapExitLayoutNumWords, d,
                    &alloc, &masm, &code->stackMaps);
     if (!f.init()) {
--- a/js/src/wasm/WasmIonCompile.cpp
+++ b/js/src/wasm/WasmIonCompile.cpp
@@ -4030,18 +4030,17 @@ bool wasm::IonCompileFunctions(const Mod
     Decoder d(func.begin, func.end, func.lineOrBytecode, error);
 
     // Build the local types vector.
 
     ValTypeVector locals;
     if (!locals.appendAll(env.funcTypes[func.index]->args())) {
       return false;
     }
-    if (!DecodeLocalEntries(d, env.kind, env.types, env.gcTypesEnabled(),
-                            &locals)) {
+    if (!DecodeLocalEntries(d,  env.types, env.gcTypesEnabled(), &locals)) {
       return false;
     }
 
     // Set up for Ion compilation.
 
     const JitCompileOptions options;
     MIRGraph graph(&alloc);
     CompileInfo compileInfo(locals.length());
--- a/js/src/wasm/WasmOpIter.h
+++ b/js/src/wasm/WasmOpIter.h
@@ -528,16 +528,17 @@ class MOZ_STACK_CLASS OpIter : private P
   MOZ_MUST_USE bool readTableSize(uint32_t* tableIndex);
   MOZ_MUST_USE bool readStructNew(uint32_t* typeIndex, ValueVector* argValues);
   MOZ_MUST_USE bool readStructGet(uint32_t* typeIndex, uint32_t* fieldIndex,
                                   Value* ptr);
   MOZ_MUST_USE bool readStructSet(uint32_t* typeIndex, uint32_t* fieldIndex,
                                   Value* ptr, Value* val);
   MOZ_MUST_USE bool readStructNarrow(ValType* inputType, ValType* outputType,
                                      Value* ptr);
+  MOZ_MUST_USE bool readValType(ValType* type);
   MOZ_MUST_USE bool readReferenceType(ValType* type, const char* const context);
 
   // At a location where readOp is allowed, peek at the next opcode
   // without consuming it or updating any internal state.
   // Never fails: returns uint16_t(Op::Limit) in op->b0 if it can't read.
   void peekOp(OpBytes* op);
 
   // ------------------------------------------------------------------------
@@ -1550,38 +1551,27 @@ inline bool OpIter<Policy>::readF64Const
 template <typename Policy>
 inline bool OpIter<Policy>::readRefNull() {
   MOZ_ASSERT(Classify(op_) == OpKind::RefNull);
 
   return push(StackType(ValType::NullRef));
 }
 
 template <typename Policy>
+inline bool OpIter<Policy>::readValType(ValType* type) {
+  return d_.readValType(env_.types, env_.gcTypesEnabled(), type);
+}
+
+template <typename Policy>
 inline bool OpIter<Policy>::readReferenceType(ValType* type,
                                               const char* context) {
-  uint8_t code;
-  uint32_t refTypeIndex;
-
-  if (!d_.readValType(&code, &refTypeIndex)) {
+  if (!readValType(type) || !type->isReference()) {
     return fail_ctx("invalid reference type for %s", context);
   }
 
-  if (code == uint8_t(ValType::Code::Ref)) {
-    if (refTypeIndex >= env_.types.length()) {
-      return fail_ctx("invalid reference type for %s", context);
-    }
-    if (!env_.types[refTypeIndex].isStructType()) {
-      return fail_ctx("reference to struct required for %s", context);
-    }
-  } else if (code != uint8_t(ValType::Code::AnyRef)) {
-    return fail_ctx("invalid reference type for %s", context);
-  }
-
-  *type = ValType(ValType::Code(code), refTypeIndex);
-
   return true;
 }
 
 template <typename Policy>
 inline bool OpIter<Policy>::popCallArgs(const ValTypeVector& expectedTypes,
                                         ValueVector* values) {
   // Iterate through the argument types backward so that pops occur in the
   // right order.
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -218,16 +218,17 @@ static inline PackedTypeCode PackTypeCod
   MOZ_ASSERT(tc != TypeCode::Ref);
   return PackedTypeCode((NoRefTypeIndex << 8) | uint32_t(tc));
 }
 
 static inline PackedTypeCode PackTypeCode(TypeCode tc, uint32_t refTypeIndex) {
   MOZ_ASSERT(uint32_t(tc) <= 0xFF);
   MOZ_ASSERT_IF(tc != TypeCode::Ref, refTypeIndex == NoRefTypeIndex);
   MOZ_ASSERT_IF(tc == TypeCode::Ref, refTypeIndex <= MaxTypes);
+  static_assert(MaxTypes < (1 << (32 - 8)), "enough bits");
   return PackedTypeCode((refTypeIndex << 8) | uint32_t(tc));
 }
 
 static inline PackedTypeCode PackedTypeCodeFromBits(uint32_t bits) {
   return PackTypeCode(TypeCode(bits & 255), bits >> 8);
 }
 
 static inline bool IsValid(PackedTypeCode ptc) {
--- a/js/src/wasm/WasmValidate.cpp
+++ b/js/src/wasm/WasmValidate.cpp
@@ -389,111 +389,54 @@ bool wasm::EncodeLocalEntries(Encoder& e
     if (!e.writeValType(prev)) {
       return false;
     }
   }
 
   return true;
 }
 
-static bool DecodeValType(Decoder& d, ModuleKind kind, uint32_t numTypes,
-                          bool gcTypesEnabled, ValType* type) {
-  uint8_t uncheckedCode;
-  uint32_t uncheckedRefTypeIndex;
-  if (!d.readValType(&uncheckedCode, &uncheckedRefTypeIndex)) {
-    return false;
-  }
-
-  switch (uncheckedCode) {
-    case uint8_t(ValType::I32):
-    case uint8_t(ValType::F32):
-    case uint8_t(ValType::F64):
-    case uint8_t(ValType::I64):
-      *type = ValType(ValType::Code(uncheckedCode));
-      return true;
-    case uint8_t(ValType::AnyRef):
-      if (!gcTypesEnabled) {
-        return d.fail("reference types not enabled");
-      }
-      *type = ValType(ValType::Code(uncheckedCode));
-      return true;
-    case uint8_t(ValType::Ref): {
-      if (!gcTypesEnabled) {
-        return d.fail("reference types not enabled");
-      }
-      if (uncheckedRefTypeIndex >= numTypes) {
-        return d.fail("ref index out of range");
-      }
-      // We further validate ref types in the caller.
-      *type = ValType(ValType::Code(uncheckedCode), uncheckedRefTypeIndex);
-      return true;
-    }
-    default:
-      break;
-  }
-  return d.fail("bad type");
-}
-
-static bool ValidateRefType(Decoder& d, const TypeDefVector& types,
-                            ValType type) {
-  if (type.isRef() && !types[type.refTypeIndex()].isStructType()) {
-    return d.fail("ref does not reference a struct type");
-  }
-  return true;
-}
-
-bool wasm::DecodeLocalEntries(Decoder& d, ModuleKind kind,
-                              const TypeDefVector& types, bool gcTypesEnabled,
-                              ValTypeVector* locals) {
+bool wasm::DecodeLocalEntries(Decoder& d, const TypeDefVector& types,
+                              bool gcTypesEnabled, ValTypeVector* locals) {
   uint32_t numLocalEntries;
   if (!d.readVarU32(&numLocalEntries)) {
     return d.fail("failed to read number of local entries");
   }
 
   for (uint32_t i = 0; i < numLocalEntries; i++) {
     uint32_t count;
     if (!d.readVarU32(&count)) {
       return d.fail("failed to read local entry count");
     }
 
     if (MaxLocals - locals->length() < count) {
       return d.fail("too many locals");
     }
 
     ValType type;
-    if (!DecodeValType(d, kind, types.length(), gcTypesEnabled, &type)) {
-      return false;
-    }
-    if (!ValidateRefType(d, types, type)) {
+    if (!d.readValType(types, gcTypesEnabled, &type)) {
       return false;
     }
 
     if (!locals->appendN(type, count)) {
       return false;
     }
   }
 
   return true;
 }
 
 bool wasm::DecodeValidatedLocalEntries(Decoder& d, ValTypeVector* locals) {
   uint32_t numLocalEntries;
   MOZ_ALWAYS_TRUE(d.readVarU32(&numLocalEntries));
 
   for (uint32_t i = 0; i < numLocalEntries; i++) {
-    uint32_t count;
-    MOZ_ALWAYS_TRUE(d.readVarU32(&count));
+    uint32_t count = d.uncheckedReadVarU32();
     MOZ_ASSERT(MaxLocals - locals->length() >= count);
-
-    uint8_t uncheckedCode;
-    uint32_t uncheckedRefTypeIndex;
-    MOZ_ALWAYS_TRUE(d.readValType(&uncheckedCode, &uncheckedRefTypeIndex));
-
-    ValType type = ValType(ValType::Code(uncheckedCode), uncheckedRefTypeIndex);
-    if (!locals->appendN(type, count)) {
+    if (!locals->appendN(d.uncheckedReadValType(), count)) {
       return false;
     }
   }
 
   return true;
 }
 
 // Function body validation.
@@ -1188,18 +1131,17 @@ bool wasm::ValidateFunctionBody(const Mo
 
   ValTypeVector locals;
   if (!locals.appendAll(funcType.args())) {
     return false;
   }
 
   const uint8_t* bodyBegin = d.currentPosition();
 
-  if (!DecodeLocalEntries(d, ModuleKind::Wasm, env.types, env.gcTypesEnabled(),
-                          &locals)) {
+  if (!DecodeLocalEntries(d, env.types, env.gcTypesEnabled(), &locals)) {
     return false;
   }
 
   if (!DecodeFunctionBodyExprs(env, funcType, locals, bodyBegin + bodySize,
                                &d)) {
     return false;
   }
 
@@ -1226,18 +1168,18 @@ static bool DecodePreamble(Decoder& d) {
 
   return true;
 }
 
 enum class TypeState { None, Struct, ForwardStruct, Func };
 
 typedef Vector<TypeState, 0, SystemAllocPolicy> TypeStateVector;
 
-static bool ValidateRefType(Decoder& d, TypeStateVector* typeState,
-                            ValType type) {
+static bool ValidateTypeState(Decoder& d, TypeStateVector* typeState,
+                              ValType type) {
   if (!type.isRef()) {
     return true;
   }
 
   uint32_t refTypeIndex = type.refTypeIndex();
   switch ((*typeState)[refTypeIndex]) {
     case TypeState::None:
       (*typeState)[refTypeIndex] = TypeState::ForwardStruct;
@@ -1272,21 +1214,20 @@ static bool DecodeFuncType(Decoder& d, M
   }
 
   ValTypeVector args;
   if (!args.resize(numArgs)) {
     return false;
   }
 
   for (uint32_t i = 0; i < numArgs; i++) {
-    if (!DecodeValType(d, ModuleKind::Wasm, env->types.length(),
-                       env->gcTypesEnabled(), &args[i])) {
+    if (!d.readValType(env->types.length(), env->gcTypesEnabled(), &args[i])) {
       return false;
     }
-    if (!ValidateRefType(d, typeState, args[i])) {
+    if (!ValidateTypeState(d, typeState, args[i])) {
       return false;
     }
   }
 
   uint32_t numRets;
   if (!d.readVarU32(&numRets)) {
     return d.fail("bad number of function returns");
   }
@@ -1294,21 +1235,20 @@ static bool DecodeFuncType(Decoder& d, M
   if (numRets > 1) {
     return d.fail("too many returns in signature");
   }
 
   ExprType result = ExprType::Void;
 
   if (numRets == 1) {
     ValType type;
-    if (!DecodeValType(d, ModuleKind::Wasm, env->types.length(),
-                       env->gcTypesEnabled(), &type)) {
+    if (!d.readValType(env->types.length(), env->gcTypesEnabled(), &type)) {
       return false;
     }
-    if (!ValidateRefType(d, typeState, type)) {
+    if (!ValidateTypeState(d, typeState, type)) {
       return false;
     }
 
     result = ExprType(type);
   }
 
   if ((*typeState)[typeIndex] != TypeState::None) {
     return d.fail("function type entry referenced as struct");
@@ -1345,21 +1285,21 @@ static bool DecodeStructType(Decoder& d,
     uint8_t flags;
     if (!d.readFixedU8(&flags)) {
       return d.fail("expected flag");
     }
     if ((flags & ~uint8_t(FieldFlags::AllowedMask)) != 0) {
       return d.fail("garbage flag bits");
     }
     fields[i].isMutable = flags & uint8_t(FieldFlags::Mutable);
-    if (!DecodeValType(d, ModuleKind::Wasm, env->types.length(),
-                       env->gcTypesEnabled(), &fields[i].type)) {
+    if (!d.readValType(env->types.length(), env->gcTypesEnabled(),
+                       &fields[i].type)) {
       return false;
     }
-    if (!ValidateRefType(d, typeState, fields[i].type)) {
+    if (!ValidateTypeState(d, typeState, fields[i].type)) {
       return false;
     }
 
     CheckedInt32 offset;
     switch (fields[i].type.code()) {
       case ValType::I32:
         offset = layout.addScalar(Scalar::Int32);
         break;
@@ -1667,21 +1607,17 @@ static bool GlobalIsJSCompatible(Decoder
   }
 
   return true;
 }
 
 static bool DecodeGlobalType(Decoder& d, const TypeDefVector& types,
                              bool gcTypesEnabled, ValType* type,
                              bool* isMutable) {
-  if (!DecodeValType(d, ModuleKind::Wasm, types.length(), gcTypesEnabled,
-                     type)) {
-    return false;
-  }
-  if (!ValidateRefType(d, types, *type)) {
+  if (!d.readValType(types, gcTypesEnabled, type)) {
     return false;
   }
 
   uint8_t flags;
   if (!d.readFixedU8(&flags)) {
     return d.fail("expected global flags");
   }
 
--- a/js/src/wasm/WasmValidate.h
+++ b/js/src/wasm/WasmValidate.h
@@ -578,30 +578,71 @@ class Decoder {
   MOZ_MUST_USE bool readVarU32(uint32_t* out) {
     return readVarU<uint32_t>(out);
   }
   MOZ_MUST_USE bool readVarS32(int32_t* out) { return readVarS<int32_t>(out); }
   MOZ_MUST_USE bool readVarU64(uint64_t* out) {
     return readVarU<uint64_t>(out);
   }
   MOZ_MUST_USE bool readVarS64(int64_t* out) { return readVarS<int64_t>(out); }
-  MOZ_MUST_USE bool readValType(uint8_t* code, uint32_t* refTypeIndex) {
+
+  MOZ_MUST_USE ValType uncheckedReadValType() {
+    uint8_t code = uncheckedReadFixedU8();
+    switch (code) {
+      case uint8_t(ValType::Ref):
+        return ValType(ValType::Code(code), uncheckedReadVarU32());
+      default:
+        return ValType::Code(code);
+    }
+  }
+  MOZ_MUST_USE bool readValType(uint32_t numTypes, bool gcTypesEnabled,
+                                ValType* type) {
     static_assert(uint8_t(TypeCode::Limit) <= UINT8_MAX, "fits");
-    if (!readFixedU8(code)) {
+    uint8_t code;
+    if (!readFixedU8(&code)) {
       return false;
     }
-    if (*code == uint8_t(TypeCode::Ref)) {
-      if (!readVarU32(refTypeIndex)) {
-        return false;
+    switch (code) {
+      case uint8_t(ValType::I32):
+      case uint8_t(ValType::F32):
+      case uint8_t(ValType::F64):
+      case uint8_t(ValType::I64):
+        *type = ValType::Code(code);
+        return true;
+      case uint8_t(ValType::AnyRef):
+        if (!gcTypesEnabled) {
+          return fail("reference types not enabled");
+        }
+        *type = ValType::Code(code);
+        return true;
+      case uint8_t(ValType::Ref): {
+        if (!gcTypesEnabled) {
+          return fail("reference types not enabled");
+        }
+        uint32_t typeIndex;
+        if (!readVarU32(&typeIndex)) {
+          return false;
+        }
+        if (typeIndex >= numTypes) {
+          return fail("ref index out of range");
+        }
+        *type = ValType(ValType::Code(code), typeIndex);
+        return true;
       }
-      if (*refTypeIndex > MaxTypes) {
-        return false;
-      }
-    } else {
-      *refTypeIndex = NoRefTypeIndex;
+      default:
+        return fail("bad type");
+    }
+  }
+  MOZ_MUST_USE bool readValType(const TypeDefVector& types, bool gcTypesEnabled,
+                                ValType* type) {
+    if (!readValType(types.length(), gcTypesEnabled, type)) {
+      return false;
+    }
+    if (type->isRef() && !types[type->refTypeIndex()].isStructType()) {
+      return fail("ref does not reference a struct type");
     }
     return true;
   }
   MOZ_MUST_USE bool readBlockType(uint8_t* code, uint32_t* refTypeIndex) {
     static_assert(size_t(TypeCode::Limit) <= UINT8_MAX, "fits");
     if (!readFixedU8(code)) {
       return false;
     }
@@ -738,18 +779,17 @@ MOZ_MUST_USE bool EncodeLocalEntries(Enc
 // This performs no validation; the local entries must already have been
 // validated by an earlier pass.
 
 MOZ_MUST_USE bool DecodeValidatedLocalEntries(Decoder& d,
                                               ValTypeVector* locals);
 
 // This validates the entries.
 
-MOZ_MUST_USE bool DecodeLocalEntries(Decoder& d, ModuleKind kind,
-                                     const TypeDefVector& types,
+MOZ_MUST_USE bool DecodeLocalEntries(Decoder& d, const TypeDefVector& types,
                                      bool gcTypesEnabled,
                                      ValTypeVector* locals);
 
 // Returns whether the given [begin, end) prefix of a module's bytecode starts a
 // code section and, if so, returns the SectionRange of that code section.
 // Note that, even if this function returns 'false', [begin, end) may actually
 // be a valid module in the special case when there are no function defs and the
 // code section is not present. Such modules can be valid so the caller must