Bug 1587050 - Adjust table.copy part 2. r=jseward
authorLars T Hansen <lhansen@mozilla.com>
Fri, 11 Oct 2019 06:09:26 +0000
changeset 497264 f8479467f708eae59cd544baecf59705e8971077
parent 497263 83d82f306f89326f6a9c81702780cca0bde80f90
child 497265 f0ce1a1f84cba2d5313fa46adbc8e04edec85835
push id97762
push userarchaeopteryx@coole-files.de
push dateFri, 11 Oct 2019 07:29:48 +0000
treeherderautoland@f8479467f708 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjseward
bugs1587050
milestone71.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 1587050 - Adjust table.copy part 2. r=jseward Differential Revision: https://phabricator.services.mozilla.com/D48537
js/src/wasm/WasmInstance.cpp
js/src/wasm/WasmOpIter.h
js/src/wasm/WasmTable.cpp
js/src/wasm/WasmTable.h
--- a/js/src/wasm/WasmInstance.cpp
+++ b/js/src/wasm/WasmInstance.cpp
@@ -663,17 +663,17 @@ static int32_t PerformWait(Instance* ins
   uint32_t dstTableLen = dstTable->length();
 
   if (len == 0) {
     // Zero length copies that are out-of-bounds do not trap.
     return 0;
   }
 
   // Here, we know that |len - 1| cannot underflow.
-  bool mustTrap = false;
+  bool isOOB = false;
 
   // As we're supposed to write data until we trap we have to deal with
   // arithmetic overflow in the limit calculation.
   uint64_t highestDstOffset = uint64_t(dstOffset) + (len - 1);
   uint64_t highestSrcOffset = uint64_t(srcOffset) + (len - 1);
 
   bool copyDown = srcOffset < dstOffset;
 
@@ -687,44 +687,54 @@ static int32_t PerformWait(Instance* ins
     } else {
       // Compute what we have space for in target and what's available in the
       // source and pick the lowest value as the new len.
       uint64_t srcAvail = srcTableLen < srcOffset ? 0 : srcTableLen - srcOffset;
       uint64_t dstAvail = dstTableLen < dstOffset ? 0 : dstTableLen - dstOffset;
       MOZ_ASSERT(len > Min(srcAvail, dstAvail));
       len = uint32_t(Min(srcAvail, dstAvail));
     }
-    mustTrap = true;
+    isOOB = true;
   }
 
+  bool isOOM = false;
+
   if (len > 0) {
     // The required write direction is indicated by `copyDown`, but apart from
     // the trap that may happen without writing anything, the direction is not
     // currently observable as there are no fences nor any read/write protect
     // operation.  So Table::copy is good enough, so long as we handle
     // overlaps.
     if (&srcTable == &dstTable && dstOffset > srcOffset) {
       for (uint32_t i = len; i > 0; i--) {
-        dstTable->copy(*srcTable, dstOffset + (i - 1), srcOffset + (i - 1));
+        if (!dstTable->copy(*srcTable, dstOffset + (i - 1),
+                            srcOffset + (i - 1))) {
+          isOOM = true;
+          break;
+        }
       }
     } else if (&srcTable == &dstTable && dstOffset == srcOffset) {
       // No-op
     } else {
       for (uint32_t i = 0; i < len; i++) {
-        dstTable->copy(*srcTable, dstOffset + i, srcOffset + i);
+        if (!dstTable->copy(*srcTable, dstOffset + i, srcOffset + i)) {
+          isOOM = true;
+          break;
+        }
       }
     }
   }
 
-  if (!mustTrap) {
+  if (!isOOB && !isOOM) {
     return 0;
   }
-
-  JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
-                            JSMSG_WASM_OUT_OF_BOUNDS);
+  if (isOOB && !isOOM) {
+    JS_ReportErrorNumberASCII(TlsContext.get(), GetErrorMessage, nullptr,
+                              JSMSG_WASM_OUT_OF_BOUNDS);
+  }
   return -1;
 }
 
 /* static */ int32_t Instance::elemDrop(Instance* instance, uint32_t segIndex) {
   MOZ_ASSERT(SASigDataDrop.failureMode == FailureMode::FailOnNegI32);
 
   MOZ_RELEASE_ASSERT(size_t(segIndex) < instance->passiveElemSegments_.length(),
                      "ensured by validation");
--- a/js/src/wasm/WasmOpIter.h
+++ b/js/src/wasm/WasmOpIter.h
@@ -1974,16 +1974,21 @@ inline bool OpIter<Policy>::readMemOrTab
     if (*srcMemOrTableIndex != 0 || *dstMemOrTableIndex != 0) {
       return fail("memory index out of range for memory.copy");
     }
   } else {
     if (*dstMemOrTableIndex >= env_.tables.length() ||
         *srcMemOrTableIndex >= env_.tables.length()) {
       return fail("table index out of range for table.copy");
     }
+    ValType dstElemType = ToElemValType(env_.tables[*dstMemOrTableIndex].kind);
+    ValType srcElemType = ToElemValType(env_.tables[*srcMemOrTableIndex].kind);
+    if (!checkIsSubtypeOf(srcElemType, dstElemType)) {
+      return false;
+    }
   }
 
   if (!popWithType(ValType::I32, len)) {
     return false;
   }
 
   if (!popWithType(ValType::I32, src)) {
     return false;
--- a/js/src/wasm/WasmTable.cpp
+++ b/js/src/wasm/WasmTable.cpp
@@ -251,45 +251,64 @@ void Table::setNull(uint32_t index) {
       break;
     }
     case TableKind::AsmJS: {
       MOZ_CRASH("Should not happen");
     }
   }
 }
 
-void Table::copy(const Table& srcTable, uint32_t dstIndex, uint32_t srcIndex) {
+bool Table::copy(const Table& srcTable, uint32_t dstIndex, uint32_t srcIndex) {
+  MOZ_RELEASE_ASSERT(srcTable.kind() != TableKind::AsmJS);
   switch (kind_) {
     case TableKind::FuncRef: {
-      FunctionTableElem& dst = functions_[dstIndex];
-      if (dst.tls) {
-        JSObject::writeBarrierPre(dst.tls->instance->objectUnbarriered());
-      }
+      if (srcTable.kind() == TableKind::FuncRef) {
+        FunctionTableElem& dst = functions_[dstIndex];
+        if (dst.tls) {
+          JSObject::writeBarrierPre(dst.tls->instance->objectUnbarriered());
+        }
+
+        FunctionTableElem& src = srcTable.functions_[srcIndex];
+        dst.code = src.code;
+        dst.tls = src.tls;
 
-      FunctionTableElem& src = srcTable.functions_[srcIndex];
-      dst.code = src.code;
-      dst.tls = src.tls;
-
-      if (dst.tls) {
-        MOZ_ASSERT(dst.code);
-        MOZ_ASSERT(dst.tls->instance->objectUnbarriered()->isTenured(),
-                   "no writeBarrierPost (Table::copy)");
+        if (dst.tls) {
+          MOZ_ASSERT(dst.code);
+          MOZ_ASSERT(dst.tls->instance->objectUnbarriered()->isTenured(),
+                     "no writeBarrierPost (Table::copy)");
+        } else {
+          MOZ_ASSERT(!dst.code);
+        }
       } else {
-        MOZ_ASSERT(!dst.code);
+        // Downcast should not happen.
+        MOZ_CRASH("NYI");
       }
       break;
     }
     case TableKind::AnyRef: {
-      fillAnyRef(dstIndex, 1, srcTable.getAnyRef(srcIndex));
+      if (srcTable.kind() == TableKind::AnyRef) {
+        fillAnyRef(dstIndex, 1, srcTable.getAnyRef(srcIndex));
+      } else {
+        // Upcast. Possibly suboptimal to grab the cx here for every iteration
+        // of the outer copy loop.
+        JSContext* cx = TlsContext.get();
+        RootedFunction fun(cx);
+        if (!srcTable.getFuncRef(cx, srcIndex, &fun)) {
+          // OOM, so just pass it on.
+          return false;
+        }
+        fillAnyRef(dstIndex, 1, AnyRef::fromJSObject(fun));
+      }
       break;
     }
     case TableKind::AsmJS: {
       MOZ_CRASH("Bad table type");
     }
   }
+  return true;
 }
 
 uint32_t Table::grow(uint32_t delta) {
   // This isn't just an optimization: movingGrowable() assumes that
   // onMovingGrowTable does not fire when length == maximum.
   if (!delta) {
     return length_;
   }
--- a/js/src/wasm/WasmTable.h
+++ b/js/src/wasm/WasmTable.h
@@ -91,19 +91,19 @@ class Table : public ShareableBase<Table
   void fillFuncRef(uint32_t index, uint32_t fillCount, AnyRef ref,
                    JSContext* cx);
 
   AnyRef getAnyRef(uint32_t index) const;
   void fillAnyRef(uint32_t index, uint32_t fillCount, AnyRef ref);
 
   void setNull(uint32_t index);
 
-  // Copy entry from |srcTable| at |srcIndex| to this table at |dstIndex|.
-  // Used by table.copy.
-  void copy(const Table& srcTable, uint32_t dstIndex, uint32_t srcIndex);
+  // Copy entry from |srcTable| at |srcIndex| to this table at |dstIndex|.  Used
+  // by table.copy.  May OOM if it needs to box up a function during an upcast.
+  bool copy(const Table& srcTable, uint32_t dstIndex, uint32_t srcIndex);
 
   // grow() returns (uint32_t)-1 if it could not grow.
   uint32_t grow(uint32_t delta);
   bool movingGrowable() const;
   bool addMovingGrowObserver(JSContext* cx, WasmInstanceObject* instance);
 
   // about:memory reporting: