Bug 1529298 - Only pass BufferContents containing a non-null pointer to |ArrayBufferObject::createForContents|. r=sfink
☠☠ backed out by eed1098d0d6c ☠ ☠
authorJeff Walden <jwalden@mit.edu>
Mon, 18 Feb 2019 22:52:42 -0800
changeset 460368 e7fad41d68e05c402c314b596465d16670f7eb89
parent 460367 b3e668a95d3211f479b03b0ca7b2c5fa6a8fa39e
child 460369 deaa41ca96da55cb27d03ea2c60895a25474488a
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 - Only pass BufferContents containing a non-null pointer to |ArrayBufferObject::createForContents|. r=sfink
js/src/jsapi.h
js/src/vm/ArrayBufferObject.cpp
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2016,19 +2016,22 @@ extern JS_PUBLIC_API bool IsSetObject(JS
 /**
  * Assign 'undefined' to all of the object's non-reserved slots. Note: this is
  * done for all slots, regardless of the associated property descriptor.
  */
 JS_PUBLIC_API void JS_SetAllNonReservedSlotsToUndefined(JSContext* cx,
                                                         JSObject* objArg);
 
 /**
- * Create a new array buffer with the given contents. It must be legal to pass
- * these contents to JS_free(). On success, the ownership is transferred to the
- * new array buffer.
+ * Create a new array buffer with the given |contents|, which may be null only
+ * if |nbytes == 0|.  |contents| must be allocated compatible with deallocation
+ * by |JS_free|.
+ *
+ * If and only if an array buffer is successfully created and returned,
+ * ownership of |contents| is transferred to the new array buffer.
  */
 extern JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
                                                              size_t nbytes,
                                                              void* contents);
 
 namespace JS {
 
 using BufferContentsFreeFunc = void (*)(void* contents, void* userData);
@@ -2061,19 +2064,32 @@ using BufferContentsFreeFunc = void (*)(
  * function. This also allows using non-reference-counted contents that must be
  * freed with some function other than free().
  */
 extern JS_PUBLIC_API JSObject* JS_NewExternalArrayBuffer(
     JSContext* cx, size_t nbytes, void* contents,
     JS::BufferContentsFreeFunc freeFunc, void* freeUserData = nullptr);
 
 /**
- * Create a new array buffer with the given contents.  The array buffer does not
- * take ownership of contents.  JS_DetachArrayBuffer must be called before
- * the contents are disposed of by the user; this call will always succeed.
+ * Create a new ArrayBuffer with the given non-null |contents|.
+ *
+ * Ownership of |contents| remains with the caller: it isn't transferred to the
+ * returned ArrayBuffer.  Callers of this function *must* ensure that they
+ * perform these two steps, in this order, to properly relinquish ownership of
+ * |contents|:
+ *
+ *   1. Call |JS_DetachArrayBuffer| on the buffer returned by this function.
+ *      (|JS_DetachArrayBuffer| is generally fallible, but a call under these
+ *      circumstances is guaranteed to succeed.)
+ *   2. |contents| may be deallocated or discarded consistent with the manner
+ *      in which it was allocated.
+ *
+ * Do not simply allow the returned buffer to be garbage-collected before
+ * deallocating |contents|, because in general there is no way to know *when*
+ * an object is fully garbage-collected to the point where this would be safe.
  */
 extern JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(
     JSContext* cx, size_t nbytes, void* contents);
 
 /**
  * Steal the contents of the given array buffer. The array buffer has its
  * length set to 0 and its contents array cleared. The caller takes ownership
  * of the return value and must free it or transfer ownership via
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1715,45 +1715,52 @@ JS_FRIEND_API JSObject* JS_NewArrayBuffe
 
 JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
                                                       size_t nbytes,
                                                       void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
   MOZ_ASSERT_IF(!data, nbytes == 0);
 
+  if (!data) {
+    // Don't pass nulled contents to |createForContents|.
+    return ArrayBufferObject::createZeroed(cx, 0);
+  }
+
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createMalloced(data);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::OwnsData);
 }
 
 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);
+  using BufferContents = ArrayBufferObject::BufferContents;
+
+  BufferContents contents =
+      BufferContents::createExternal(data, freeFunc, freeUserData);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::OwnsData);
 }
 
 JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(JSContext* cx,
                                                                size_t nbytes,
                                                                void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  MOZ_ASSERT_IF(!data, nbytes == 0);
+
+  MOZ_ASSERT(data);
 
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createUserOwned(data);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::DoesntOwnData);
 }
 
@@ -1824,19 +1831,21 @@ JS_PUBLIC_API void* JS_StealArrayBufferC
 
 JS_PUBLIC_API JSObject* JS_NewMappedArrayBufferWithContents(JSContext* cx,
                                                             size_t nbytes,
                                                             void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
 
   MOZ_ASSERT(data);
-  ArrayBufferObject::BufferContents contents =
-      ArrayBufferObject::BufferContents::create<ArrayBufferObject::MAPPED>(
-          data);
+
+  using BufferContents = ArrayBufferObject::BufferContents;
+
+  BufferContents contents =
+      BufferContents::create<ArrayBufferObject::MAPPED>(data);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::OwnsData);
 }
 
 JS_PUBLIC_API void* JS_CreateMappedArrayBufferContents(int fd, size_t offset,
                                                        size_t length) {
   return ArrayBufferObject::createMappedContents(fd, offset, length).data();
 }