diff --git a/src/api/callback.cc b/src/api/callback.cc index 0f458f470501..85da82ff84a5 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -245,7 +245,7 @@ MaybeLocal InternalMakeCallback(Environment* env, Local context = env->context(); if (use_async_hooks_trampoline) { - MaybeStackBuffer, 16> args(3 + argc); + MaybeStackBuffer args(env->isolate(), 3 + argc); args[0] = Number::New(env->isolate(), asyncContext.async_id); args[1] = resource; args[2] = callback; diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index a5a119fc8396..1c9c6ddaabd5 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -187,12 +187,12 @@ Local AddrTTLToArray( Environment* env, const T* addrttls, size_t naddrttls) { - MaybeStackBuffer, 8> ttls(naddrttls); + MaybeStackBuffer ttls(env->isolate(), naddrttls); for (size_t i = 0; i < naddrttls; i++) { ttls[i] = Integer::NewFromUnsigned(env->isolate(), addrttls[i].ttl); } - return Array::New(env->isolate(), ttls.out(), naddrttls); + return ttls.ToArray(); } // Parse the CSV produced by ares_get_servers_csv() back into (ip, port) diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index 8ef74aee2d0e..04ffece14493 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -41,7 +41,6 @@ using ncrypto::MarkPopErrorOnReturn; using ncrypto::SSLPointer; using ncrypto::SSLSessionPointer; using ncrypto::X509Pointer; -using v8::Array; using v8::ArrayBuffer; using v8::ArrayBufferView; using v8::BackingStore; @@ -1958,7 +1957,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo& args) { SSL* ssl = w->ssl_.get(); int nsig = SSL_get_shared_sigalgs(ssl, 0, nullptr, nullptr, nullptr, nullptr, nullptr); - MaybeStackBuffer, 16> ret_arr(nsig); + MaybeStackBuffer ret_arr(env->isolate(), nsig); for (int i = 0; i < nsig; i++) { int hash_nid; @@ -2025,8 +2024,7 @@ void TLSWrap::GetSharedSigalgs(const FunctionCallbackInfo& args) { ret_arr[i] = OneByteString(env->isolate(), sig_with_md); } - args.GetReturnValue().Set( - Array::New(env->isolate(), ret_arr.out(), ret_arr.length())); + args.GetReturnValue().Set(ret_arr.ToArray()); } void TLSWrap::ExportKeyingMaterial(const FunctionCallbackInfo& args) { diff --git a/src/js_stream.cc b/src/js_stream.cc index 0d6d6a7ac80a..3a3a1c943d25 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -11,7 +11,6 @@ namespace node { using errors::TryCatchScope; -using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -119,17 +118,14 @@ int JSStream::DoWrite(WriteWrap* w, int value_int = UV_EPROTO; - MaybeStackBuffer, 16> bufs_arr(count); + MaybeStackBuffer bufs_arr(env()->isolate(), count); for (size_t i = 0; i < count; i++) { if (!Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocal(&bufs_arr[i])) { return value_int; } } - Local argv[] = { - w->object(), - Array::New(env()->isolate(), bufs_arr.out(), count) - }; + Local argv[] = {w->object(), bufs_arr.ToArray()}; TryCatchScope try_catch(env()); Local value; diff --git a/src/js_udp_wrap.cc b/src/js_udp_wrap.cc index 1648d162b8f7..83b91042023f 100644 --- a/src/js_udp_wrap.cc +++ b/src/js_udp_wrap.cc @@ -11,7 +11,6 @@ namespace node { using errors::TryCatchScope; -using v8::Array; using v8::Context; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -98,7 +97,7 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, int64_t value_int = JS_EXCEPTION_PENDING; size_t total_len = 0; - MaybeStackBuffer, 16> buffers(nbufs); + MaybeStackBuffer buffers(env()->isolate(), nbufs); for (size_t i = 0; i < nbufs; i++) { if (!Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocal(&buffers[i])) { return value_int; @@ -110,9 +109,9 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, if (!AddressToJS(env(), addr).ToLocal(&address)) return value_int; Local args[] = { - listener()->CreateSendWrap(total_len)->object(), - Array::New(env()->isolate(), buffers.out(), nbufs), - address, + listener()->CreateSendWrap(total_len)->object(), + buffers.ToArray(), + address, }; if (!MakeCallback(env()->onwrite_string(), arraysize(args), args) diff --git a/src/node_dir.cc b/src/node_dir.cc index 952161d9e2cf..eca5140b6423 100644 --- a/src/node_dir.cc +++ b/src/node_dir.cc @@ -206,7 +206,7 @@ static MaybeLocal DirentListToArray(Environment* env, uv_dirent_t* ents, int num, enum encoding encoding) { - MaybeStackBuffer, 64> entries(num * 2); + MaybeStackBuffer entries(env->isolate(), num * 2); // Return an array of all read filenames. int j = 0; @@ -222,7 +222,8 @@ static MaybeLocal DirentListToArray(Environment* env, entries[j++] = Integer::New(env->isolate(), ents[i].type); } - return Array::New(env->isolate(), entries.out(), j); + CHECK_EQ(j, num * 2); + return entries.ToArray(); } static void AfterDirRead(uv_fs_t* req) { diff --git a/src/node_env_var.cc b/src/node_env_var.cc index e94180cd659d..25e405be86d2 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -201,7 +201,7 @@ MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { auto cleanup = OnScopeLeave([&]() { uv_os_free_environ(items, count); }); CHECK_EQ(uv_os_environ(&items, &count), 0); - MaybeStackBuffer, 256> env_v(count); + MaybeStackBuffer env_v(isolate, count); int env_v_index = 0; for (int i = 0; i < count; i++) { #ifdef _WIN32 @@ -216,7 +216,10 @@ MaybeLocal RealEnvStore::Enumerate(Isolate* isolate) const { env_v[env_v_index++] = str; } - return Array::New(isolate, env_v.out(), env_v_index); + // We're possibly not filling the entire buffer. + CHECK_LE(env_v_index, count); + env_v.SetLength(env_v_index); + return env_v.ToArray(); } std::shared_ptr KVStore::Clone(Isolate* isolate) const { diff --git a/src/node_http2.cc b/src/node_http2.cc index 04b2acca148d..58f12c5561b5 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1609,8 +1609,8 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { // this way for performance reasons (it's faster to generate and pass an // array than it is to generate and pass the object). - MaybeStackBuffer, 64> headers_v(stream->headers_count() * 2); - MaybeStackBuffer, 32> sensitive_v(stream->headers_count()); + MaybeStackBuffer headers_v(isolate, stream->headers_count() * 2); + MaybeStackBuffer sensitive_v(isolate, stream->headers_count()); size_t sensitive_count = 0; stream->TransferHeaders([&](const Http2Header& header, size_t i) { @@ -1627,13 +1627,14 @@ void Http2Session::HandleHeadersFrame(const nghttp2_frame* frame) { stream->retained_headers_length_ += stream->current_headers_length_; stream->current_headers_length_ = 0; + sensitive_v.SetLength(sensitive_count); Local args[] = { - stream->object(), - Integer::New(isolate, id), - Integer::New(isolate, stream->headers_category()), - Integer::New(isolate, frame->hd.flags), - Array::New(isolate, headers_v.out(), headers_v.length()), - Array::New(isolate, sensitive_v.out(), sensitive_count), + stream->object(), + Integer::New(isolate, id), + Integer::New(isolate, stream->headers_category()), + Integer::New(isolate, frame->hd.flags), + headers_v.ToArray(), + sensitive_v.ToArray(), }; MakeCallback(env()->http2session_on_headers_function(), arraysize(args), args); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index f00ab803fef0..5a0c4962d3f7 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1093,7 +1093,7 @@ void MessagePort::PostMessage(const FunctionCallbackInfo& args) { "MessagePort.postMessage"); } - TransferList transfer_list; + TransferList transfer_list(env->isolate()); if (!GetTransferList(env, context, args[1], &transfer_list)) { return; } @@ -1607,7 +1607,7 @@ static void StructuredClone(const FunctionCallbackInfo& args) { Local value = args[0]; - TransferList transfer_list; + TransferList transfer_list(isolate); Local options = args[1].As(); Local transfer_list_v; if (!options->Get(context, env->transfer_string()) diff --git a/src/node_messaging.h b/src/node_messaging.h index 3a838a39200a..0abd7da3bf02 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -17,7 +17,7 @@ namespace worker { class MessagePortData; class MessagePort; -typedef MaybeStackBuffer, 8> TransferList; +typedef MaybeStackBuffer TransferList; // Used to represent the in-flight structure of an object that is being // transferred or cloned using postMessage(). diff --git a/src/node_v8.cc b/src/node_v8.cc index 34e8460790b7..14ee626c9f3e 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -745,17 +745,17 @@ void Initialize(Local target, // Heap space names are extracted once and exposed to JavaScript to // avoid excessive creation of heap space name Strings. HeapSpaceStatistics s; - MaybeStackBuffer, 16> heap_spaces(number_of_heap_spaces); + MaybeStackBuffer heap_spaces(env->isolate(), + number_of_heap_spaces); for (size_t i = 0; i < number_of_heap_spaces; i++) { env->isolate()->GetHeapSpaceStatistics(&s, i); heap_spaces[i] = String::NewFromUtf8(env->isolate(), s.space_name()) .ToLocalChecked(); } target - ->Set( - context, - FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"), - Array::New(env->isolate(), heap_spaces.out(), number_of_heap_spaces)) + ->Set(context, + FIXED_ONE_BYTE_STRING(env->isolate(), "kHeapSpaces"), + heap_spaces.ToArray()) .Check(); SetMethod(context, diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 2da2e18950ad..0358bb995fee 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -767,7 +767,7 @@ MaybeLocal SyncProcessRunner::BuildOutputArray() { CHECK(!stdio_pipes_.empty()); EscapableHandleScope scope(env()->isolate()); - MaybeStackBuffer, 8> js_output(stdio_pipes_.size()); + MaybeStackBuffer js_output(env()->isolate(), stdio_pipes_.size()); for (uint32_t i = 0; i < stdio_pipes_.size(); i++) { SyncProcessStdioPipe* h = stdio_pipes_[i].get(); @@ -781,8 +781,7 @@ MaybeLocal SyncProcessRunner::BuildOutputArray() { } } - return scope.Escape( - Array::New(env()->isolate(), js_output.out(), js_output.length())); + return scope.Escape(js_output.ToArray()); } Maybe SyncProcessRunner::ParseOptions(Local js_value) { diff --git a/src/util-inl.h b/src/util-inl.h index e357d15a1449..909334fec3e7 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -394,14 +394,13 @@ v8::MaybeLocal ToV8Value(v8::Local context, if (isolate == nullptr) isolate = v8::Isolate::GetCurrent(); v8::EscapableHandleScope handle_scope(isolate); - MaybeStackBuffer, 128> arr(vec.size()); - arr.SetLength(vec.size()); + MaybeStackBuffer arr(isolate, vec.size()); for (size_t i = 0; i < vec.size(); ++i) { if (!ToV8Value(context, vec[i], isolate).ToLocal(&arr[i])) return v8::MaybeLocal(); } - return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); + return handle_scope.Escape(arr.ToArray()); } template @@ -430,8 +429,7 @@ v8::MaybeLocal ToV8Value(v8::Local context, if (isolate == nullptr) isolate = v8::Isolate::GetCurrent(); v8::EscapableHandleScope handle_scope(isolate); - MaybeStackBuffer, 128> arr(vec.size()); - arr.SetLength(vec.size()); + MaybeStackBuffer arr(isolate, vec.size()); auto it = vec.begin(); for (size_t i = 0; i < vec.size(); ++i) { if (!ToV8Value(context, *it, isolate).ToLocal(&arr[i])) @@ -439,7 +437,7 @@ v8::MaybeLocal ToV8Value(v8::Local context, std::advance(it, 1); } - return handle_scope.Escape(v8::Array::New(isolate, arr.out(), arr.length())); + return handle_scope.Escape(arr.ToArray()); } template @@ -523,7 +521,14 @@ v8::Local ToV8ValuePrimitiveArray(v8::Local context, } SlicedArguments::SlicedArguments( - const v8::FunctionCallbackInfo& args, size_t start) { + const v8::FunctionCallbackInfo& args, size_t start) + : SlicedArguments(args.GetIsolate(), args, start) {} + +SlicedArguments::SlicedArguments( + v8::Isolate* isolate, + const v8::FunctionCallbackInfo& args, + size_t start) + : MaybeStackBuffer(isolate) { const size_t length = static_cast(args.Length()); if (start >= length) return; const size_t size = length - start; @@ -549,6 +554,27 @@ void MaybeStackBuffer::AllocateSufficientStorage( length_ = storage; } +template +void MaybeStackBuffer::AllocateSufficientStorage( + size_t storage) { + CHECK(!IsInvalidated()); + if (storage > capacity()) { + if (!local_vector_.has_value()) { + local_vector_.emplace(isolate_, storage); + // Copy existing stack data into the LocalVector. + for (size_t i = 0; i < length_; i++) { + (*local_vector_)[i] = buf_st_[i]; + } + } else { + local_vector_->resize(storage); + } + buf_ = local_vector_->data(); + capacity_ = storage; + } + + length_ = storage; +} + template ArrayBufferViewContents::ArrayBufferViewContents( v8::Local value) { diff --git a/src/util.h b/src/util.h index 48305bfdc131..8b31ee1390b7 100644 --- a/src/util.h +++ b/src/util.h @@ -398,6 +398,12 @@ concept standard_char_type = std::is_same_v || std::is_same_v || std::is_same_v; +// Types that can reside on V8's managed heap (v8::Value, v8::Object, etc.). +// Used to select the MaybeStackBuffer specialization that holds handles in a +// v8::LocalVector instead of malloc'd memory. +template +concept v8_type = std::is_base_of_v; + // Allocates an array of member type T. For up to kStackStorageSize items, // the stack is used, otherwise malloc(). template @@ -530,6 +536,85 @@ class MaybeStackBuffer { T buf_st_[kStackStorageSize]; }; +template +class MaybeStackBuffer { + public: + using V = v8::Local; + + MaybeStackBuffer(const MaybeStackBuffer&) = delete; + MaybeStackBuffer& operator=(const MaybeStackBuffer& other) = delete; + + const V* out() const { return buf_; } + V* out() { return buf_; } + + // operator* for compatibility with `v8::String::(Utf8)Value` + V* operator*() { return buf_; } + const V* operator*() const { return buf_; } + + V& operator[](size_t index) { + CHECK_LT(index, length()); + return buf_[index]; + } + + const V& operator[](size_t index) const { + CHECK_LT(index, length()); + return buf_[index]; + } + + size_t length() const { return length_; } + + // Current maximum capacity of the buffer with which SetLength() can be used + // without first calling AllocateSufficientStorage(). + size_t capacity() const { return capacity_; } + + // Make sure enough space for `storage` entries is available. + // This method can be called multiple times throughout the lifetime of the + // buffer, but once this has been called Invalidate() cannot be used. + // Content of the buffer in the range [0, length()) is preserved. + void AllocateSufficientStorage(size_t storage); + + void SetLength(size_t length) { + // capacity() returns how much memory is actually available. + CHECK_LE(length, capacity()); + length_ = length; + } + + // If the buffer is stored in a LocalVector rather than on the stack. + bool IsAllocated() const { return !IsInvalidated() && buf_ != buf_st_; } + + // If Invalidate() has been called. + bool IsInvalidated() const { return buf_ == nullptr; } + + explicit MaybeStackBuffer(v8::Isolate* isolate) + : isolate_(isolate), + length_(0), + capacity_(arraysize(buf_st_)), + buf_(buf_st_) { + // Default to a zero-length, null-terminated buffer. + buf_[0] = V(); + } + + MaybeStackBuffer(v8::Isolate* isolate, size_t storage) + : MaybeStackBuffer(isolate) { + AllocateSufficientStorage(storage); + } + + // LocalVector (via optional) handles cleanup automatically. + ~MaybeStackBuffer() = default; + + v8::Local ToArray() const { + return v8::Array::New(isolate_, buf_, length_); + } + + private: + v8::Isolate* isolate_; + size_t length_; + size_t capacity_; + V* buf_; + V buf_st_[kStackStorageSize]; + std::optional> local_vector_; +}; + // Provides access to an ArrayBufferView's storage, either the original, // or for small data, a copy of it. This object's lifetime is bound to the // original ArrayBufferView's lifetime. @@ -791,10 +876,13 @@ constexpr inline bool IsBigEndian() { static_assert(IsLittleEndian() || IsBigEndian(), "Node.js does not support mixed-endian systems"); -class SlicedArguments : public MaybeStackBuffer> { +class SlicedArguments : public MaybeStackBuffer { public: inline explicit SlicedArguments( const v8::FunctionCallbackInfo& args, size_t start = 0); + inline SlicedArguments(v8::Isolate* isolate, + const v8::FunctionCallbackInfo& args, + size_t start = 0); }; // Convert a v8::PersistentBase, e.g. v8::Global, to a Local, with an extra