Bug 1529298 - Make AllocateArrayBufferContents return uint8_t*, and make its callers consistently not redundantly report OOMs. r=sfink
☠☠ backed out by eed1098d0d6c ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Mon, 18 Feb 2019 22:52:42 -0800
changeset 460367 b3e668a95d3211f479b03b0ca7b2c5fa6a8fa39e
parent 460366 c9f62f10eeb5db27027ebb2c6ad54db1d23a9db6
child 460368 e7fad41d68e05c402c314b596465d16670f7eb89
push id112082
push userjwalden@mit.edu
push dateFri, 22 Feb 2019 02:22:21 +0000
treeherdermozilla-inbound@d80b681a68e6 [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 - Make AllocateArrayBufferContents return uint8_t*, and make its callers consistently not redundantly report OOMs. r=sfink
js/src/vm/ArrayBufferObject.cpp
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -437,21 +437,22 @@ bool ArrayBufferObject::class_constructo
   JSObject* bufobj = createZeroed(cx, uint32_t(byteLength), proto);
   if (!bufobj) {
     return false;
   }
   args.rval().setObject(*bufobj);
   return true;
 }
 
-static ArrayBufferObject::BufferContents AllocateArrayBufferContents(
-    JSContext* cx, uint32_t nbytes) {
-  uint8_t* p =
-      cx->pod_callocCanGC<uint8_t>(nbytes, js::ArrayBufferContentsArena);
-  return ArrayBufferObject::BufferContents::createMalloced(p);
+static uint8_t* AllocateArrayBufferContents(JSContext* cx, uint32_t nbytes) {
+  auto* p = cx->pod_callocCanGC<uint8_t>(nbytes, js::ArrayBufferContentsArena);
+  if (!p) {
+    ReportOutOfMemory(cx);
+  }
+  return p;
 }
 
 static void NoteViewBufferWasDetached(
     ArrayBufferViewObject* view, ArrayBufferObject::BufferContents newContents,
     JSContext* cx) {
   MOZ_ASSERT(!view->isSharedMemory());
 
   view->notifyBufferDetached(newContents.data());
@@ -957,23 +958,22 @@ bool js::CreateWasmBuffer(JSContext* cx,
              buffer->isExternal());
 
   // Buffers already prepared for asm.js need no further work.
   if (buffer->isPreparedForAsmJS()) {
     return true;
   }
 
   if (!buffer->ownsData()) {
-    BufferContents contents =
-        AllocateArrayBufferContents(cx, buffer->byteLength());
-    if (!contents) {
+    uint8_t* data = AllocateArrayBufferContents(cx, buffer->byteLength());
+    if (!data) {
       return false;
     }
-    memcpy(contents.data(), buffer->dataPointer(), buffer->byteLength());
-    buffer->changeContents(cx, contents, OwnsData);
+    memcpy(data, buffer->dataPointer(), buffer->byteLength());
+    buffer->changeContents(cx, BufferContents::createMalloced(data), OwnsData);
   }
 
   buffer->setIsPreparedForAsmJS();
   return true;
 }
 
 ArrayBufferObject::BufferContents ArrayBufferObject::createMappedContents(
     int fd, size_t offset, size_t length) {
@@ -1239,21 +1239,22 @@ ArrayBufferObject* ArrayBufferObject::cr
   } else {
     MOZ_ASSERT(ownsState == OwnsData);
     if (nbytes <= MaxInlineBytes) {
       int newSlots = JS_HOWMANY(nbytes, sizeof(Value));
       MOZ_ASSERT(int(nbytes) <= newSlots * int(sizeof(Value)));
       nslots = reservedSlots + newSlots;
       contents = BufferContents::createInlineData(nullptr);
     } else {
-      contents = AllocateArrayBufferContents(cx, nbytes);
-      if (!contents) {
-        ReportOutOfMemory(cx);
+      uint8_t* data = AllocateArrayBufferContents(cx, nbytes);
+      if (!data) {
         return nullptr;
       }
+
+      contents = BufferContents::createMalloced(data);
       allocated = true;
     }
   }
 
   MOZ_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
   gc::AllocKind allocKind = gc::GetGCObjectKind(nslots);
 
   AutoSetNewObjectMetadata metadata(cx);
@@ -1299,19 +1300,18 @@ ArrayBufferObject* ArrayBufferObject::cr
   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();
+    data = AllocateArrayBufferContents(cx, nbytes);
     if (!data) {
-      ReportOutOfMemory(cx);
       return nullptr;
     }
   }
 
   MOZ_ASSERT(!(class_.flags & JSCLASS_HAS_PRIVATE));
   gc::AllocKind allocKind = gc::GetGCObjectKind(nslots);
 
   AutoSetNewObjectMetadata metadata(cx);
@@ -1396,27 +1396,26 @@ ArrayBufferObject* ArrayBufferObject::cr
     buffer->setOwnsData(DoesntOwnData);  // Do not free the stolen data.
     ArrayBufferObject::detach(cx, buffer, newContents);
     buffer->setOwnsData(DoesntOwnData);  // Do not free the nullptr.
     return oldContents;
   }
 
   // Create a new chunk of memory to return since we cannot steal the
   // existing contents away from the buffer.
-  BufferContents contentsCopy =
-      AllocateArrayBufferContents(cx, buffer->byteLength());
-  if (!contentsCopy) {
+  uint8_t* dataCopy = AllocateArrayBufferContents(cx, buffer->byteLength());
+  if (!dataCopy) {
     return BufferContents::createFailed();
   }
 
   if (buffer->byteLength() > 0) {
-    memcpy(contentsCopy.data(), oldContents.data(), buffer->byteLength());
+    memcpy(dataCopy, oldContents.data(), buffer->byteLength());
   }
   ArrayBufferObject::detach(cx, buffer, oldContents);
-  return contentsCopy;
+  return BufferContents::createMalloced(dataCopy);
 }
 
 /* static */ void ArrayBufferObject::addSizeOfExcludingThis(
     JSObject* obj, mozilla::MallocSizeOf mallocSizeOf, JS::ClassInfo* info) {
   ArrayBufferObject& buffer = AsArrayBuffer(obj);
 
   if (!buffer.ownsData()) {
     return;