Bug 1529298 - Rename the two ArrayBufferObject::create overloads to ArrayBufferObject::create{Zeroed,WithContents}, and inline a simplified form of the more-complex ArrayBufferObject::create into the new createZeroed function. r=sfink
authorJeff Walden <jwalden@mit.edu>
Mon, 18 Feb 2019 22:52:41 -0800
changeset 460384 765bc7a86f726c7fd3633ca29026cc10f4950864
parent 460383 5a304d4b53c05587d9da2fdb4db5276380c6ff5b
child 460385 92fc3d9b6699885b0af2feb47b9577640d6d2898
push id112085
push userjwalden@mit.edu
push dateFri, 22 Feb 2019 04:41:17 +0000
treeherdermozilla-inbound@5b7a07b449ba [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 - Rename the two ArrayBufferObject::create overloads to ArrayBufferObject::create{Zeroed,WithContents}, and inline a simplified form of the more-complex ArrayBufferObject::create into the new createZeroed function. r=sfink
js/src/builtin/TypedObject.cpp
js/src/shell/js.cpp
js/src/vm/ArrayBufferObject.cpp
js/src/vm/ArrayBufferObject.h
js/src/vm/StructuredClone.cpp
js/src/vm/TypedArrayObject.cpp
js/src/wasm/WasmJS.cpp
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -1646,17 +1646,17 @@ void OutlineTypedObject::attach(JSContex
       cx, OutlineTypedObject::createUnattached(cx, descr, heap));
   if (!obj) {
     return nullptr;
   }
 
   // Allocate and initialize the memory for this instance.
   size_t totalSize = descr->size();
   Rooted<ArrayBufferObject*> buffer(cx);
-  buffer = ArrayBufferObject::create(cx, totalSize);
+  buffer = ArrayBufferObject::createZeroed(cx, totalSize);
   if (!buffer) {
     return nullptr;
   }
   descr->initInstances(cx->runtime(), buffer->dataPointer(), 1);
   obj->attach(cx, *buffer, 0);
   return obj;
 }
 
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -1915,17 +1915,17 @@ static uint8_t* CacheEntry_getBytecode(J
 static bool CacheEntry_setBytecode(JSContext* cx, HandleObject cache,
                                    uint8_t* buffer, uint32_t length) {
   MOZ_ASSERT(CacheEntry_isCacheEntry(cache));
 
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createMalloced(buffer);
   Rooted<ArrayBufferObject*> arrayBuffer(
-      cx, ArrayBufferObject::create(cx, length, contents));
+      cx, ArrayBufferObject::createForContents(cx, length, contents));
   if (!arrayBuffer) {
     return false;
   }
 
   SetReservedSlot(cache, CacheEntry_BYTECODE, ObjectValue(*arrayBuffer));
   return true;
 }
 
@@ -6976,17 +6976,17 @@ class StreamCacheEntryObject : public Na
     if (!args.thisv().isObject() ||
         !args.thisv().toObject().is<StreamCacheEntryObject>()) {
       return false;
     }
 
     auto& bytes =
         args.thisv().toObject().as<StreamCacheEntryObject>().cache().bytes();
     RootedArrayBufferObject buffer(
-        cx, ArrayBufferObject::create(cx, bytes.length()));
+        cx, ArrayBufferObject::createZeroed(cx, bytes.length()));
     if (!buffer) {
       return false;
     }
 
     memcpy(buffer->dataPointer(), bytes.begin(), bytes.length());
 
     args.rval().setObject(*buffer);
     return true;
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "vm/ArrayBufferObject-inl.h"
 #include "vm/ArrayBufferObject.h"
 
 #include "mozilla/Alignment.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/FloatingPoint.h"
+#include "mozilla/Likely.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/PodOperations.h"
 #include "mozilla/TaggedAnonymousMemory.h"
 
 #include <string.h>
 #ifndef XP_WIN
 #  include <sys/mman.h>
 #endif
@@ -428,17 +429,17 @@ bool ArrayBufferObject::class_constructo
   // Refuse to allocate too large buffers, currently limited to ~2 GiB.
   if (byteLength > INT32_MAX) {
     JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
                               JSMSG_BAD_ARRAY_LENGTH);
     return false;
   }
 
   // 24.1.1.1, steps 1 and 4-6.
-  JSObject* bufobj = create(cx, uint32_t(byteLength), proto);
+  JSObject* bufobj = createZeroed(cx, uint32_t(byteLength), proto);
   if (!bufobj) {
     return false;
   }
   args.rval().setObject(*bufobj);
   return true;
 }
 
 static ArrayBufferObject::BufferContents AllocateArrayBufferContents(
@@ -1180,27 +1181,36 @@ uint32_t ArrayBufferObjectMaybeShared::w
 uint32_t ArrayBufferObject::flags() const {
   return uint32_t(getFixedSlot(FLAGS_SLOT).toInt32());
 }
 
 void ArrayBufferObject::setFlags(uint32_t flags) {
   setFixedSlot(FLAGS_SLOT, Int32Value(flags));
 }
 
-ArrayBufferObject* ArrayBufferObject::create(
+static MOZ_MUST_USE bool CheckArrayBufferTooLarge(JSContext* cx,
+                                                  uint32_t nbytes) {
+  // Refuse to allocate too large buffers, currently limited to ~2 GiB.
+  if (MOZ_UNLIKELY(nbytes > ArrayBufferObject::MaxBufferByteLength)) {
+    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                              JSMSG_BAD_ARRAY_LENGTH);
+    return false;
+  }
+
+  return true;
+}
+
+ArrayBufferObject* ArrayBufferObject::createForContents(
     JSContext* cx, uint32_t nbytes, BufferContents contents,
     OwnsState ownsState /* = OwnsData */, HandleObject proto /* = nullptr */,
     NewObjectKind newKind /* = GenericObject */) {
   MOZ_ASSERT_IF(contents.kind() == MAPPED, contents);
 
   // 24.1.1.1, step 3 (Inlined 6.2.6.1 CreateByteDataBlock, step 2).
-  // Refuse to allocate too large buffers, currently limited to ~2 GiB.
-  if (nbytes > INT32_MAX) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_BAD_ARRAY_LENGTH);
+  if (!CheckArrayBufferTooLarge(cx, nbytes)) {
     return nullptr;
   }
 
   // If we need to allocate data, try to use a larger object size class so
   // that the array buffer's data can be allocated inline with the object.
   // The extra space will be left unused by the object's fixed slots and
   // available for the buffer's data, see NewObject().
   size_t reservedSlots = JSCLASS_RESERVED_SLOTS(&class_);
@@ -1253,38 +1263,87 @@ ArrayBufferObject* ArrayBufferObject::cr
   if (!obj) {
     if (allocated) {
       js_free(contents.data());
     }
     return nullptr;
   }
 
   MOZ_ASSERT(obj->getClass() == &class_);
-  MOZ_ASSERT(!gc::IsInsideNursery(obj));
+  MOZ_ASSERT(!gc::IsInsideNursery(obj),
+             "ArrayBufferObject has a finalizer that must be called to not "
+             "leak in some cases, so it can't be nursery-allocated");
 
   if (!contents) {
     MOZ_ASSERT(contents.kind() == ArrayBufferObject::INLINE_DATA);
     void* data = obj->inlineDataPointer();
     memset(data, 0, nbytes);
     obj->initialize(nbytes, BufferContents::createInlineData(data),
                     DoesntOwnData);
   } else {
     obj->initialize(nbytes, contents, ownsState);
   }
 
   return obj;
 }
 
-ArrayBufferObject* ArrayBufferObject::create(
+ArrayBufferObject* ArrayBufferObject::createZeroed(
     JSContext* cx, uint32_t nbytes, HandleObject proto /* = nullptr */) {
-  // The contents supplied here are not used other than for a nullity-check, so
-  // the choice to pass |createMalloced()| contents is arbitrary.  (Hopefully
-  // in time we can eliminate this internal wart.)
-  return create(cx, nbytes, BufferContents::createMalloced(nullptr),
-                OwnsState::OwnsData, proto);
+  // 24.1.1.1, step 3 (Inlined 6.2.6.1 CreateByteDataBlock, step 2).
+  if (!CheckArrayBufferTooLarge(cx, nbytes)) {
+    return nullptr;
+  }
+
+  // Try fitting the data inline with the object by repurposing fixed-slot
+  // storage.  Add extra fixed slots if necessary to accomplish this, but don't
+  // exceed the maximum number of fixed slots!
+  size_t nslots = JSCLASS_RESERVED_SLOTS(&class_);
+  uint8_t* data;
+  if (nbytes <= MaxInlineBytes) {
+    int newSlots = JS_HOWMANY(nbytes, sizeof(Value));
+    MOZ_ASSERT(int(nbytes) <= newSlots * int(sizeof(Value)));
+
+    nslots += newSlots;
+    data = nullptr;
+  } else {
+    data = AllocateArrayBufferContents(cx, nbytes).data();
+    if (!data) {
+      ReportOutOfMemory(cx);
+      return nullptr;
+    }
+  }
+
+  MOZ_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
+  gc::AllocKind allocKind = gc::GetGCObjectKind(nslots);
+
+  AutoSetNewObjectMetadata metadata(cx);
+  Rooted<ArrayBufferObject*> buffer(
+      cx, NewObjectWithClassProto<ArrayBufferObject>(cx, proto, allocKind,
+                                                     GenericObject));
+  if (!buffer) {
+    if (data) {
+      js_free(data);
+    }
+    return nullptr;
+  }
+
+  MOZ_ASSERT(!gc::IsInsideNursery(buffer),
+             "ArrayBufferObject has a finalizer that must be called to not "
+             "leak in some cases, so it can't be nursery-allocated");
+
+  if (data) {
+    buffer->initialize(nbytes, BufferContents::createMalloced(data), OwnsData);
+  } else {
+    void* inlineData = buffer->inlineDataPointer();
+    memset(inlineData, 0, nbytes);
+    buffer->initialize(nbytes, BufferContents::createInlineData(inlineData),
+                       DoesntOwnData);
+  }
+
+  return buffer;
 }
 
 ArrayBufferObject* ArrayBufferObject::createEmpty(JSContext* cx) {
   AutoSetNewObjectMetadata metadata(cx);
   ArrayBufferObject* obj = NewBuiltinClassInstance<ArrayBufferObject>(cx);
   if (!obj) {
     return nullptr;
   }
@@ -1646,65 +1705,65 @@ JS_FRIEND_API bool JS_IsDetachedArrayBuf
   }
 
   return aobj->isDetached();
 }
 
 JS_FRIEND_API JSObject* JS_NewArrayBuffer(JSContext* cx, uint32_t nbytes) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  MOZ_ASSERT(nbytes <= INT32_MAX);
-  return ArrayBufferObject::create(cx, nbytes);
+
+  return ArrayBufferObject::createZeroed(cx, nbytes);
 }
 
 JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
                                                       size_t nbytes,
                                                       void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   MOZ_ASSERT_IF(!data, nbytes == 0);
 
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createMalloced(data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
-                                   /* proto = */ nullptr, TenuredObject);
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
+      /* proto = */ nullptr, TenuredObject);
 }
 
 JS_PUBLIC_API JSObject* JS_NewExternalArrayBuffer(
     JSContext* cx, size_t nbytes, void* data,
     JS::BufferContentsFreeFunc freeFunc, void* freeUserData) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
 
   MOZ_ASSERT(data);
   MOZ_ASSERT(nbytes > 0);
 
   ArrayBufferObject::BufferContents contents =
       ArrayBufferObject::BufferContents::createExternal(data, freeFunc,
                                                         freeUserData);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
-                                   /* proto = */ nullptr, TenuredObject);
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
+      /* proto = */ nullptr, TenuredObject);
 }
 
 JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(JSContext* cx,
                                                                size_t nbytes,
                                                                void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   MOZ_ASSERT_IF(!data, nbytes == 0);
 
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createUserOwned(data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::DoesntOwnData,
-                                   /* proto = */ nullptr, TenuredObject);
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::DoesntOwnData,
+      /* proto = */ nullptr, TenuredObject);
 }
 
 JS_FRIEND_API bool JS_IsArrayBufferObject(JSObject* obj) {
   return obj->canUnwrapAs<ArrayBufferObject>();
 }
 
 JS_FRIEND_API bool JS_ArrayBufferHasData(JSObject* obj) {
   return !obj->unwrapAs<ArrayBufferObject>().isDetached();
@@ -1772,19 +1831,19 @@ JS_PUBLIC_API JSObject* JS_NewMappedArra
                                                             void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
 
   MOZ_ASSERT(data);
   ArrayBufferObject::BufferContents contents =
       ArrayBufferObject::BufferContents::create<ArrayBufferObject::MAPPED>(
           data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
-                                   /* proto = */ nullptr, TenuredObject);
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
+      /* proto = */ nullptr, TenuredObject);
 }
 
 JS_PUBLIC_API void* JS_CreateMappedArrayBufferContents(int fd, size_t offset,
                                                        size_t length) {
   return ArrayBufferObject::createMappedContents(fd, offset, length).data();
 }
 
 JS_PUBLIC_API void JS_ReleaseMappedArrayBufferContents(void* contents,
--- a/js/src/vm/ArrayBufferObject.h
+++ b/js/src/vm/ArrayBufferObject.h
@@ -160,18 +160,17 @@ class ArrayBufferObject : public ArrayBu
   static const size_t ARRAY_BUFFER_ALIGNMENT = 8;
 
   static_assert(FLAGS_SLOT == JS_ARRAYBUFFER_FLAGS_SLOT,
                 "self-hosted code with burned-in constants must get the "
                 "right flags slot");
 
   // The length of an ArrayBuffer or SharedArrayBuffer can be at most
   // INT32_MAX, and much code must change if this changes.
-
-  static const size_t MaxBufferByteLength = INT32_MAX;
+  static constexpr size_t MaxBufferByteLength = INT32_MAX;
 
   /** The largest number of bytes that can be stored inline. */
   static constexpr size_t MaxInlineBytes =
     (NativeObject::MAX_FIXED_SLOTS - RESERVED_SLOTS) * sizeof(JS::Value);
 
  public:
   enum OwnsState {
     DoesntOwnData = 0,
@@ -309,23 +308,23 @@ class ArrayBufferObject : public ArrayBu
   static bool fun_slice(JSContext* cx, unsigned argc, Value* vp);
 
   static bool fun_isView(JSContext* cx, unsigned argc, Value* vp);
 
   static bool fun_species(JSContext* cx, unsigned argc, Value* vp);
 
   static bool class_constructor(JSContext* cx, unsigned argc, Value* vp);
 
-  static ArrayBufferObject* create(JSContext* cx, uint32_t nbytes,
-                                   BufferContents contents,
-                                   OwnsState ownsState = OwnsData,
-                                   HandleObject proto = nullptr,
-                                   NewObjectKind newKind = GenericObject);
-  static ArrayBufferObject* create(JSContext* cx, uint32_t nbytes,
-                                   HandleObject proto = nullptr);
+  static ArrayBufferObject* createForContents(
+      JSContext* cx, uint32_t nbytes, BufferContents contents,
+      OwnsState ownsState = OwnsData, HandleObject proto = nullptr,
+      NewObjectKind newKind = GenericObject);
+
+  static ArrayBufferObject* createZeroed(JSContext* cx, uint32_t nbytes,
+                                         HandleObject proto = nullptr);
 
   // Create an ArrayBufferObject that is safely finalizable and can later be
   // initialize()d to become a real, content-visible ArrayBufferObject.
   static ArrayBufferObject* createEmpty(JSContext* cx);
 
   // Create an ArrayBufferObject using the provided buffer and size.  Assumes
   // ownership of |buffer| even in case of failure, i.e. on failure |buffer|
   // is deallocated.
--- a/js/src/vm/StructuredClone.cpp
+++ b/js/src/vm/StructuredClone.cpp
@@ -2199,17 +2199,17 @@ bool JSStructuredCloneReader::readDataVi
 
   allObjs[placeholderIndex].set(vp);
 
   return true;
 }
 
 bool JSStructuredCloneReader::readArrayBuffer(uint32_t nbytes,
                                               MutableHandleValue vp) {
-  JSObject* obj = ArrayBufferObject::create(context(), nbytes);
+  JSObject* obj = ArrayBufferObject::createZeroed(context(), nbytes);
   if (!obj) {
     return false;
   }
   vp.setObject(*obj);
   ArrayBufferObject& buffer = obj->as<ArrayBufferObject>();
   MOZ_ASSERT(buffer.byteLength() == nbytes);
   return in.readArray(buffer.dataPointer(), nbytes);
 }
@@ -2317,17 +2317,17 @@ bool JSStructuredCloneReader::readV1Arra
       TypedArrayElemSize(static_cast<Scalar::Type>(arrayType));
   if (!nbytes.isValid() || nbytes.value() > UINT32_MAX) {
     JS_ReportErrorNumberASCII(context(), GetErrorMessage, nullptr,
                               JSMSG_SC_BAD_SERIALIZED_DATA,
                               "invalid typed array size");
     return false;
   }
 
-  JSObject* obj = ArrayBufferObject::create(context(), nbytes.value());
+  JSObject* obj = ArrayBufferObject::createZeroed(context(), nbytes.value());
   if (!obj) {
     return false;
   }
   vp.setObject(*obj);
   ArrayBufferObject& buffer = obj->as<ArrayBufferObject>();
   MOZ_ASSERT(buffer.byteLength() == nbytes);
 
   switch (arrayType) {
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -74,17 +74,17 @@ using mozilla::IsAsciiDigit;
 /* static */ bool TypedArrayObject::ensureHasBuffer(
     JSContext* cx, Handle<TypedArrayObject*> tarray) {
   if (tarray->hasBuffer()) {
     return true;
   }
 
   AutoRealm ar(cx, tarray);
   Rooted<ArrayBufferObject*> buffer(
-      cx, ArrayBufferObject::create(cx, tarray->byteLength()));
+      cx, ArrayBufferObject::createZeroed(cx, tarray->byteLength()));
   if (!buffer) {
     return false;
   }
 
   // Attaching the first view to an array buffer is infallible.
   MOZ_ALWAYS_TRUE(buffer->addView(cx, tarray));
 
   // tarray is not shared, because if it were it would have a buffer.
@@ -902,17 +902,17 @@ class TypedArrayObjectTemplate : public 
                   "ArrayBuffer inline storage shouldn't waste any space");
 
     if (!nonDefaultProto && byteLength <= INLINE_BUFFER_LIMIT) {
       // The array's data can be inline, and the buffer created lazily.
       return true;
     }
 
     ArrayBufferObject* buf =
-        ArrayBufferObject::create(cx, byteLength, nonDefaultProto);
+        ArrayBufferObject::createZeroed(cx, byteLength, nonDefaultProto);
     if (!buf) {
       return false;
     }
 
     buffer.set(buf);
     return true;
   }
 
--- a/js/src/wasm/WasmJS.cpp
+++ b/js/src/wasm/WasmJS.cpp
@@ -988,17 +988,17 @@ static JSString* UTF8CharsToString(JSCon
   for (const CustomSection& cs : module->customSections()) {
     if (name.length() != cs.name.length()) {
       continue;
     }
     if (memcmp(name.begin(), cs.name.begin(), name.length())) {
       continue;
     }
 
-    buf = ArrayBufferObject::create(cx, cs.payload->length());
+    buf = ArrayBufferObject::createZeroed(cx, cs.payload->length());
     if (!buf) {
       return false;
     }
 
     memcpy(buf->dataPointer(), cs.payload->begin(), cs.payload->length());
     if (!elems.append(ObjectValue(*buf))) {
       return false;
     }