Bug 1603496 - abstract reference-type checking on JS values. r=rhunt
authorLars T Hansen <lhansen@mozilla.com>
Mon, 16 Dec 2019 08:59:09 +0000
changeset 507056 f23128d93320d080700b559277b8a034744adb8b
parent 507055 572a5cc1d0b5779c434dda5cba78a8ad59304eff
child 507057 1054b841f0fbb1358c00089a592fd00bbc381809
push id36922
push userncsoregi@mozilla.com
push dateMon, 16 Dec 2019 17:21:47 +0000
treeherdermozilla-central@27d0d6cc2131 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhunt
bugs1603496
milestone73.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 1603496 - abstract reference-type checking on JS values. r=rhunt Lift the code that checks JS values against reference types (and boxes anyref) from all the places where it is repeated into a common function. Clean up some code that assumes that the only reference types are anyref and funcref. Differential Revision: https://phabricator.services.mozilla.com/D56971
js/src/wasm/WasmBaselineCompile.cpp
js/src/wasm/WasmInstance.cpp
js/src/wasm/WasmJS.cpp
js/src/wasm/WasmJS.h
js/src/wasm/WasmTypes.h
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -11482,19 +11482,24 @@ bool BaseCompiler::emitStructNarrow() {
   if (!iter_.readStructNarrow(&inputType, &outputType, &nothing)) {
     return false;
   }
 
   if (deadCode_) {
     return true;
   }
 
-  // Currently not supported by struct.narrow validation.
-  MOZ_ASSERT(inputType != RefType::func());
-  MOZ_ASSERT(outputType != RefType::func());
+  // struct.narrow validation ensures that these hold.
+
+  MOZ_ASSERT(inputType.refTypeKind() == RefType::Any ||
+             inputType.refTypeKind() == RefType::TypeIndex);
+  MOZ_ASSERT(outputType.refTypeKind() == RefType::Any ||
+             outputType.refTypeKind() == RefType::TypeIndex);
+  MOZ_ASSERT_IF(outputType.refTypeKind() == RefType::Any,
+                inputType.refTypeKind() == RefType::Any);
 
   // AnyRef -> AnyRef is a no-op, just leave the value on the stack.
 
   if (inputType == RefType::any() && outputType == RefType::any()) {
     return true;
   }
 
   RegPtr rp = popRef();
--- a/js/src/wasm/WasmInstance.cpp
+++ b/js/src/wasm/WasmInstance.cpp
@@ -1744,64 +1744,40 @@ bool Instance::callExport(JSContext* cx,
       case ValType::F64:
         if (!ToNumber(cx, v, (double*)&exportArgs[i])) {
           return false;
         }
         DebugCodegen(DebugChannel::Function, "f64(%lf) ",
                      *(double*)&exportArgs[i]);
         break;
       case ValType::Ref:
+        RootedFunction fun(cx);
+        RootedAnyRef any(cx, AnyRef::null());
+        if (!CheckRefType(cx, funcType->arg(i).refTypeKind(), v, &fun, &any)) {
+          return false;
+        }
+        ASSERT_ANYREF_IS_JSOBJECT;
+        // Store in rooted array until no more GC is possible.
         switch (funcType->arg(i).refTypeKind()) {
-          case RefType::Func: {
-            RootedFunction fun(cx);
-            if (!CheckFuncRefValue(cx, v, &fun)) {
-              return false;
-            }
-            // Store in rooted array until no more GC is possible.
-            ASSERT_ANYREF_IS_JSOBJECT;
+          case RefType::Func:
             if (!refs.emplaceBack(fun)) {
               return false;
             }
-            DebugCodegen(DebugChannel::Function, "ptr(#%d) ",
-                         int(refs.length() - 1));
             break;
-          }
-          case RefType::Null: {
-            if (!v.isNull()) {
-              JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
-                                       JSMSG_WASM_NULL_REQUIRED);
-              return false;
-            }
-            // Store in rooted array until no more GC is possible.
-            ASSERT_ANYREF_IS_JSOBJECT;
-            if (!refs.emplaceBack(AnyRef::null().asJSObject())) {
+          case RefType::Null:
+          case RefType::Any:
+            if (!refs.emplaceBack(any.get().asJSObject())) {
               return false;
             }
-            DebugCodegen(DebugChannel::Function, "nullptr(#%d) ",
-                         int(refs.length() - 1));
             break;
-          }
-          case RefType::Any: {
-            RootedAnyRef ar(cx, AnyRef::null());
-            if (!BoxAnyRef(cx, v, &ar)) {
-              return false;
-            }
-            // Store in rooted array until no more GC is possible.
-            ASSERT_ANYREF_IS_JSOBJECT;
-            if (!refs.emplaceBack(ar.get().asJSObject())) {
-              return false;
-            }
-            DebugCodegen(DebugChannel::Function, "ptr(#%d) ",
-                         int(refs.length() - 1));
-            break;
-          }
-          case RefType::TypeIndex: {
+          case RefType::TypeIndex:
             MOZ_CRASH("temporarily unsupported Ref type in callExport");
-          }
         }
+        DebugCodegen(DebugChannel::Function, "ref(#%d) ",
+                     int(refs.length() - 1));
         break;
     }
   }
 
   DebugCodegen(DebugChannel::Function, "\n");
 
   // Copy over reference values from the rooted array, if any.
   if (refs.length() > 0) {
--- a/js/src/wasm/WasmJS.cpp
+++ b/js/src/wasm/WasmJS.cpp
@@ -185,16 +185,44 @@ bool wasm::HasStreamingSupport(JSContext
          CanUseExtraThreads() && cx->runtime()->consumeStreamCallback &&
          cx->runtime()->reportStreamErrorCallback;
 }
 
 bool wasm::HasCachingSupport(JSContext* cx) {
   return HasStreamingSupport(cx) && HasOptimizedCompilerTier(cx);
 }
 
+bool wasm::CheckRefType(JSContext* cx, RefType::Kind targetTypeKind,
+                        HandleValue v, MutableHandleFunction fnval,
+                        MutableHandleAnyRef refval) {
+  switch (targetTypeKind) {
+    case RefType::Func:
+      if (!CheckFuncRefValue(cx, v, fnval)) {
+        return false;
+      }
+      break;
+    case RefType::Null:
+      if (!v.isNull()) {
+        JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
+                                 JSMSG_WASM_NULL_REQUIRED);
+        return false;
+      }
+      refval.set(AnyRef::null());
+      break;
+    case RefType::Any:
+      if (!BoxAnyRef(cx, v, refval)) {
+        return false;
+      }
+      break;
+    case RefType::TypeIndex:
+      MOZ_CRASH("temporarily unsupported Ref type");
+  }
+  return true;
+}
+
 static bool ToWebAssemblyValue(JSContext* cx, ValType targetType, HandleValue v,
                                MutableHandleVal val) {
   switch (targetType.kind()) {
     case ValType::I32: {
       int32_t i32;
       if (!ToInt32(cx, v, &i32)) {
         return false;
       }
@@ -226,45 +254,31 @@ static bool ToWebAssemblyValue(JSContext
         }
         val.set(Val(BigInt::toUint64(bigint)));
         return true;
       }
 #endif
       break;
     }
     case ValType::Ref: {
+      RootedFunction fun(cx);
+      RootedAnyRef any(cx, AnyRef::null());
+      if (!CheckRefType(cx, targetType.refTypeKind(), v, &fun, &any)) {
+        return false;
+      }
       switch (targetType.refTypeKind()) {
-        case RefType::Func: {
-          RootedFunction fun(cx);
-          if (!CheckFuncRefValue(cx, v, &fun)) {
-            return false;
-          }
+        case RefType::Func:
           val.set(Val(RefType::func(), FuncRef::fromJSFunction(fun)));
           return true;
-        }
-        case RefType::Null: {
-          if (!v.isNull()) {
-            JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
-                                     JSMSG_WASM_NULL_REQUIRED);
-            return false;
-          }
-          val.set(Val(RefType::null(), AnyRef::null()));
+        case RefType::Any:
+        case RefType::Null:
+          val.set(Val(targetType.refType(), any));
           return true;
-        }
-        case RefType::Any: {
-          RootedAnyRef tmp(cx, AnyRef::null());
-          if (!BoxAnyRef(cx, v, &tmp)) {
-            return false;
-          }
-          val.set(Val(RefType::any(), tmp));
-          return true;
-        }
-        case RefType::TypeIndex: {
+        case RefType::TypeIndex:
           break;
-        }
       }
       break;
     }
   }
   MOZ_CRASH("unexpected import value type, caller must guard");
 }
 
 static bool ToJSValue(JSContext* cx, const Val& val, MutableHandleValue out) {
@@ -2373,49 +2387,36 @@ bool WasmTableObject::setImpl(JSContext*
     return false;
   }
 
   uint32_t index;
   if (!ToTableIndex(cx, args.get(0), table, "set index", &index)) {
     return false;
   }
 
+  MOZ_ASSERT(index < MaxTableLength);
+  static_assert(MaxTableLength < UINT32_MAX, "Invariant");
+
   RootedValue fillValue(cx, args[1]);
+  RootedFunction fun(cx);
+  RootedAnyRef any(cx, AnyRef::null());
+  if (!CheckRefType(cx, ToElemValType(table.kind()).refTypeKind(), fillValue,
+                    &fun, &any)) {
+    return false;
+  }
   switch (table.kind()) {
-    case TableKind::AsmJS: {
+    case TableKind::AsmJS:
       MOZ_CRASH("Should not happen");
-    }
-    case TableKind::FuncRef: {
-      RootedFunction fun(cx);
-      if (!CheckFuncRefValue(cx, fillValue, &fun)) {
-        return false;
-      }
-      MOZ_ASSERT(index < MaxTableLength);
-      static_assert(MaxTableLength < UINT32_MAX, "Invariant");
+    case TableKind::FuncRef:
       table.fillFuncRef(index, 1, FuncRef::fromJSFunction(fun), cx);
       break;
-    }
-    case TableKind::NullRef: {
-      if (!fillValue.isNull()) {
-        JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
-                                 JSMSG_WASM_NULL_REQUIRED);
-        return false;
-      }
-      RootedAnyRef tmp(cx, AnyRef::null());
-      table.fillAnyRef(index, 1, tmp);
+    case TableKind::NullRef:
+    case TableKind::AnyRef:
+      table.fillAnyRef(index, 1, any);
       break;
-    }
-    case TableKind::AnyRef: {
-      RootedAnyRef tmp(cx, AnyRef::null());
-      if (!BoxAnyRef(cx, fillValue, &tmp)) {
-        return false;
-      }
-      table.fillAnyRef(index, 1, tmp);
-      break;
-    }
   }
 
   args.rval().setUndefined();
   return true;
 }
 
 /* static */
 bool WasmTableObject::set(JSContext* cx, unsigned argc, Value* vp) {
@@ -2452,67 +2453,50 @@ bool WasmTableObject::growImpl(JSContext
     fillValue = args[1];
   }
 
   MOZ_ASSERT(delta <= MaxTableLength);              // grow() should ensure this
   MOZ_ASSERT(oldLength <= MaxTableLength - delta);  // ditto
 
   static_assert(MaxTableLength < UINT32_MAX, "Invariant");
 
-  switch (table.kind()) {
-    case TableKind::AsmJS: {
-      MOZ_CRASH("asm.js not supported");
+  if (!fillValue.isNull()) {
+    RootedFunction fun(cx);
+    RootedAnyRef any(cx, AnyRef::null());
+    if (!CheckRefType(cx, ToElemValType(table.kind()).refTypeKind(), fillValue,
+                      &fun, &any)) {
+      return false;
     }
-    case TableKind::FuncRef: {
-      if (fillValue.isNull()) {
+    switch (table.repr()) {
+      case TableRepr::Func:
+        MOZ_ASSERT(table.kind() == TableKind::FuncRef);
+        table.fillFuncRef(oldLength, delta, FuncRef::fromJSFunction(fun), cx);
+        break;
+      case TableRepr::Ref:
+        table.fillAnyRef(oldLength, delta, any);
+        break;
+    }
+  }
+
 #ifdef DEBUG
+  if (fillValue.isNull()) {
+    switch (table.repr()) {
+      case TableRepr::Func:
         for (uint32_t index = oldLength; index < oldLength + delta; index++) {
           MOZ_ASSERT(table.getFuncRef(index).code == nullptr);
         }
-#endif
-      } else {
-        RootedFunction fun(cx);
-        if (!CheckFuncRefValue(cx, fillValue, &fun)) {
-          return false;
-        }
-        table.fillFuncRef(oldLength, delta, FuncRef::fromJSFunction(fun), cx);
-      }
-      break;
-    }
-    case TableKind::AnyRef: {
-      RootedAnyRef tmp(cx, AnyRef::null());
-      if (!BoxAnyRef(cx, fillValue, &tmp)) {
-        return false;
-      }
-      if (!tmp.get().isNull()) {
-        table.fillAnyRef(oldLength, delta, tmp);
-      } else {
-#ifdef DEBUG
+        break;
+      case TableRepr::Ref:
         for (uint32_t index = oldLength; index < oldLength + delta; index++) {
           MOZ_ASSERT(table.getAnyRef(index).isNull());
         }
-#endif
-      }
-      break;
+        break;
     }
-    case TableKind::NullRef: {
-      if (fillValue.isNull()) {
-#ifdef DEBUG
-        for (uint32_t index = oldLength; index < oldLength + delta; index++) {
-          MOZ_ASSERT(table.getAnyRef(index).isNull());
-        }
+  }
 #endif
-      } else {
-        JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
-                                 JSMSG_WASM_NULL_REQUIRED);
-        return false;
-      }
-      break;
-    }
-  }
 
   args.rval().setInt32(oldLength);
   return true;
 }
 
 /* static */
 bool WasmTableObject::grow(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
--- a/js/src/wasm/WasmJS.h
+++ b/js/src/wasm/WasmJS.h
@@ -108,24 +108,37 @@ MOZ_MUST_USE bool DeserializeModule(JSCo
 // A WebAssembly "Exported Function" is the spec name for the JS function
 // objects created to wrap wasm functions. This predicate returns false
 // for asm.js functions which are semantically just normal JS functions
 // (even if they are implemented via wasm under the hood). The accessor
 // functions for extracting the instance and func-index of a wasm function
 // can be used for both wasm and asm.js, however.
 
 bool IsWasmExportedFunction(JSFunction* fun);
-bool CheckFuncRefValue(JSContext* cx, HandleValue v, MutableHandleFunction fun);
+MOZ_MUST_USE bool CheckFuncRefValue(JSContext* cx, HandleValue v,
+                                    MutableHandleFunction fun);
 
 Instance& ExportedFunctionToInstance(JSFunction* fun);
 WasmInstanceObject* ExportedFunctionToInstanceObject(JSFunction* fun);
 uint32_t ExportedFunctionToFuncIndex(JSFunction* fun);
 
 bool IsSharedWasmMemoryObject(JSObject* obj);
 
+// Check a value against the given reference type kind.  If the targetTypeKind
+// is RefType::Any then the test always passes, but the value may be boxed.  If
+// the test passes then the value is stored either in fnval (for RefType::Func)
+// or in refval (for other types); this split is not strictly necessary but is
+// convenient for the users of this function.
+//
+// This can return false if the type check fails, or if a boxing into AnyRef
+// throws an OOM.
+MOZ_MUST_USE bool CheckRefType(JSContext* cx, RefType::Kind targetTypeKind,
+                               HandleValue v, MutableHandleFunction fnval,
+                               MutableHandleAnyRef refval);
+
 }  // namespace wasm
 
 // The class of the WebAssembly global namespace object.
 
 extern const JSClass WebAssemblyClass;
 
 // The class of WebAssembly.Module. Each WasmModuleObject owns a
 // wasm::Module. These objects are used both as content-facing JS objects and as
--- a/js/src/wasm/WasmTypes.h
+++ b/js/src/wasm/WasmTypes.h
@@ -199,17 +199,18 @@ struct ShareableBytes : ShareableBase<Sh
   }
 };
 
 typedef RefPtr<ShareableBytes> MutableBytes;
 typedef RefPtr<const ShareableBytes> SharedBytes;
 
 // A PackedTypeCode represents a TypeCode paired with a refTypeIndex (valid only
 // for TypeCode::Ref).  PackedTypeCode is guaranteed to be POD.  The TypeCode
-// spans the full range of type codes including the specialized AnyRef, FuncRef.
+// spans the full range of type codes including the specialized AnyRef, FuncRef,
+// NullRef.
 //
 // 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 {};
 
@@ -745,17 +746,17 @@ class FuncRef {
   JSFunction* value_;
 
   explicit FuncRef() : value_((JSFunction*)-1) {}
   explicit FuncRef(JSFunction* p) : value_(p) {
     MOZ_ASSERT(((uintptr_t)p & 0x03) == 0);
   }
 
  public:
-  // Given a void* that comes from compiled wasm code, turn it into AnyRef.
+  // Given a void* that comes from compiled wasm code, turn it into FuncRef.
   static FuncRef fromCompiledCode(void* p) { return FuncRef((JSFunction*)p); }
 
   // Given a JSFunction* that comes from JS, turn it into FuncRef.
   static FuncRef fromJSFunction(JSFunction* p) { return FuncRef(p); }
 
   // Given an AnyRef that represents a possibly-null funcref, turn it into a
   // FuncRef.
   static FuncRef fromAnyRefUnchecked(AnyRef p) {
@@ -1011,53 +1012,50 @@ class FuncType {
     for (ValType result : results()) {
       if (result == ValType::I64) {
         return true;
       }
     }
     return false;
   }
   // For JS->wasm jit entries, AnyRef parameters and returns are allowed,
-  // as are FuncRef and nullref returns.
+  // as are all reference types apart from TypeIndex.
   bool temporarilyUnsupportedReftypeForEntry() const {
     for (ValType arg : args()) {
       if (arg.isReference() && arg != RefType::any()) {
         return true;
       }
     }
     for (ValType result : results()) {
-      if (result.isReference() && result != RefType::any() &&
-          result != RefType::func() && result != RefType::null()) {
+      if (result.isRef()) {
         return true;
       }
     }
     return false;
   }
   // For inlined JS->wasm jit entries, AnyRef parameters and returns are
-  // allowed, as are FuncRef and nullref returns.
+  // allowed, as are all reference types apart from TypeIndex.
   bool temporarilyUnsupportedReftypeForInlineEntry() const {
     for (ValType arg : args()) {
       if (arg.isReference() && arg != RefType::any()) {
         return true;
       }
     }
     for (ValType result : results()) {
-      if (result.isReference() && result != RefType::any() &&
-          result != RefType::func() && result != RefType::null()) {
+      if (result.isRef()) {
         return true;
       }
     }
     return false;
   }
   // For wasm->JS jit exits, AnyRef parameters and returns are allowed, as are
-  // FuncRef and nullref parameters.
+  // reference type parameters of all types except TypeIndex.
   bool temporarilyUnsupportedReftypeForExit() const {
     for (ValType arg : args()) {
-      if (arg.isReference() && arg != RefType::any() &&
-          arg != RefType::func() && arg != RefType::null()) {
+      if (arg.isRef()) {
         return true;
       }
     }
     for (ValType result : results()) {
       if (result.isReference() && result != RefType::any()) {
         return true;
       }
     }
@@ -2226,21 +2224,22 @@ struct Limits {
   explicit Limits(uint32_t initial, const Maybe<uint32_t>& maximum = Nothing(),
                   Shareable shared = Shareable::False)
       : initial(initial), maximum(maximum), shared(shared) {}
 };
 
 // TableDesc describes a table as well as the offset of the table's base pointer
 // in global memory. The TableKind determines the representation:
 //  - AnyRef: a wasm anyref word (wasm::AnyRef)
+//  - NullRef: as AnyRef
 //  - FuncRef: a two-word FunctionTableElem (wasm indirect call ABI)
 //  - AsmJS: a two-word FunctionTableElem (asm.js ABI)
 // Eventually there should be a single unified AnyRef representation.
 
-enum class TableKind { AnyRef, FuncRef, NullRef, AsmJS };
+enum class TableKind { AnyRef, NullRef, FuncRef, AsmJS };
 
 static inline ValType ToElemValType(TableKind tk) {
   switch (tk) {
     case TableKind::AnyRef:
       return RefType::any();
     case TableKind::NullRef:
       return RefType::null();
     case TableKind::FuncRef: