Bug 1841314 - Part 3: Prefer JS::NewArrayBufferWithContents with UniquePtr. r=sfink,saschanaz

Replace the existing callers of `JS::NewArrayBufferWithContents` with the new
`UniquePtr` alternative.

Three callers to the old `JS::NewArrayBufferWithContents` function were left
unchanged:
- `mozilla::dom::FileReader::OnLoadEndArrayBuffer()` and
  `mozilla::dom::ArrayBufferBuilder::TakeArrayBuffer()` both store the data
  buffer as members and therefore have a more complicated lifetime.
- `JSStructuredCloneReader::readTransferMap()` because it's not clear if the
  data can be free'ed when `ArrayBuffectObject` allocation fails.

Differential Revision: https://phabricator.services.mozilla.com/D182588
This commit is contained in:
André Bargull 2023-07-06 11:41:17 +00:00
parent 33e67df734
commit 0c386d5c4b
20 changed files with 75 additions and 63 deletions

View File

@ -651,7 +651,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
AssertIsOnTargetThread();
// This makes sure that we free the data correctly.
auto autoFree = mozilla::MakeScopeExit([&] { free(aResult); });
UniquePtr<uint8_t[], JS::FreePolicy> resultPtr{aResult};
if (mBodyConsumed) {
return;
@ -689,7 +689,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
}
// Finish successfully consuming body according to type.
MOZ_ASSERT(aResult);
MOZ_ASSERT(resultPtr);
AutoJSAPI jsapi;
if (!jsapi.Init(mGlobal)) {
@ -703,16 +703,14 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
switch (mConsumeType) {
case CONSUME_ARRAYBUFFER: {
JS::Rooted<JSObject*> arrayBuffer(cx);
BodyUtil::ConsumeArrayBuffer(cx, &arrayBuffer, aResultLength, aResult,
error);
BodyUtil::ConsumeArrayBuffer(cx, &arrayBuffer, aResultLength,
std::move(resultPtr), error);
if (!error.Failed()) {
JS::Rooted<JS::Value> val(cx);
val.setObjectOrNull(arrayBuffer);
localPromise->MaybeResolve(val);
// ArrayBuffer takes over ownership.
aResult = nullptr;
}
break;
}
@ -722,8 +720,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
}
case CONSUME_FORMDATA: {
nsCString data;
data.Adopt(reinterpret_cast<char*>(aResult), aResultLength);
aResult = nullptr;
data.Adopt(reinterpret_cast<char*>(resultPtr.release()), aResultLength);
RefPtr<dom::FormData> fd = BodyUtil::ConsumeFormData(
mGlobal, mBodyMimeType, mMixedCaseMimeType, data, error);
@ -737,7 +734,7 @@ void BodyConsumer::ContinueConsumeBody(nsresult aStatus, uint32_t aResultLength,
case CONSUME_JSON: {
nsString decoded;
if (NS_SUCCEEDED(
BodyUtil::ConsumeText(aResultLength, aResult, decoded))) {
BodyUtil::ConsumeText(aResultLength, resultPtr.get(), decoded))) {
if (mConsumeType == CONSUME_TEXT) {
localPromise->MaybeResolve(decoded);
} else {

View File

@ -353,11 +353,13 @@ class MOZ_STACK_CLASS FormDataParser {
// static
void BodyUtil::ConsumeArrayBuffer(JSContext* aCx,
JS::MutableHandle<JSObject*> aValue,
uint32_t aInputLength, uint8_t* aInput,
uint32_t aInputLength,
UniquePtr<uint8_t[], JS::FreePolicy> aInput,
ErrorResult& aRv) {
UniquePtr<void, JS::FreePolicy> dataPtr{aInput.release()};
JS::Rooted<JSObject*> arrayBuffer(aCx);
arrayBuffer = JS::NewArrayBufferWithContents(aCx, aInputLength,
reinterpret_cast<void*>(aInput));
arrayBuffer =
JS::NewArrayBufferWithContents(aCx, aInputLength, std::move(dataPtr));
if (!arrayBuffer) {
JS_ClearPendingException(aCx);
aRv.Throw(NS_ERROR_OUT_OF_MEMORY);

View File

@ -13,6 +13,8 @@
#include "mozilla/dom/File.h"
#include "mozilla/dom/FormData.h"
#include "js/Utility.h" // JS::FreePolicy
namespace mozilla {
class ErrorResult;
@ -30,7 +32,8 @@ class BodyUtil final {
*/
static void ConsumeArrayBuffer(JSContext* aCx,
JS::MutableHandle<JSObject*> aValue,
uint32_t aInputLength, uint8_t* aInput,
uint32_t aInputLength,
UniquePtr<uint8_t[], JS::FreePolicy> aInput,
ErrorResult& aRv);
/**

View File

@ -108,7 +108,7 @@ class CompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
do {
static uint16_t kBufferSize = 16384;
UniquePtr<uint8_t> buffer(
UniquePtr<uint8_t[], JS::FreePolicy> buffer(
static_cast<uint8_t*>(JS_malloc(aCx, kBufferSize)));
if (!buffer) {
aRv.ThrowTypeError("Out of memory");
@ -164,8 +164,8 @@ class CompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
// into Uint8Arrays.
// (The buffer is 'split' by having a fixed sized buffer above.)
JS::Rooted<JSObject*> view(
aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, written, buffer));
JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
aCx, written, std::move(buffer)));
if (!view || !array.append(view)) {
JS_ClearPendingException(aCx);
aRv.ThrowTypeError("Out of memory");

View File

@ -107,7 +107,7 @@ class DecompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
do {
static uint16_t kBufferSize = 16384;
UniquePtr<uint8_t> buffer(
UniquePtr<uint8_t[], JS::FreePolicy> buffer(
static_cast<uint8_t*>(JS_malloc(aCx, kBufferSize)));
if (!buffer) {
aRv.ThrowTypeError("Out of memory");
@ -194,8 +194,8 @@ class DecompressionStreamAlgorithms : public TransformerAlgorithmsWrapper {
// into Uint8Arrays.
// (The buffer is 'split' by having a fixed sized buffer above.)
JS::Rooted<JSObject*> view(
aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, written, buffer));
JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
aCx, written, std::move(buffer)));
if (!view || !array.append(view)) {
JS_ClearPendingException(aCx);
aRv.ThrowTypeError("Out of memory");

View File

@ -186,17 +186,16 @@ bool nsJSUtils::DumpEnabled() {
#endif
}
JSObject* nsJSUtils::MoveBufferAsUint8Array(JSContext* aCx, size_t aSize,
UniquePtr<uint8_t>& aBuffer) {
JSObject* nsJSUtils::MoveBufferAsUint8Array(
JSContext* aCx, size_t aSize,
UniquePtr<uint8_t[], JS::FreePolicy> aBuffer) {
UniquePtr<void, JS::FreePolicy> dataPtr{aBuffer.release()};
JS::Rooted<JSObject*> arrayBuffer(
aCx, JS::NewArrayBufferWithContents(aCx, aSize, aBuffer.get()));
aCx, JS::NewArrayBufferWithContents(aCx, aSize, std::move(dataPtr)));
if (!arrayBuffer) {
return nullptr;
}
// Now the ArrayBuffer owns the buffer, so let's release our ownership
(void)aBuffer.release();
return JS_NewUint8ArrayWithBuffer(aCx, arrayBuffer, 0,
static_cast<int64_t>(aSize));
}

View File

@ -21,6 +21,7 @@
#include "js/Conversions.h"
#include "js/SourceText.h"
#include "js/String.h" // JS::{,Lossy}CopyLinearStringChars, JS::CopyStringChars, JS::Get{,Linear}StringLength, JS::MaxStringLength, JS::StringHasLatin1Chars
#include "js/Utility.h" // JS::FreePolicy
#include "nsString.h"
#include "xpcpublic.h"
@ -87,8 +88,9 @@ class nsJSUtils {
// Note that the buffer needs to be created by JS_malloc (or at least can be
// freed by JS_free), as the resulting Uint8Array takes the ownership of the
// buffer.
static JSObject* MoveBufferAsUint8Array(JSContext* aCx, size_t aSize,
mozilla::UniquePtr<uint8_t>& aBuffer);
static JSObject* MoveBufferAsUint8Array(
JSContext* aCx, size_t aSize,
mozilla::UniquePtr<uint8_t[], JS::FreePolicy> aBuffer);
};
inline void AssignFromStringBuffer(nsStringBuffer* buffer, size_t len,

View File

@ -68,7 +68,7 @@ static void EncodeNative(JSContext* aCx, mozilla::Decoder* aDecoder,
return;
}
UniquePtr<uint8_t> buffer(
UniquePtr<uint8_t[], JS::FreePolicy> buffer(
static_cast<uint8_t*>(JS_malloc(aCx, needed.value())));
if (!buffer) {
aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
@ -98,8 +98,9 @@ static void EncodeNative(JSContext* aCx, mozilla::Decoder* aDecoder,
// https://encoding.spec.whatwg.org/#encode-and-enqueue-a-chunk
// Step 4.2.2.1. Let chunk be a Uint8Array object wrapping an ArrayBuffer
// containing output.
UniquePtr<void, JS::FreePolicy> dataPtr{buffer.release()};
JS::Rooted<JSObject*> arrayBuffer(
aCx, JS::NewArrayBufferWithContents(aCx, written, buffer.release()));
aCx, JS::NewArrayBufferWithContents(aCx, written, std::move(dataPtr)));
if (!arrayBuffer.get()) {
JS_ClearPendingException(aCx);
aRv.Throw(NS_ERROR_OUT_OF_MEMORY);

View File

@ -84,15 +84,13 @@ void FileReaderSync::ReadAsArrayBuffer(JSContext* aCx,
return;
}
UniquePtr<void, JS::FreePolicy> dataPtr{bufferData.release()};
JSObject* arrayBuffer =
JS::NewArrayBufferWithContents(aCx, blobSize, bufferData.get());
JS::NewArrayBufferWithContents(aCx, blobSize, std::move(dataPtr));
if (!arrayBuffer) {
aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
return;
}
// arrayBuffer takes the ownership when it is not null. Otherwise we
// need to release it explicitly.
(void)bufferData.release();
aRetval.set(arrayBuffer);
}

View File

@ -855,12 +855,19 @@ JSObject* Key::DecodeBinary(const EncodedDataType*& aPos,
DecodeStringy<eBinary, uint8_t>(
aPos, aEnd,
[&rv, aCx](uint8_t** out, uint32_t decodedSize) {
*out = static_cast<uint8_t*>(JS_malloc(aCx, decodedSize));
if (NS_WARN_IF(!*out)) {
UniquePtr<void, JS::FreePolicy> ptr{JS_malloc(aCx, decodedSize)};
if (NS_WARN_IF(!ptr)) {
*out = nullptr;
rv = nullptr;
return false;
}
rv = JS::NewArrayBufferWithContents(aCx, decodedSize, *out);
*out = static_cast<uint8_t*>(ptr.get());
rv = JS::NewArrayBufferWithContents(aCx, decodedSize, std::move(ptr));
if (NS_WARN_IF(!rv)) {
*out = nullptr;
return false;
}
return true;
},
[&rv, aCx] { rv = JS::NewArrayBuffer(aCx, 0); });

View File

@ -238,8 +238,9 @@ INSTANTIATE_TEST_SUITE_P(DOM_IndexedDB_Key, TestWithParam_LiteralString,
static JS::Value CreateArrayBufferValue(JSContext* const aContext,
const size_t aSize, char* const aData) {
Rooted<JSObject*> arrayBuffer{
aContext, JS::NewArrayBufferWithContents(aContext, aSize, aData)};
mozilla::UniquePtr<void, JS::FreePolicy> ptr{aData};
Rooted<JSObject*> arrayBuffer{aContext, JS::NewArrayBufferWithContents(
aContext, aSize, std::move(ptr))};
EXPECT_TRUE(arrayBuffer);
return JS::ObjectValue(*arrayBuffer);
}

View File

@ -2166,9 +2166,10 @@ void PeerConnectionImpl::DumpPacket_m(size_t level, dom::mozPacketDumpType type,
return;
}
UniquePtr<void, JS::FreePolicy> packetPtr{packet.release()};
JS::Rooted<JSObject*> jsobj(
jsapi.cx(),
JS::NewArrayBufferWithContents(jsapi.cx(), size, packet.release()));
JS::NewArrayBufferWithContents(jsapi.cx(), size, std::move(packetPtr)));
RootedSpiderMonkeyInterface<ArrayBuffer> arrayBuffer(jsapi.cx());
if (!arrayBuffer.Init(jsobj)) {

View File

@ -32,12 +32,10 @@ bool DeserializeArrayBuffer(JSContext* cx, const nsTArray<uint8_t>& aBuffer,
if (!data) return false;
memcpy(data.get(), aBuffer.Elements(), aBuffer.Length());
mozilla::UniquePtr<void, JS::FreePolicy> dataPtr{data.release()};
JSObject* obj =
JS::NewArrayBufferWithContents(cx, aBuffer.Length(), data.get());
JS::NewArrayBufferWithContents(cx, aBuffer.Length(), std::move(dataPtr));
if (!obj) return false;
// If JS::NewArrayBufferWithContents returns non-null, the ownership of
// the data is transfered to obj, so we release the ownership here.
mozilla::Unused << data.release();
aVal.setObject(*obj);
return true;

View File

@ -1093,7 +1093,9 @@ void PushMessageData::ArrayBuffer(JSContext* cx,
ErrorResult& aRv) {
uint8_t* data = GetContentsCopy();
if (data) {
BodyUtil::ConsumeArrayBuffer(cx, aRetval, mBytes.Length(), data, aRv);
UniquePtr<uint8_t[], JS::FreePolicy> dataPtr(data);
BodyUtil::ConsumeArrayBuffer(cx, aRetval, mBytes.Length(),
std::move(dataPtr), aRv);
}
}

View File

@ -26,7 +26,8 @@ JSObject* TransferArrayBuffer(JSContext* aCx, JS::Handle<JSObject*> aObject) {
size_t bufferLength = JS::GetArrayBufferByteLength(aObject);
// Step 2 (Reordered)
void* bufferData = JS::StealArrayBufferContents(aCx, aObject);
UniquePtr<void, JS::FreePolicy> bufferData{
JS::StealArrayBufferContents(aCx, aObject)};
// Step 4.
if (!JS::DetachArrayBuffer(aCx, aObject)) {
@ -34,7 +35,8 @@ JSObject* TransferArrayBuffer(JSContext* aCx, JS::Handle<JSObject*> aObject) {
}
// Step 5.
return JS::NewArrayBufferWithContents(aCx, bufferLength, bufferData);
return JS::NewArrayBufferWithContents(aCx, bufferLength,
std::move(bufferData));
}
// https://streams.spec.whatwg.org/#can-transfer-array-buffer

View File

@ -429,7 +429,8 @@ void InputToReadableStreamAlgorithms::PullFromInputStream(JSContext* aCx,
else {
// Step 9.1. Set view to the result of creating a Uint8Array from pulled in
// streams relevant Realm.
UniquePtr<uint8_t> buffer(static_cast<uint8_t*>(JS_malloc(aCx, pullSize)));
UniquePtr<uint8_t[], JS::FreePolicy> buffer(
static_cast<uint8_t*>(JS_malloc(aCx, pullSize)));
if (!buffer) {
aRv.ThrowTypeError("Out of memory");
return;
@ -446,8 +447,8 @@ void InputToReadableStreamAlgorithms::PullFromInputStream(JSContext* aCx,
}
MOZ_DIAGNOSTIC_ASSERT(pullSize == bytesWritten);
JS::Rooted<JSObject*> view(
aCx, nsJSUtils::MoveBufferAsUint8Array(aCx, bytesWritten, buffer));
JS::Rooted<JSObject*> view(aCx, nsJSUtils::MoveBufferAsUint8Array(
aCx, bytesWritten, std::move(buffer)));
if (!view) {
JS_ClearPendingException(aCx);
aRv.ThrowTypeError("Out of memory");

View File

@ -2708,20 +2708,16 @@ JSObject* IOUtils::JsBuffer::IntoUint8Array(JSContext* aCx, JsBuffer aBuffer) {
return JS_NewUint8Array(aCx, 0);
}
char* rawBuffer = aBuffer.mBuffer.release();
UniquePtr<void, JS::FreePolicy> rawBuffer{aBuffer.mBuffer.release()};
MOZ_RELEASE_ASSERT(rawBuffer);
JS::Rooted<JSObject*> arrayBuffer(
aCx, JS::NewArrayBufferWithContents(aCx, aBuffer.mLength,
reinterpret_cast<void*>(rawBuffer)));
std::move(rawBuffer)));
if (!arrayBuffer) {
// The array buffer does not take ownership of the data pointer unless
// creation succeeds. We are still on the hook to free it.
//
// aBuffer will be destructed at end of scope, but its destructor does not
// take into account |mCapacity| or |mLength|, so it is OK for them to be
// non-zero here with a null |mBuffer|.
js_free(rawBuffer);
return nullptr;
}

View File

@ -4989,10 +4989,10 @@ class CloneBufferObject : public NativeObject {
return false;
}
auto* rawBuffer = buffer.release();
JSObject* arrayBuffer = JS::NewArrayBufferWithContents(cx, size, rawBuffer);
UniquePtr<void, JS::FreePolicy> dataBuffer{buffer.release()};
JSObject* arrayBuffer =
JS::NewArrayBufferWithContents(cx, size, std::move(dataBuffer));
if (!arrayBuffer) {
js_free(rawBuffer);
return false;
}

View File

@ -66,14 +66,15 @@ BEGIN_TEST(testArrayBuffer_bug720949_steal) {
CHECK(v.isInt32(MAGIC_VALUE_2));
// Steal the contents
void* contents = JS::StealArrayBufferContents(cx, obj);
mozilla::UniquePtr<void, JS::FreePolicy> contents{
JS::StealArrayBufferContents(cx, obj)};
CHECK(contents != nullptr);
CHECK(JS::IsDetachedArrayBufferObject(obj));
// Transfer to a new ArrayBuffer
JS::RootedObject dst(cx,
JS::NewArrayBufferWithContents(cx, size, contents));
JS::RootedObject dst(
cx, JS::NewArrayBufferWithContents(cx, size, std::move(contents)));
CHECK(JS::IsArrayBufferObject(dst));
{
JS::AutoCheckCannotGC nogc;

View File

@ -490,8 +490,9 @@ class Span {
/**
* Constructor for mozilla::UniquePtr holding an array and length.
*/
template <class ArrayElementType = std::add_pointer<element_type>>
constexpr Span(const mozilla::UniquePtr<ArrayElementType>& aPtr,
template <class ArrayElementType = std::add_pointer<element_type>,
class DeleterType>
constexpr Span(const mozilla::UniquePtr<ArrayElementType, DeleterType>& aPtr,
index_type aLength)
: storage_(aPtr.get(), aLength) {}