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

--HG--
extra : rebase_source : 0cc09680ee0e4b7b0cb616aac1157c59b05de5e6

js/src/builtin/TypedObject.cpp

@@ -1651,7 +1651,7 @@ void OutlineTypedObject::attach(JSContext* cx, TypedObject& typedObj,
   // 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;
   }

js/src/shell/js.cpp

@@ -1920,7 +1920,7 @@ static bool CacheEntry_setBytecode(JSContext* cx, HandleObject cache,
 
   BufferContents contents = BufferContents::createMalloced(buffer);
   Rooted<ArrayBufferObject*> arrayBuffer(
-      cx, ArrayBufferObject::create(cx, length, contents));
+      cx, ArrayBufferObject::createForContents(cx, length, contents));
   if (!arrayBuffer) {
     return false;
   }
@@ -6981,7 +6981,7 @@ class StreamCacheEntryObject : public NativeObject {
     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;
     }

js/src/vm/ArrayBufferObject.cpp

@@ -10,6 +10,7 @@
 #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"
@@ -433,7 +434,7 @@ bool ArrayBufferObject::class_constructor(JSContext* cx, unsigned argc,
   }
 
   // 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;
   }
@@ -1185,17 +1186,26 @@ 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;
   }
 
@@ -1258,7 +1268,9 @@ ArrayBufferObject* ArrayBufferObject::create(
   }
 
   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);
@@ -1273,13 +1285,60 @@ ArrayBufferObject* ArrayBufferObject::create(
   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) {
@@ -1651,8 +1710,8 @@ JS_FRIEND_API bool JS_IsDetachedArrayBufferObject(JSObject* obj) {
 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,
@@ -1665,8 +1724,8 @@ JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createMalloced(data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
       /* proto = */ nullptr, TenuredObject);
 }
 
@@ -1682,8 +1741,8 @@ JS_PUBLIC_API JSObject* JS_NewExternalArrayBuffer(
   ArrayBufferObject::BufferContents contents =
       ArrayBufferObject::BufferContents::createExternal(data, freeFunc,
                                                         freeUserData);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
       /* proto = */ nullptr, TenuredObject);
 }
 
@@ -1697,8 +1756,8 @@ JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(JSContext* cx,
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createUserOwned(data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::DoesntOwnData,
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::DoesntOwnData,
       /* proto = */ nullptr, TenuredObject);
 }
 
@@ -1777,8 +1836,8 @@ JS_PUBLIC_API JSObject* JS_NewMappedArrayBufferWithContents(JSContext* cx,
   ArrayBufferObject::BufferContents contents =
       ArrayBufferObject::BufferContents::create<ArrayBufferObject::MAPPED>(
           data);
-  return ArrayBufferObject::create(cx, nbytes, contents,
-                                   ArrayBufferObject::OwnsData,
+  return ArrayBufferObject::createForContents(
+      cx, nbytes, contents, ArrayBufferObject::OwnsData,
       /* proto = */ nullptr, TenuredObject);
 }
 

js/src/vm/ArrayBufferObject.h

@@ -165,8 +165,7 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared {
 
   // 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 =
@@ -314,12 +313,12 @@ class ArrayBufferObject : public ArrayBufferObjectMaybeShared {
 
   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,
+  static ArrayBufferObject* createForContents(
+      JSContext* cx, uint32_t nbytes, BufferContents contents,
+      OwnsState ownsState = OwnsData, HandleObject proto = nullptr,
       NewObjectKind newKind = GenericObject);
-  static ArrayBufferObject* create(JSContext* cx, uint32_t nbytes,
+
+  static ArrayBufferObject* createZeroed(JSContext* cx, uint32_t nbytes,
                                          HandleObject proto = nullptr);
 
   // Create an ArrayBufferObject that is safely finalizable and can later be

js/src/vm/StructuredClone.cpp

@@ -2204,7 +2204,7 @@ bool JSStructuredCloneReader::readDataView(uint32_t byteLength,
 
 bool JSStructuredCloneReader::readArrayBuffer(uint32_t nbytes,
                                               MutableHandleValue vp) {
-  JSObject* obj = ArrayBufferObject::create(context(), nbytes);
+  JSObject* obj = ArrayBufferObject::createZeroed(context(), nbytes);
   if (!obj) {
     return false;
   }
@@ -2322,7 +2322,7 @@ bool JSStructuredCloneReader::readV1ArrayBuffer(uint32_t arrayType,
     return false;
   }
 
-  JSObject* obj = ArrayBufferObject::create(context(), nbytes.value());
+  JSObject* obj = ArrayBufferObject::createZeroed(context(), nbytes.value());
   if (!obj) {
     return false;
   }

js/src/vm/TypedArrayObject.cpp

@@ -79,7 +79,7 @@ using mozilla::IsAsciiDigit;
 
   AutoRealm ar(cx, tarray);
   Rooted<ArrayBufferObject*> buffer(
-      cx, ArrayBufferObject::create(cx, tarray->byteLength()));
+      cx, ArrayBufferObject::createZeroed(cx, tarray->byteLength()));
   if (!buffer) {
     return false;
   }
@@ -907,7 +907,7 @@ class TypedArrayObjectTemplate : public TypedArrayObject {
     }
 
     ArrayBufferObject* buf =
-        ArrayBufferObject::create(cx, byteLength, nonDefaultProto);
+        ArrayBufferObject::createZeroed(cx, byteLength, nonDefaultProto);
     if (!buf) {
       return false;
     }

js/src/wasm/WasmJS.cpp

@@ -993,7 +993,7 @@ static JSString* UTF8CharsToString(JSContext* cx, const char* chars) {
       continue;
     }
 
-    buf = ArrayBufferObject::create(cx, cs.payload->length());
+    buf = ArrayBufferObject::createZeroed(cx, cs.payload->length());
     if (!buf) {
       return false;
     }