Bug 1529298 - Implement ArrayBufferObject::prepareForAsmJS using a switch, not a series of ifs that's less obviously exhaustive. r=sfink
authorJeff Walden <jwalden@mit.edu>
Thu, 21 Feb 2019 17:58:46 -0800
changeset 461629 bb879a6a95f89fc3319899d3267c77a071d88076
parent 461628 9103748036d1d9f72d34474014c9ce0267271eb5
child 461630 afbcec667ac42d4a17303ad65bd499a29e43c403
push id35626
push usercsabou@mozilla.com
push dateThu, 28 Feb 2019 11:31:08 +0000
treeherdermozilla-central@2ea0c1db7e60 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1529298
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 1529298 - Implement ArrayBufferObject::prepareForAsmJS using a switch, not a series of ifs that's less obviously exhaustive. r=sfink
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/wasm/AsmJS.cpp
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -858,60 +858,60 @@ bool js::CreateWasmBuffer(JSContext* cx,
     }
     return CreateBuffer<SharedArrayBufferObject, SharedArrayRawBuffer>(
         cx, memory.initial, maxSize, buffer);
   }
   return CreateBuffer<ArrayBufferObject, WasmArrayRawBuffer>(cx, memory.initial,
                                                              maxSize, buffer);
 }
 
-// Note this function can return false with or without an exception pending. The
-// asm.js caller checks cx->isExceptionPending before propagating failure.
-// Returning false without throwing means that asm.js linking will fail which
-// will recompile as non-asm.js.
-/* static */ bool ArrayBufferObject::prepareForAsmJS(
-    JSContext* cx, Handle<ArrayBufferObject*> buffer) {
-  MOZ_ASSERT(buffer->byteLength() % wasm::PageSize == 0,
+bool ArrayBufferObject::prepareForAsmJS() {
+  MOZ_ASSERT(byteLength() % wasm::PageSize == 0,
              "prior size checking should have guaranteed page-size multiple");
-  MOZ_ASSERT(buffer->byteLength() > 0,
+  MOZ_ASSERT(byteLength() > 0,
              "prior size checking should have excluded empty buffers");
-  MOZ_ASSERT(!buffer->isNoData(),
-             "size checking should have excluded detached or empty buffers");
+
+  switch (bufferKind()) {
+    case MALLOCED:
+    case MAPPED:
+    case EXTERNAL:
+      // It's okay if this uselessly sets the flag a second time.
+      setIsPreparedForAsmJS();
+      return true;
 
-  static_assert(wasm::PageSize > MaxInlineBytes,
-                "inline data must be too small to be a page size multiple");
-  MOZ_ASSERT(!buffer->isInlineData(),
-             "inline-data buffers are implicitly excluded by size checks");
+    case INLINE_DATA:
+      static_assert(wasm::PageSize > MaxInlineBytes,
+                    "inline data must be too small to be a page size multiple");
+      MOZ_ASSERT_UNREACHABLE(
+          "inline-data buffers should be implicitly excluded by size checks");
+      return false;
+
+    case NO_DATA:
+      MOZ_ASSERT_UNREACHABLE(
+          "size checking should have excluded detached or empty buffers");
+      return false;
 
-  // Don't assert cx->wasmHaveSignalHandlers because (1) they aren't needed
-  // for asm.js, (2) they are only installed for WebAssembly, not asm.js.
+    // asm.js code and associated buffers are potentially long-lived.  Yet a
+    // buffer of user-owned data *must* be detached by the user before the
+    // user-owned data is disposed.  No caller wants to use a user-owned
+    // ArrayBuffer with asm.js, so just don't support this and avoid a mess of
+    // complexity.
+    case USER_OWNED:
+    // wasm buffers can be detached at any time.
+    case WASM:
+      MOZ_ASSERT(!isPreparedForAsmJS());
+      return false;
 
-  // wasm buffers can be detached at any time.
-  if (buffer->isWasm()) {
-    MOZ_ASSERT(!buffer->isPreparedForAsmJS());
-    return false;
+    case BAD1:
+      MOZ_ASSERT_UNREACHABLE("invalid bufferKind() encountered");
+      return false;
   }
 
-  // asm.js code and associated buffers are potentially long-lived.  Yet if
-  // |buffer->hasUserOwnedData()|, |buffer| *must* be detached by the user
-  // before the user-provided data is disposed.  Eliminate the complexity of
-  // this edge case by not allowing buffers with user-provided content to be
-  // used with asm.js, as no callers exist that want to use such buffer with
-  // asm.js.
-  if (buffer->hasUserOwnedData()) {
-    MOZ_ASSERT(!buffer->isPreparedForAsmJS());
-    return false;
-  }
-
-  MOZ_ASSERT(buffer->isMalloced() || buffer->isMapped() ||
-             buffer->isExternal());
-
-  // It's fine if this uselessly sets the flag a second time.
-  buffer->setIsPreparedForAsmJS();
-  return true;
+  MOZ_ASSERT_UNREACHABLE("non-exhaustive kind-handling switch?");
+  return false;
 }
 
 ArrayBufferObject::BufferContents ArrayBufferObject::createMappedContents(
     int fd, size_t offset, size_t length) {
   void* data =
       gc::AllocateMappedContent(fd, offset, length, ARRAY_BUFFER_ALIGNMENT);
   return BufferContents::createMapped(data);
 }
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -401,18 +401,23 @@ class ArrayBufferObject : public ArrayBu
   bool isWasm() const { return bufferKind() == WASM; }
   bool isMapped() const { return bufferKind() == MAPPED; }
   bool isExternal() const { return bufferKind() == EXTERNAL; }
 
   bool isDetached() const { return flags() & DETACHED; }
   bool isPreparedForAsmJS() const { return flags() & FOR_ASMJS; }
 
   // WebAssembly support:
-  static MOZ_MUST_USE bool prepareForAsmJS(JSContext* cx,
-                                           Handle<ArrayBufferObject*> buffer);
+
+  /**
+   * Prepare this ArrayBuffer for use with asm.js.  Returns true on success,
+   * false on failure.  This function reports no errors.
+   */
+  MOZ_MUST_USE bool prepareForAsmJS();
+
   size_t wasmMappedSize() const;
   mozilla::Maybe<uint32_t> wasmMaxSize() const;
   static MOZ_MUST_USE bool wasmGrowToSizeInPlace(
       uint32_t newSize, Handle<ArrayBufferObject*> oldBuf,
       MutableHandle<ArrayBufferObject*> newBuf, JSContext* cx);
 #ifndef WASM_HUGE_MEMORY
   static MOZ_MUST_USE bool wasmMovingGrowToSize(
       uint32_t newSize, Handle<ArrayBufferObject*> oldBuf,
--- a/js/src/wasm/AsmJS.cpp
+++ b/js/src/wasm/AsmJS.cpp
@@ -6748,17 +6748,17 @@ static bool CheckBuffer(JSContext* cx, c
       return false;
     }
     return LinkFail(cx, msg.get());
   }
 
   if (buffer->is<ArrayBufferObject>()) {
     Rooted<ArrayBufferObject*> arrayBuffer(cx,
                                            &buffer->as<ArrayBufferObject>());
-    if (!ArrayBufferObject::prepareForAsmJS(cx, arrayBuffer)) {
+    if (!arrayBuffer->prepareForAsmJS()) {
       return LinkFail(cx, "Unable to prepare ArrayBuffer for asm.js use");
     }
   } else {
     return LinkFail(cx, "Unable to prepare SharedArrayBuffer for asm.js use");
   }
 
   MOZ_ASSERT(buffer->isPreparedForAsmJS());
   return true;