From 07cda176d09597be476cbf9b50edcd59967a3844 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Feb 2019 18:37:58 +0100 Subject: [PATCH 1/8] deps: V8: cherry-pick d3308d0 Original commit message: [api] Add `Isolate::GetArrayBufferAllocator()` This allows non-monolithic embedders to always allocate memory for ArrayBuffer instances using the right allocation method. This is based on a patch that Electron is currently using. Refs: https://github.com/electron/electron/blob/1898f9162073910c05958295c612deec6121a892/patches/common/v8/array_buffer.patch Change-Id: I39a614343118a0594aab48699a99cc2aad5b7ba9 Reviewed-on: https://chromium-review.googlesource.com/c/1462003 Reviewed-by: Yang Guo Commit-Queue: Yang Guo Cr-Commit-Position: refs/heads/master@{#59697} Refs: https://github.com/v8/v8/commit/d3308d042c9637958491333831c33335ab9fc734 --- deps/v8/include/v8.h | 3 +++ deps/v8/src/api.cc | 5 +++++ deps/v8/test/cctest/test-api.cc | 1 + 3 files changed, 9 insertions(+) diff --git a/deps/v8/include/v8.h b/deps/v8/include/v8.h index 01fb56b55db193..0f0eb2e739a981 100644 --- a/deps/v8/include/v8.h +++ b/deps/v8/include/v8.h @@ -7642,6 +7642,9 @@ class V8_EXPORT Isolate { */ void SetIdle(bool is_idle); + /** Returns the ArrayBuffer::Allocator used in this isolate. */ + ArrayBuffer::Allocator* GetArrayBufferAllocator(); + /** Returns true if this isolate has a current context. */ bool InContext(); diff --git a/deps/v8/src/api.cc b/deps/v8/src/api.cc index 09db471982ecde..40e8b41e69cce5 100644 --- a/deps/v8/src/api.cc +++ b/deps/v8/src/api.cc @@ -8007,6 +8007,11 @@ void Isolate::SetIdle(bool is_idle) { isolate->SetIdle(is_idle); } +ArrayBuffer::Allocator* Isolate::GetArrayBufferAllocator() { + i::Isolate* isolate = reinterpret_cast(this); + return isolate->array_buffer_allocator(); +} + bool Isolate::InContext() { i::Isolate* isolate = reinterpret_cast(this); return isolate->context() != nullptr; diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 3b564d9bf3eaa5..70763547ea86ae 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -20881,6 +20881,7 @@ TEST(IsolateNewDispose) { CHECK_NOT_NULL(isolate); CHECK(current_isolate != isolate); CHECK(current_isolate == CcTest::isolate()); + CHECK(isolate->GetArrayBufferAllocator() == CcTest::array_buffer_allocator()); isolate->SetFatalErrorHandler(StoringErrorCallback); last_location = last_message = nullptr; From 1b6bbca8e33387b1610087484bd50d518374ba13 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 Feb 2019 17:30:42 +0100 Subject: [PATCH 2/8] src: make IsolateData store ArrayBufferAllocator This enables us to identify whether we are using an allocator that we know more about than what the generic `ArrayBuffer::Allocator` API provides, in particular whether it is `malloc()`-compatible. --- src/api/environment.cc | 6 +----- src/env-inl.h | 12 ++++++++++-- src/env.cc | 13 ++++++++----- src/env.h | 14 ++++++++++---- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 689f82bf245168..7b6de8ba8dc367 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -124,11 +124,7 @@ IsolateData* CreateIsolateData(Isolate* isolate, uv_loop_t* loop, MultiIsolatePlatform* platform, ArrayBufferAllocator* allocator) { - return new IsolateData( - isolate, - loop, - platform, - allocator != nullptr ? allocator->zero_fill_field() : nullptr); + return new IsolateData(isolate, loop, platform, allocator); } void FreeIsolateData(IsolateData* isolate_data) { diff --git a/src/env-inl.h b/src/env-inl.h index aca817605673fd..51c7e0d7b06561 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -49,8 +49,16 @@ inline uv_loop_t* IsolateData::event_loop() const { return event_loop_; } -inline uint32_t* IsolateData::zero_fill_field() const { - return zero_fill_field_; +inline bool IsolateData::uses_node_allocator() const { + return uses_node_allocator_; +} + +inline v8::ArrayBuffer::Allocator* IsolateData::allocator() const { + return allocator_; +} + +inline ArrayBufferAllocator* IsolateData::node_allocator() const { + return node_allocator_; } inline MultiIsolatePlatform* IsolateData::platform() const { diff --git a/src/env.cc b/src/env.cc index 01aee464406628..b7b6d745d8a231 100644 --- a/src/env.cc +++ b/src/env.cc @@ -74,11 +74,14 @@ void* const Environment::kNodeContextTagPtr = const_cast( IsolateData::IsolateData(Isolate* isolate, uv_loop_t* event_loop, MultiIsolatePlatform* platform, - uint32_t* zero_fill_field) : - isolate_(isolate), - event_loop_(event_loop), - zero_fill_field_(zero_fill_field), - platform_(platform) { + ArrayBufferAllocator* node_allocator) + : isolate_(isolate), + event_loop_(event_loop), + allocator_(isolate->GetArrayBufferAllocator()), + node_allocator_(node_allocator), + uses_node_allocator_(allocator_ == node_allocator_), + platform_(platform) { + CHECK_NOT_NULL(allocator_); if (platform_ != nullptr) platform_->RegisterIsolate(isolate_, event_loop); diff --git a/src/env.h b/src/env.h index 5f578dd54a9a7f..527a28f6957fd8 100644 --- a/src/env.h +++ b/src/env.h @@ -394,16 +394,20 @@ class Environment; class IsolateData { public: - IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop, + IsolateData(v8::Isolate* isolate, + uv_loop_t* event_loop, MultiIsolatePlatform* platform = nullptr, - uint32_t* zero_fill_field = nullptr); + ArrayBufferAllocator* node_allocator = nullptr); ~IsolateData(); inline uv_loop_t* event_loop() const; - inline uint32_t* zero_fill_field() const; inline MultiIsolatePlatform* platform() const; inline std::shared_ptr options(); inline void set_options(std::shared_ptr options); + inline bool uses_node_allocator() const; + inline v8::ArrayBuffer::Allocator* allocator() const; + inline ArrayBufferAllocator* node_allocator() const; + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) @@ -436,7 +440,9 @@ class IsolateData { v8::Isolate* const isolate_; uv_loop_t* const event_loop_; - uint32_t* const zero_fill_field_; + v8::ArrayBuffer::Allocator* const allocator_; + ArrayBufferAllocator* const node_allocator_; + const bool uses_node_allocator_; MultiIsolatePlatform* platform_; std::shared_ptr options_; From 814c8a9b4ced145e688745c620ccf1a33fff239a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 19 Feb 2019 14:56:52 +0100 Subject: [PATCH 3/8] worker: copy transferList ArrayBuffers on unknown allocator If the `ArrayBuffer::Allocator` used to create `ArrayBuffer`s in the current `Isolate` is not a Node.js `ArrayBufferAllocator`, we cannot know that it is `malloc()`-based, an in particular it might not be compatible with the `ArrayBuffer::Allocator` on the receiving end of the connection. --- src/node_messaging.cc | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 16e46f9dd02409..c659ac06f1d41d 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -132,6 +132,18 @@ MaybeLocal Message::Deserialize(Environment* env, // Attach all transferred ArrayBuffers to their new Isolate. for (uint32_t i = 0; i < array_buffer_contents_.size(); ++i) { + if (!env->isolate_data()->uses_node_allocator()) { + // We don't use Node's allocator on the receiving side, so we have + // to create the ArrayBuffer from a copy of the memory. + AllocatedBuffer buf = + env->AllocateManaged(array_buffer_contents_[i].size); + memcpy(buf.data(), + array_buffer_contents_[i].data, + array_buffer_contents_[i].size); + deserializer.TransferArrayBuffer(i, buf.ToArrayBuffer()); + continue; + } + Local ab = ArrayBuffer::New(env->isolate(), array_buffer_contents_[i].release(), @@ -288,8 +300,10 @@ Maybe Message::Serialize(Environment* env, Local ab = entry.As(); // If we cannot render the ArrayBuffer unusable in this Isolate and // take ownership of its memory, copying the buffer will have to do. - if (!ab->IsNeuterable() || ab->IsExternal()) + if (!ab->IsNeuterable() || ab->IsExternal() || + !env->isolate_data()->uses_node_allocator()) { continue; + } if (std::find(array_buffers.begin(), array_buffers.end(), ab) != array_buffers.end()) { ThrowDataCloneException( From b2f9bb18172a576d2c2996313f1decbbdc8a2cc0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 Feb 2019 17:30:10 +0100 Subject: [PATCH 4/8] src: add debugging array allocator Add a subclass of `ArrayBufferAllocator` that performs additional debug checking, which in particular verifies that: - All `ArrayBuffer` backing stores have been allocated with this allocator, or have been explicitly marked as coming from a compatible source. - All memory allocated by the allocator has been freed once it is destroyed. --- src/api/environment.cc | 76 +++++++++++++++++++++++++++++++++++++++++- src/node_internals.h | 23 +++++++++++++ src/node_options.cc | 4 +++ src/node_options.h | 1 + 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 7b6de8ba8dc367..6a56ddb5117958 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -75,8 +75,82 @@ void* ArrayBufferAllocator::Allocate(size_t size) { return UncheckedMalloc(size); } +DebuggingArrayBufferAllocator::~DebuggingArrayBufferAllocator() { + CHECK(allocations_.empty()); +} + +void* DebuggingArrayBufferAllocator::Allocate(size_t size) { + Mutex::ScopedLock lock(mutex_); + void* data = ArrayBufferAllocator::Allocate(size); + RegisterPointerInternal(data, size); + return data; +} + +void* DebuggingArrayBufferAllocator::AllocateUninitialized(size_t size) { + Mutex::ScopedLock lock(mutex_); + void* data = ArrayBufferAllocator::AllocateUninitialized(size); + RegisterPointerInternal(data, size); + return data; +} + +void DebuggingArrayBufferAllocator::Free(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + UnregisterPointerInternal(data, size); + ArrayBufferAllocator::Free(data, size); +} + +void* DebuggingArrayBufferAllocator::Reallocate(void* data, + size_t old_size, + size_t size) { + Mutex::ScopedLock lock(mutex_); + void* ret = ArrayBufferAllocator::Reallocate(data, old_size, size); + if (ret == nullptr) { + if (size == 0) // i.e. equivalent to free(). + UnregisterPointerInternal(data, old_size); + return nullptr; + } + + if (data != nullptr) { + auto it = allocations_.find(data); + CHECK_NE(it, allocations_.end()); + allocations_.erase(it); + } + + RegisterPointerInternal(ret, size); + return ret; +} + +void DebuggingArrayBufferAllocator::RegisterPointer(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + RegisterPointerInternal(data, size); +} + +void DebuggingArrayBufferAllocator::UnregisterPointer(void* data, size_t size) { + Mutex::ScopedLock lock(mutex_); + UnregisterPointerInternal(data, size); +} + +void DebuggingArrayBufferAllocator::UnregisterPointerInternal(void* data, + size_t size) { + if (data == nullptr) return; + auto it = allocations_.find(data); + CHECK_NE(it, allocations_.end()); + CHECK_EQ(it->second, size); + allocations_.erase(it); +} + +void DebuggingArrayBufferAllocator::RegisterPointerInternal(void* data, + size_t size) { + if (data == nullptr) return; + CHECK_EQ(allocations_.count(data), 0); + allocations_[data] = size; +} + ArrayBufferAllocator* CreateArrayBufferAllocator() { - return new ArrayBufferAllocator(); + if (per_process::cli_options->debug_arraybuffer_allocations) + return new DebuggingArrayBufferAllocator(); + else + return new ArrayBufferAllocator(); } void FreeArrayBufferAllocator(ArrayBufferAllocator* allocator) { diff --git a/src/node_internals.h b/src/node_internals.h index 6f09708371b97c..9ac2b0a331c531 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -109,11 +109,34 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { void* AllocateUninitialized(size_t size) override { return node::UncheckedMalloc(size); } void Free(void* data, size_t) override { free(data); } + virtual void* Reallocate(void* data, size_t old_size, size_t size) { + return static_cast( + UncheckedRealloc(static_cast(data), size)); + } + virtual void RegisterPointer(void* data, size_t size) {} + virtual void UnregisterPointer(void* data, size_t size) {} private: uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land. }; +class DebuggingArrayBufferAllocator final : public ArrayBufferAllocator { + public: + ~DebuggingArrayBufferAllocator() override; + void* Allocate(size_t size) override; + void* AllocateUninitialized(size_t size) override; + void Free(void* data, size_t size) override; + void* Reallocate(void* data, size_t old_size, size_t size) override; + void RegisterPointer(void* data, size_t size) override; + void UnregisterPointer(void* data, size_t size) override; + + private: + void RegisterPointerInternal(void* data, size_t size); + void UnregisterPointerInternal(void* data, size_t size); + Mutex mutex_; + std::unordered_map allocations_; +}; + namespace Buffer { v8::MaybeLocal Copy(Environment* env, const char* data, size_t len); v8::MaybeLocal New(Environment* env, size_t size); diff --git a/src/node_options.cc b/src/node_options.cc index 937298e0d833df..bdd39d5d71fe90 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -387,6 +387,10 @@ PerProcessOptionsParser::PerProcessOptionsParser() { "SlowBuffer instances", &PerProcessOptions::zero_fill_all_buffers, kAllowedInEnvironment); + AddOption("--debug-arraybuffer-allocations", + "", /* undocumented, only for debugging */ + &PerProcessOptions::debug_arraybuffer_allocations, + kAllowedInEnvironment); AddOption("--security-reverts", "", &PerProcessOptions::security_reverts); AddOption("--completion-bash", diff --git a/src/node_options.h b/src/node_options.h index fe2e1034c937aa..9313c4a5384662 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -158,6 +158,7 @@ class PerProcessOptions : public Options { uint64_t max_http_header_size = 8 * 1024; int64_t v8_thread_pool_size = 4; bool zero_fill_all_buffers = false; + bool debug_arraybuffer_allocations = false; std::vector security_reverts; bool print_bash_completion = false; From 41e81b1d063ba8fbe446701a96c839cfb122d0e1 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 Feb 2019 22:58:19 +0100 Subject: [PATCH 5/8] src: add allocation utils to env Add a RAII utility for managing blocks of memory that have been allocated with the `ArrayBuffer::Allocator` for a given `Isolate`. --- src/env-inl.h | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/env.cc | 18 ++++++++++ src/env.h | 39 ++++++++++++++++++++ 3 files changed, 156 insertions(+) diff --git a/src/env-inl.h b/src/env-inl.h index 51c7e0d7b06561..267567d80cdfaa 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -715,6 +715,105 @@ inline IsolateData* Environment::isolate_data() const { return isolate_data_; } +inline char* Environment::AllocateUnchecked(size_t size) { + return static_cast( + isolate_data()->allocator()->AllocateUninitialized(size)); +} + +inline char* Environment::Allocate(size_t size) { + char* ret = AllocateUnchecked(size); + CHECK_NE(ret, nullptr); + return ret; +} + +inline void Environment::Free(char* data, size_t size) { + if (data != nullptr) + isolate_data()->allocator()->Free(data, size); +} + +inline AllocatedBuffer Environment::AllocateManaged(size_t size, bool checked) { + char* data = checked ? Allocate(size) : AllocateUnchecked(size); + if (data == nullptr) size = 0; + return AllocatedBuffer(this, uv_buf_init(data, size)); +} + +inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf) + : env_(env), buffer_(buf) {} + +inline void AllocatedBuffer::Resize(size_t len) { + char* new_data = env_->Reallocate(buffer_.base, buffer_.len, len); + CHECK_IMPLIES(len > 0, new_data != nullptr); + buffer_.base = new_data; + buffer_.len = len; +} + +inline uv_buf_t AllocatedBuffer::release() { + uv_buf_t ret = buffer_; + buffer_ = uv_buf_init(nullptr, 0); + return ret; +} + +inline char* AllocatedBuffer::data() { + return buffer_.base; +} + +inline const char* AllocatedBuffer::data() const { + return buffer_.base; +} + +inline size_t AllocatedBuffer::size() const { + return buffer_.len; +} + +inline AllocatedBuffer::AllocatedBuffer(Environment* env) + : env_(env), buffer_(uv_buf_init(nullptr, 0)) {} + +inline AllocatedBuffer::AllocatedBuffer(AllocatedBuffer&& other) + : AllocatedBuffer() { + *this = std::move(other); +} + +inline AllocatedBuffer& AllocatedBuffer::operator=(AllocatedBuffer&& other) { + clear(); + env_ = other.env_; + buffer_ = other.release(); + return *this; +} + +inline AllocatedBuffer::~AllocatedBuffer() { + clear(); +} + +inline void AllocatedBuffer::clear() { + uv_buf_t buf = release(); + env_->Free(buf.base, buf.len); +} + +// It's a bit awkward to define this Buffer::New() overload here, but it +// avoids a circular dependency with node_internals.h. +namespace Buffer { +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc); +} + +inline v8::MaybeLocal AllocatedBuffer::ToBuffer() { + CHECK_NOT_NULL(env_); + v8::MaybeLocal obj = Buffer::New(env_, data(), size(), false); + if (!obj.IsEmpty()) release(); + return obj; +} + +inline v8::Local AllocatedBuffer::ToArrayBuffer() { + CHECK_NOT_NULL(env_); + uv_buf_t buf = release(); + return v8::ArrayBuffer::New(env_->isolate(), + buf.base, + buf.len, + v8::ArrayBufferCreationMode::kInternalized); +} + inline void Environment::ThrowError(const char* errmsg) { ThrowError(v8::Exception::Error, errmsg); } diff --git a/src/env.cc b/src/env.cc index b7b6d745d8a231..577d2abdcee959 100644 --- a/src/env.cc +++ b/src/env.cc @@ -22,6 +22,7 @@ namespace node { using errors::TryCatchScope; using v8::Boolean; +using v8::ArrayBuffer; using v8::Context; using v8::EmbedderGraph; using v8::External; @@ -905,6 +906,23 @@ void Environment::BuildEmbedderGraph(Isolate* isolate, }); } +char* Environment::Reallocate(char* data, size_t old_size, size_t size) { + // If we know that the allocator is our ArrayBufferAllocator, we can let + // if reallocate directly. + if (isolate_data()->uses_node_allocator()) { + return static_cast( + isolate_data()->node_allocator()->Reallocate(data, old_size, size)); + } + // Generic allocators do not provide a reallocation method; we need to + // allocate a new chunk of memory and copy the data over. + char* new_data = AllocateUnchecked(size); + if (new_data == nullptr) return nullptr; + memcpy(new_data, data, std::min(size, old_size)); + if (size > old_size) + memset(new_data + old_size, 0, size - old_size); + Free(data, old_size); + return new_data; +} // Not really any better place than env.cc at this moment. void BaseObject::DeleteMe(void* data) { diff --git a/src/env.h b/src/env.h index 527a28f6957fd8..b2e4b2bc7e4079 100644 --- a/src/env.h +++ b/src/env.h @@ -476,6 +476,36 @@ enum class DebugCategory { CATEGORY_COUNT }; +// A unique-pointer-ish object that is compatible with the JS engine's +// ArrayBuffer::Allocator. +struct AllocatedBuffer { + public: + explicit inline AllocatedBuffer(Environment* env = nullptr); + inline AllocatedBuffer(Environment* env, uv_buf_t buf); + inline ~AllocatedBuffer(); + inline void Resize(size_t len); + + inline uv_buf_t release(); + inline char* data(); + inline const char* data() const; + inline size_t size() const; + inline void clear(); + + inline v8::MaybeLocal ToBuffer(); + inline v8::Local ToArrayBuffer(); + + inline AllocatedBuffer(AllocatedBuffer&& other); + inline AllocatedBuffer& operator=(AllocatedBuffer&& other); + AllocatedBuffer(const AllocatedBuffer& other) = delete; + AllocatedBuffer& operator=(const AllocatedBuffer& other) = delete; + + private: + Environment* env_; + uv_buf_t buffer_; + + friend class Environment; +}; + class Environment { public: class AsyncHooks { @@ -697,6 +727,15 @@ class Environment { inline IsolateData* isolate_data() const; + // Utilites that allocate memory using the Isolate's ArrayBuffer::Allocator. + // In particular, using AllocateManaged() will provide a RAII-style object + // with easy conversion to `Buffer` and `ArrayBuffer` objects. + inline AllocatedBuffer AllocateManaged(size_t size, bool checked = true); + inline char* Allocate(size_t size); + inline char* AllocateUnchecked(size_t size); + char* Reallocate(char* data, size_t old_size, size_t size); + inline void Free(char* data, size_t size); + inline bool printed_error() const; inline void set_printed_error(bool value); From 1e17828cb28939c4ee3e4045f9fb86f182059e14 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 21 Feb 2019 15:56:03 +0100 Subject: [PATCH 6/8] fixup! src: add allocation utils to env --- src/env-inl.h | 3 +-- src/env.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/env-inl.h b/src/env-inl.h index 267567d80cdfaa..63b71daf15a245 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -743,8 +743,7 @@ inline AllocatedBuffer::AllocatedBuffer(Environment* env, uv_buf_t buf) inline void AllocatedBuffer::Resize(size_t len) { char* new_data = env_->Reallocate(buffer_.base, buffer_.len, len); CHECK_IMPLIES(len > 0, new_data != nullptr); - buffer_.base = new_data; - buffer_.len = len; + buffer_ = uv_buf_init(new_data, len); } inline uv_buf_t AllocatedBuffer::release() { diff --git a/src/env.h b/src/env.h index b2e4b2bc7e4079..3856f5241d63b6 100644 --- a/src/env.h +++ b/src/env.h @@ -501,6 +501,8 @@ struct AllocatedBuffer { private: Environment* env_; + // We do not pass this to libuv directly, but uv_buf_t is a convenient way + // to represent a chunk of memory, and plays nicely with other parts of core. uv_buf_t buffer_; friend class Environment; From 5854771285dc4060a4721832dc4f1eb234347ef6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 21 Feb 2019 16:00:58 +0100 Subject: [PATCH 7/8] fixup! src: add allocation utils to env --- src/env.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/env.cc b/src/env.cc index 577d2abdcee959..fce949f63a8481 100644 --- a/src/env.cc +++ b/src/env.cc @@ -21,8 +21,8 @@ namespace node { using errors::TryCatchScope; -using v8::Boolean; using v8::ArrayBuffer; +using v8::Boolean; using v8::Context; using v8::EmbedderGraph; using v8::External; From a55092f48f958a62050c14c1ce761b798cb8504a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 18 Feb 2019 22:58:27 +0100 Subject: [PATCH 8/8] src: allocate Buffer memory using ArrayBuffer allocator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always use the right allocator for memory that is turned into an `ArrayBuffer` at a later point. This enables embedders to use their own `ArrayBuffer::Allocator`s, and is inspired by Electron’s electron/node@f61bae3440e. It should render their downstream patch unnecessary. Refs: https://github.com/electron/node/commit/f61bae3440e1bfcc83bba6ff0785adfb89b4045e --- src/node_buffer.cc | 98 +++++-------- src/node_crypto.cc | 283 ++++++++++++++++-------------------- src/node_crypto.h | 15 +- src/node_http2.cc | 23 ++- src/node_http2.h | 1 + src/node_http_parser_impl.h | 5 +- src/node_internals.h | 12 +- src/node_messaging.cc | 8 + src/node_native_module.cc | 10 +- src/node_serdes.cc | 5 +- src/stream_base-inl.h | 15 +- src/stream_base.cc | 47 +++--- src/stream_base.h | 14 +- src/stream_pipe.cc | 15 +- src/stream_pipe.h | 2 +- src/udp_wrap.cc | 22 ++- 16 files changed, 256 insertions(+), 319 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 9fed15622e05d5..aca1842c30b37b 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -54,17 +54,6 @@ size_t length = end - start; namespace node { - -namespace { - -inline void* BufferMalloc(size_t length) { - return per_process::cli_options->zero_fill_all_buffers ? - node::UncheckedCalloc(length) : - node::UncheckedMalloc(length); -} - -} // namespace - namespace Buffer { using v8::ArrayBuffer; @@ -260,7 +249,7 @@ MaybeLocal New(Isolate* isolate, char* data = nullptr; if (length > 0) { - data = static_cast(BufferMalloc(length)); + data = UncheckedMalloc(length); if (data == nullptr) { THROW_ERR_MEMORY_ALLOCATION_FAILED(isolate); @@ -278,13 +267,7 @@ MaybeLocal New(Isolate* isolate, } } - Local buf; - if (New(isolate, data, actual).ToLocal(&buf)) - return scope.Escape(buf); - - // Object failed to be created. Clean up resources. - free(data); - return Local(); + return scope.EscapeMaybe(New(isolate, data, actual)); } @@ -311,26 +294,16 @@ MaybeLocal New(Environment* env, size_t length) { return Local(); } - void* data; + AllocatedBuffer ret(env); if (length > 0) { - data = BufferMalloc(length); - if (data == nullptr) { + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { THROW_ERR_MEMORY_ALLOCATION_FAILED(env); return Local(); } - } else { - data = nullptr; } - Local ab = - ArrayBuffer::New(env->isolate(), - data, - length, - ArrayBufferCreationMode::kInternalized); - Local obj; - if (Buffer::New(env, ab, 0, length).ToLocal(&obj)) - return scope.Escape(obj); - return Local(); + return scope.EscapeMaybe(ret.ToBuffer()); } @@ -357,28 +330,18 @@ MaybeLocal Copy(Environment* env, const char* data, size_t length) { return Local(); } - void* new_data; + AllocatedBuffer ret(env); if (length > 0) { CHECK_NOT_NULL(data); - new_data = node::UncheckedMalloc(length); - if (new_data == nullptr) { + ret = env->AllocateManaged(length, false); + if (ret.data() == nullptr) { THROW_ERR_MEMORY_ALLOCATION_FAILED(env); return Local(); } - memcpy(new_data, data, length); - } else { - new_data = nullptr; + memcpy(ret.data(), data, length); } - Local ab = - ArrayBuffer::New(env->isolate(), - new_data, - length, - ArrayBufferCreationMode::kInternalized); - Local obj; - if (Buffer::New(env, ab, 0, length).ToLocal(&obj)) - return scope.Escape(obj); - return Local(); + return scope.EscapeMaybe(ret.ToBuffer()); } @@ -425,7 +388,8 @@ MaybeLocal New(Environment* env, return scope.Escape(ui.ToLocalChecked()); } - +// Warning: This function needs `data` to be allocated with malloc() and not +// necessarily isolate's ArrayBuffer::Allocator. MaybeLocal New(Isolate* isolate, char* data, size_t length) { EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); @@ -435,18 +399,37 @@ MaybeLocal New(Isolate* isolate, char* data, size_t length) { return MaybeLocal(); } Local obj; - if (Buffer::New(env, data, length).ToLocal(&obj)) + if (Buffer::New(env, data, length, true).ToLocal(&obj)) return handle_scope.Escape(obj); return Local(); } - -MaybeLocal New(Environment* env, char* data, size_t length) { +// Warning: If this call comes through the public node_buffer.h API, +// the contract for this function is that `data` is allocated with malloc() +// and not necessarily isolate's ArrayBuffer::Allocator. +MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc) { if (length > 0) { CHECK_NOT_NULL(data); CHECK(length <= kMaxLength); } + if (uses_malloc) { + if (env->isolate_data()->uses_node_allocator()) { + // We don't know for sure that the allocator is malloc()-based, so we need + // to fall back to the FreeCallback variant. + auto free_callback = [](char* data, void* hint) { free(data); }; + return New(env, data, length, free_callback, nullptr); + } else { + // This is malloc()-based, so we can acquire it into our own + // ArrayBufferAllocator. + CHECK_NOT_NULL(env->isolate_data()->node_allocator()); + env->isolate_data()->node_allocator()->RegisterPointer(data, length); + } + } + Local ab = ArrayBuffer::New(env->isolate(), data, @@ -1053,15 +1036,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo& args) { Local str = args[0].As(); size_t length = str->Utf8Length(isolate); - char* data = node::UncheckedMalloc(length); + AllocatedBuffer buf = env->AllocateManaged(length); str->WriteUtf8(isolate, - data, + buf.data(), -1, // We are certain that `data` is sufficiently large nullptr, String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - auto array_buf = ArrayBuffer::New( - isolate, data, length, ArrayBufferCreationMode::kInternalized); - auto array = Uint8Array::New(array_buf, 0, length); + auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length); args.GetReturnValue().Set(array); } @@ -1123,7 +1104,8 @@ void Initialize(Local target, // It can be a nullptr when running inside an isolate where we // do not own the ArrayBuffer allocator. - if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) { + if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) { + uint32_t* zero_fill_field = allocator->zero_fill_field(); Local array_buffer = ArrayBuffer::New( env->isolate(), zero_fill_field, sizeof(*zero_fill_field)); CHECK(target diff --git a/src/node_crypto.cc b/src/node_crypto.cc index fcc35ce63fdd9a..7e5d7ac0619ca2 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -52,15 +52,6 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL | XN_FLAG_FN_SN; namespace node { -namespace Buffer { -// OpenSSL uses `unsigned char*` for raw data, make this easier for us. -v8::MaybeLocal New(Environment* env, unsigned char* udata, - size_t length) { - char* data = reinterpret_cast(udata); - return Buffer::New(env, data, length); -} -} // namespace Buffer - namespace crypto { using node::THROW_ERR_TLS_INVALID_PROTOCOL_METHOD; @@ -1651,13 +1642,18 @@ static MaybeLocal ECPointToBuffer(Environment* env, if (error != nullptr) *error = "Failed to get public key length"; return MaybeLocal(); } - MallocedBuffer buf(len); - len = EC_POINT_point2oct(group, point, form, buf.data, buf.size, nullptr); + AllocatedBuffer buf = env->AllocateManaged(len); + len = EC_POINT_point2oct(group, + point, + form, + reinterpret_cast(buf.data()), + buf.size(), + nullptr); if (len == 0) { if (error != nullptr) *error = "Failed to get public key"; return MaybeLocal(); } - return Buffer::New(env, buf.release(), len); + return buf.ToBuffer(); } @@ -2036,9 +2032,9 @@ void SSLWrap::GetFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -2059,9 +2055,9 @@ void SSLWrap::GetPeerFinished(const FunctionCallbackInfo& args) { if (len == 0) return; - char* buf = Malloc(len); - CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf, len)); - args.GetReturnValue().Set(Buffer::New(env, buf, len).ToLocalChecked()); + AllocatedBuffer buf = env->AllocateManaged(len); + CHECK_EQ(len, SSL_get_peer_finished(w->ssl_.get(), buf.data(), len)); + args.GetReturnValue().Set(buf.ToBuffer().ToLocalChecked()); } @@ -2079,10 +2075,10 @@ void SSLWrap::GetSession(const FunctionCallbackInfo& args) { int slen = i2d_SSL_SESSION(sess, nullptr); CHECK_GT(slen, 0); - char* sbuf = Malloc(slen); - unsigned char* p = reinterpret_cast(sbuf); + AllocatedBuffer sbuf = env->AllocateManaged(slen); + unsigned char* p = reinterpret_cast(sbuf.data()); i2d_SSL_SESSION(sess, &p); - args.GetReturnValue().Set(Buffer::New(env, sbuf, slen).ToLocalChecked()); + args.GetReturnValue().Set(sbuf.ToBuffer().ToLocalChecked()); } @@ -3936,11 +3932,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - CipherBase::UpdateResult CipherBase::Update(const char* data, int len, - unsigned char** out, - int* out_len) { + AllocatedBuffer* out) { if (!ctx_) return kErrorState; MarkPopErrorOnReturn mark_pop_error_on_return; @@ -3958,27 +3952,27 @@ CipherBase::UpdateResult CipherBase::Update(const char* data, CHECK(MaybePassAuthTagToOpenSSL()); } - *out_len = 0; - int buff_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); + int buf_len = len + EVP_CIPHER_CTX_block_size(ctx_.get()); // For key wrapping algorithms, get output size by calling // EVP_CipherUpdate() with null output. if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE && EVP_CipherUpdate(ctx_.get(), nullptr, - &buff_len, + &buf_len, reinterpret_cast(data), len) != 1) { return kErrorState; } - *out = Malloc(buff_len); + *out = env()->AllocateManaged(buf_len); int r = EVP_CipherUpdate(ctx_.get(), - *out, - out_len, + reinterpret_cast(out->data()), + &buf_len, reinterpret_cast(data), len); - CHECK_LE(*out_len, buff_len); + CHECK_LE(static_cast(buf_len), out->size()); + out->Resize(buf_len); // When in CCM mode, EVP_CipherUpdate will fail if the authentication tag is // invalid. In that case, remember the error and throw in final(). @@ -3996,9 +3990,8 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { CipherBase* cipher; ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); - unsigned char* out = nullptr; + AllocatedBuffer out; UpdateResult r; - int out_len = 0; // Only copy the data if we have to, because it's a string if (args[0]->IsString()) { @@ -4006,15 +3999,14 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { if (!decoder.Decode(env, args[0].As(), args[1], UTF8) .FromMaybe(false)) return; - r = cipher->Update(decoder.out(), decoder.size(), &out, &out_len); + r = cipher->Update(decoder.out(), decoder.size(), &out); } else { char* buf = Buffer::Data(args[0]); size_t buflen = Buffer::Length(args[0]); - r = cipher->Update(buf, buflen, &out, &out_len); + r = cipher->Update(buf, buflen, &out); } if (r != kSuccess) { - free(out); if (r == kErrorState) { ThrowCryptoError(env, ERR_get_error(), "Trying to add data in unsupported state"); @@ -4022,11 +4014,9 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { return; } - CHECK(out != nullptr || out_len == 0); - Local buf = - Buffer::New(env, reinterpret_cast(out), out_len).ToLocalChecked(); + CHECK(out.data() != nullptr || out.size() == 0); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4046,14 +4036,13 @@ void CipherBase::SetAutoPadding(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(b); // Possibly report invalid state failure } - -bool CipherBase::Final(unsigned char** out, int* out_len) { +bool CipherBase::Final(AllocatedBuffer* out) { if (!ctx_) return false; const int mode = EVP_CIPHER_CTX_mode(ctx_.get()); - *out = Malloc( + *out = env()->AllocateManaged( static_cast(EVP_CIPHER_CTX_block_size(ctx_.get()))); if (kind_ == kDecipher && IsSupportedAuthenticatedMode(ctx_.get())) { @@ -4065,8 +4054,17 @@ bool CipherBase::Final(unsigned char** out, int* out_len) { bool ok; if (kind_ == kDecipher && mode == EVP_CIPH_CCM_MODE) { ok = !pending_auth_failed_; + *out = AllocatedBuffer(env()); // Empty buffer. } else { - ok = EVP_CipherFinal_ex(ctx_.get(), *out, out_len) == 1; + int out_len = out->size(); + ok = EVP_CipherFinal_ex(ctx_.get(), + reinterpret_cast(out->data()), + &out_len) == 1; + + if (out_len >= 0) + out->Resize(out_len); + else + *out = AllocatedBuffer(); // *out will not be used. if (ok && kind_ == kCipher && IsAuthenticatedMode()) { // In GCM mode, the authentication tag length can be specified in advance, @@ -4095,33 +4093,21 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&cipher, args.Holder()); if (cipher->ctx_ == nullptr) return env->ThrowError("Unsupported state"); - unsigned char* out_value = nullptr; - int out_len = -1; + AllocatedBuffer out; // Check IsAuthenticatedMode() first, Final() destroys the EVP_CIPHER_CTX. const bool is_auth_mode = cipher->IsAuthenticatedMode(); - bool r = cipher->Final(&out_value, &out_len); - - if (out_len <= 0 || !r) { - free(out_value); - out_value = nullptr; - out_len = 0; - if (!r) { - const char* msg = is_auth_mode ? - "Unsupported state or unable to authenticate data" : - "Unsupported state"; - - return ThrowCryptoError(env, - ERR_get_error(), - msg); - } + bool r = cipher->Final(&out); + + if (!r) { + const char* msg = is_auth_mode + ? "Unsupported state or unable to authenticate data" + : "Unsupported state"; + + return ThrowCryptoError(env, ERR_get_error(), msg); } - Local buf = Buffer::New( - env, - reinterpret_cast(out_value), - out_len).ToLocalChecked(); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4481,20 +4467,21 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { sign->CheckThrow(err); } -static MallocedBuffer Node_SignFinal(EVPMDPointer&& mdctx, - const ManagedEVPPKey& pkey, - int padding, - int pss_salt_len) { +static AllocatedBuffer Node_SignFinal(Environment* env, + EVPMDPointer&& mdctx, + const ManagedEVPPKey& pkey, + int padding, + int pss_salt_len) { unsigned char m[EVP_MAX_MD_SIZE]; unsigned int m_len; if (!EVP_DigestFinal_ex(mdctx.get(), m, &m_len)) - return MallocedBuffer(); + return AllocatedBuffer(); int signed_sig_len = EVP_PKEY_size(pkey.get()); CHECK_GE(signed_sig_len, 0); size_t sig_len = static_cast(signed_sig_len); - MallocedBuffer sig(sig_len); + AllocatedBuffer sig = env->AllocateManaged(sig_len); EVPKeyCtxPointer pkctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); if (pkctx && @@ -4502,12 +4489,16 @@ static MallocedBuffer Node_SignFinal(EVPMDPointer&& mdctx, ApplyRSAOptions(pkey, pkctx.get(), padding, pss_salt_len) && EVP_PKEY_CTX_set_signature_md(pkctx.get(), EVP_MD_CTX_md(mdctx.get())) > 0 && - EVP_PKEY_sign(pkctx.get(), sig.data, &sig_len, m, m_len) > 0) { - sig.Truncate(sig_len); + EVP_PKEY_sign(pkctx.get(), + reinterpret_cast(sig.data()), + &sig_len, + m, + m_len) > 0) { + sig.Resize(sig_len); return sig; } - return MallocedBuffer(); + return AllocatedBuffer(); } Sign::SignResult Sign::SignFinal( @@ -4546,16 +4537,14 @@ Sign::SignResult Sign::SignFinal( } #endif // NODE_FIPS_MODE - MallocedBuffer buffer = - Node_SignFinal(std::move(mdctx), pkey, padding, salt_len); - Error error = buffer.is_empty() ? kSignPrivateKey : kSignOk; + AllocatedBuffer buffer = + Node_SignFinal(env(), std::move(mdctx), pkey, padding, salt_len); + Error error = buffer.data() == nullptr ? kSignPrivateKey : kSignOk; return SignResult(error, std::move(buffer)); } void Sign::SignFinal(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Sign* sign; ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder()); @@ -4580,13 +4569,7 @@ void Sign::SignFinal(const FunctionCallbackInfo& args) { if (ret.error != kSignOk) return sign->CheckThrow(ret.error); - MallocedBuffer sig = - std::move(ret.signature); - - Local rc = - Buffer::New(env, reinterpret_cast(sig.release()), sig.size) - .ToLocalChecked(); - args.GetReturnValue().Set(rc); + args.GetReturnValue().Set(ret.signature.ToBuffer().ToLocalChecked()); } void Verify::Initialize(Environment* env, Local target) { @@ -4693,16 +4676,15 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - template -bool PublicKeyCipher::Cipher(const ManagedEVPPKey& pkey, +bool PublicKeyCipher::Cipher(Environment* env, + const ManagedEVPPKey& pkey, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len) { + AllocatedBuffer* out) { EVPKeyCtxPointer ctx(EVP_PKEY_CTX_new(pkey.get(), nullptr)); if (!ctx) return false; @@ -4711,14 +4693,21 @@ bool PublicKeyCipher::Cipher(const ManagedEVPPKey& pkey, if (EVP_PKEY_CTX_set_rsa_padding(ctx.get(), padding) <= 0) return false; - if (EVP_PKEY_cipher(ctx.get(), nullptr, out_len, data, len) <= 0) + size_t out_len = 0; + if (EVP_PKEY_cipher(ctx.get(), nullptr, &out_len, data, len) <= 0) return false; - *out = Malloc(*out_len); + *out = env->AllocateManaged(out_len); - if (EVP_PKEY_cipher(ctx.get(), *out, out_len, data, len) <= 0) + if (EVP_PKEY_cipher(ctx.get(), + reinterpret_cast(out->data()), + &out_len, + data, + len) <= 0) { return false; + } + out->Resize(out_len); return true; } @@ -4741,33 +4730,22 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { uint32_t padding; if (!args[offset + 1]->Uint32Value(env->context()).To(&padding)) return; - unsigned char* out_value = nullptr; - size_t out_len = 0; + AllocatedBuffer out; ClearErrorOnReturn clear_error_on_return; bool r = Cipher( + env, pkey, padding, reinterpret_cast(buf), len, - &out_value, - &out_len); - - if (out_len == 0 || !r) { - free(out_value); - out_value = nullptr; - out_len = 0; - if (!r) { - return ThrowCryptoError(env, - ERR_get_error()); - } - } + &out); - Local vbuf = - Buffer::New(env, reinterpret_cast(out_value), out_len) - .ToLocalChecked(); - args.GetReturnValue().Set(vbuf); + if (!r) + return ThrowCryptoError(env, ERR_get_error()); + + args.GetReturnValue().Set(out.ToBuffer().ToLocalChecked()); } @@ -4932,10 +4910,11 @@ void DiffieHellman::GenerateKeys(const FunctionCallbackInfo& args) { DH_get0_key(diffieHellman->dh_.get(), &pub_key, nullptr); const int size = BN_num_bytes(pub_key); CHECK_GE(size, 0); - char* data = Malloc(size); + AllocatedBuffer data = env->AllocateManaged(size); CHECK_EQ(size, - BN_bn2binpad(pub_key, reinterpret_cast(data), size)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + BN_bn2binpad( + pub_key, reinterpret_cast(data.data()), size)); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } @@ -4952,10 +4931,11 @@ void DiffieHellman::GetField(const FunctionCallbackInfo& args, const int size = BN_num_bytes(num); CHECK_GE(size, 0); - char* data = Malloc(size); - CHECK_EQ(size, - BN_bn2binpad(num, reinterpret_cast(data), size)); - args.GetReturnValue().Set(Buffer::New(env, data, size).ToLocalChecked()); + AllocatedBuffer data = env->AllocateManaged(size); + CHECK_EQ( + size, + BN_bn2binpad(num, reinterpret_cast(data.data()), size)); + args.GetReturnValue().Set(data.ToBuffer().ToLocalChecked()); } void DiffieHellman::GetPrime(const FunctionCallbackInfo& args) { @@ -5013,9 +4993,9 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { Buffer::Length(args[0]), nullptr)); - MallocedBuffer data(DH_size(diffieHellman->dh_.get())); + AllocatedBuffer ret = env->AllocateManaged(DH_size(diffieHellman->dh_.get())); - int size = DH_compute_key(reinterpret_cast(data.data), + int size = DH_compute_key(reinterpret_cast(ret.data()), key.get(), diffieHellman->dh_.get()); @@ -5050,14 +5030,13 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { // DH_compute_key returns number of bytes in a remainder of exponent, which // may have less bytes than a prime number. Therefore add 0-padding to the // allocated buffer. - if (static_cast(size) != data.size) { - CHECK_GT(data.size, static_cast(size)); - memmove(data.data + data.size - size, data.data, size); - memset(data.data, 0, data.size - size); + if (static_cast(size) != ret.size()) { + CHECK_GT(ret.size(), static_cast(size)); + memmove(ret.data() + ret.size() - size, ret.data(), size); + memset(ret.data(), 0, ret.size() - size); } - args.GetReturnValue().Set( - Buffer::New(env->isolate(), data.release(), data.size).ToLocalChecked()); + args.GetReturnValue().Set(ret.ToBuffer().ToLocalChecked()); } void DiffieHellman::SetKey(const FunctionCallbackInfo& args, @@ -5231,15 +5210,14 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { // NOTE: field_size is in bits int field_size = EC_GROUP_get_degree(ecdh->group_); size_t out_len = (field_size + 7) / 8; - char* out = node::Malloc(out_len); + AllocatedBuffer out = env->AllocateManaged(out_len); - int r = ECDH_compute_key(out, out_len, pub.get(), ecdh->key_.get(), nullptr); - if (!r) { - free(out); + int r = ECDH_compute_key( + out.data(), out_len, pub.get(), ecdh->key_.get(), nullptr); + if (!r) return env->ThrowError("Failed to compute ECDH key"); - } - Local buf = Buffer::New(env, out, out_len).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -5281,11 +5259,12 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get ECDH private key"); const int size = BN_num_bytes(b); - unsigned char* out = node::Malloc(size); - CHECK_EQ(size, BN_bn2binpad(b, out, size)); + AllocatedBuffer out = env->AllocateManaged(size); + CHECK_EQ(size, BN_bn2binpad(b, + reinterpret_cast(out.data()), + size)); - Local buf = - Buffer::New(env, reinterpret_cast(out), size).ToLocalChecked(); + Local buf = out.ToBuffer().ToLocalChecked(); args.GetReturnValue().Set(buf); } @@ -6037,31 +6016,28 @@ void VerifySpkac(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(verify_result); } - -char* ExportPublicKey(const char* data, int len, size_t* size) { - char* buf = nullptr; - +AllocatedBuffer ExportPublicKey(Environment* env, + const char* data, + int len, + size_t* size) { BIOPointer bio(BIO_new(BIO_s_mem())); - if (!bio) - return nullptr; + if (!bio) return AllocatedBuffer(); NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); - if (!spki) - return nullptr; + if (!spki) return AllocatedBuffer(); EVPKeyPointer pkey(NETSCAPE_SPKI_get_pubkey(spki.get())); - if (!pkey) - return nullptr; + if (!pkey) return AllocatedBuffer(); if (PEM_write_bio_PUBKEY(bio.get(), pkey.get()) <= 0) - return nullptr; + return AllocatedBuffer(); BUF_MEM* ptr; BIO_get_mem_ptr(bio.get(), &ptr); *size = ptr->length; - buf = Malloc(*size); - memcpy(buf, ptr->data, *size); + AllocatedBuffer buf = env->AllocateManaged(*size); + memcpy(buf.data(), ptr->data, *size); return buf; } @@ -6078,12 +6054,11 @@ void ExportPublicKey(const FunctionCallbackInfo& args) { CHECK_NOT_NULL(data); size_t pkey_size; - char* pkey = ExportPublicKey(data, length, &pkey_size); - if (pkey == nullptr) + AllocatedBuffer pkey = ExportPublicKey(env, data, length, &pkey_size); + if (pkey.data() == nullptr) return args.GetReturnValue().SetEmptyString(); - Local out = Buffer::New(env, pkey, pkey_size).ToLocalChecked(); - args.GetReturnValue().Set(out); + args.GetReturnValue().Set(pkey.ToBuffer().ToLocalChecked()); } diff --git a/src/node_crypto.h b/src/node_crypto.h index 7c346a6c1435d1..2181e8b39ccd45 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -543,9 +543,8 @@ class CipherBase : public BaseObject { bool InitAuthenticated(const char* cipher_type, int iv_len, unsigned int auth_tag_len); bool CheckCCMMessageLength(int message_len); - UpdateResult Update(const char* data, int len, unsigned char** out, - int* out_len); - bool Final(unsigned char** out, int* out_len); + UpdateResult Update(const char* data, int len, AllocatedBuffer* out); + bool Final(AllocatedBuffer* out); bool SetAutoPadding(bool auto_padding); bool IsAuthenticatedMode() const; @@ -676,11 +675,11 @@ class Sign : public SignBase { struct SignResult { Error error; - MallocedBuffer signature; + AllocatedBuffer signature; explicit SignResult( Error err, - MallocedBuffer&& sig = MallocedBuffer()) + AllocatedBuffer&& sig = AllocatedBuffer()) : error(err), signature(std::move(sig)) {} }; @@ -737,12 +736,12 @@ class PublicKeyCipher { template - static bool Cipher(const ManagedEVPPKey& pkey, + static bool Cipher(Environment* env, + const ManagedEVPPKey& pkey, int padding, const unsigned char* data, int len, - unsigned char** out, - size_t* out_len); + AllocatedBuffer* out); template AllocateManaged(suggested_size).release(); +} + // Callback used to receive inbound data from the i/o stream -void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { HandleScope handle_scope(env()->isolate()); Context::Scope context_scope(env()->context()); Http2Scope h2scope(this); CHECK_NOT_NULL(stream_); Debug(this, "receiving %d bytes", nread); CHECK(stream_buf_ab_.IsEmpty()); + AllocatedBuffer buf(env(), buf_); // Only pass data on if nread > 0 if (nread <= 0) { - free(buf.base); if (nread < 0) { PassReadErrorToPreviousListener(nread); } @@ -1786,13 +1789,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { } // Shrink to the actual amount of used data. - char* base = Realloc(buf.base, nread); + buf.Resize(nread); - IncrementCurrentSessionMemory(nread); + IncrementCurrentSessionMemory(buf.size()); OnScopeLeave on_scope_leave([&]() { // Once finished handling this write, reset the stream buffer. // The memory has either been free()d or was handed over to V8. - DecrementCurrentSessionMemory(nread); + DecrementCurrentSessionMemory(buf.size()); stream_buf_ab_ = Local(); stream_buf_ = uv_buf_init(nullptr, 0); }); @@ -1803,17 +1806,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { // Remember the current buffer, so that OnDataChunkReceived knows the // offset of a DATA frame's data into the socket read buffer. - stream_buf_ = uv_buf_init(base, nread); + stream_buf_ = uv_buf_init(buf.data(), nread); Isolate* isolate = env()->isolate(); // Create an array buffer for the read data. DATA frames will be emitted // as slices of this array buffer to avoid having to copy memory. - stream_buf_ab_ = - ArrayBuffer::New(isolate, - base, - nread, - ArrayBufferCreationMode::kInternalized); + stream_buf_ab_ = buf.ToArrayBuffer(); statistics_.data_received += nread; ssize_t ret = Write(&stream_buf_, 1); diff --git a/src/node_http2.h b/src/node_http2.h index 660a713d19e779..aa953667facccf 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -783,6 +783,7 @@ class Http2Session : public AsyncWrap, public StreamListener { } // Handle reads/writes from the underlying network transport. + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; void OnStreamAfterWrite(WriteWrap* w, int status) override; diff --git a/src/node_http_parser_impl.h b/src/node_http_parser_impl.h index 7d5ea347202c59..16c0e77ae853f4 100644 --- a/src/node_http_parser_impl.h +++ b/src/node_http_parser_impl.h @@ -594,10 +594,9 @@ class Parser : public AsyncWrap, public StreamListener { uv_buf_t OnStreamAlloc(size_t suggested_size) override { // For most types of streams, OnStreamRead will be immediately after // OnStreamAlloc, and will consume all data, so using a static buffer for - // reading is more efficient. For other streams, just use the default - // allocator, which uses Malloc(). + // reading is more efficient. For other streams, just use Malloc() directly. if (env()->http_parser_buffer_in_use()) - return StreamListener::OnStreamAlloc(suggested_size); + return uv_buf_init(Malloc(suggested_size), suggested_size); env()->set_http_parser_buffer_in_use(true); if (env()->http_parser_buffer() == nullptr) diff --git a/src/node_internals.h b/src/node_internals.h index 9ac2b0a331c531..82cf5713e95bf2 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -146,10 +146,12 @@ v8::MaybeLocal New(Environment* env, size_t length, void (*callback)(char* data, void* hint), void* hint); -// Takes ownership of |data|. Must allocate |data| with malloc() or realloc() -// because ArrayBufferAllocator::Free() deallocates it again with free(). -// Mixing operator new and free() is undefined behavior so don't do that. -v8::MaybeLocal New(Environment* env, char* data, size_t length); +// Takes ownership of |data|. Must allocate |data| with the current Isolate's +// ArrayBuffer::Allocator(). +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + bool uses_malloc); // Construct a Buffer from a MaybeStackBuffer (and also its subclasses like // Utf8Value and TwoByteValue). @@ -167,7 +169,7 @@ static v8::MaybeLocal New(Environment* env, const size_t len_in_bytes = buf->length() * sizeof(buf->out()[0]); if (buf->IsAllocated()) - ret = New(env, src, len_in_bytes); + ret = New(env, src, len_in_bytes, true); else if (!buf->IsInvalidated()) ret = Copy(env, src, len_in_bytes); diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c659ac06f1d41d..34977557c5bfb8 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -144,6 +144,9 @@ MaybeLocal Message::Deserialize(Environment* env, continue; } + env->isolate_data()->node_allocator()->RegisterPointer( + array_buffer_contents_[i].data, array_buffer_contents_[i].size); + Local ab = ArrayBuffer::New(env->isolate(), array_buffer_contents_[i].release(), @@ -367,6 +370,11 @@ Maybe Message::Serialize(Environment* env, // it inaccessible in this Isolate. ArrayBuffer::Contents contents = ab->Externalize(); ab->Neuter(); + + CHECK(env->isolate_data()->uses_node_allocator()); + env->isolate_data()->node_allocator()->UnregisterPointer( + contents.Data(), contents.ByteLength()); + array_buffer_contents_.push_back( MallocedBuffer { static_cast(contents.Data()), contents.ByteLength() }); diff --git a/src/node_native_module.cc b/src/node_native_module.cc index 2d3769ebf5c949..df7e1749e52ea8 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -11,7 +11,6 @@ namespace native_module { using v8::Array; using v8::ArrayBuffer; -using v8::ArrayBufferCreationMode; using v8::Context; using v8::DEFAULT; using v8::EscapableHandleScope; @@ -153,13 +152,8 @@ MaybeLocal NativeModuleLoader::GetCodeCache(Isolate* isolate, cached_data = it->second.get(); - MallocedBuffer copied(cached_data->length); - memcpy(copied.data, cached_data->data, cached_data->length); - Local buf = - ArrayBuffer::New(isolate, - copied.release(), - cached_data->length, - ArrayBufferCreationMode::kInternalized); + Local buf = ArrayBuffer::New(isolate, cached_data->length); + memcpy(buf->GetContents().Data(), cached_data->data, cached_data->length); return scope.Escape(Uint8Array::New(buf, 0, cached_data->length)); } diff --git a/src/node_serdes.cc b/src/node_serdes.cc index ab3fca7efb0ae8..12ed313b033987 100644 --- a/src/node_serdes.cc +++ b/src/node_serdes.cc @@ -200,10 +200,13 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo& args) { SerializerContext* ctx; ASSIGN_OR_RETURN_UNWRAP(&ctx, args.Holder()); + // Note: Both ValueSerializer and this Buffer::New() variant use malloc() + // as the underlying allocator. std::pair ret = ctx->serializer_.Release(); auto buf = Buffer::New(ctx->env(), reinterpret_cast(ret.first), - ret.second); + ret.second, + true /* uses_malloc */); if (!buf.IsEmpty()) { args.GetReturnValue().Set(buf.ToLocalChecked()); diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 7db8403ced832b..9cff67cd9fecf2 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -419,18 +419,9 @@ inline void ShutdownWrap::OnDone(int status) { Dispose(); } -inline void WriteWrap::SetAllocatedStorage(char* data, size_t size) { - CHECK_NULL(storage_); - storage_ = data; - storage_size_ = size; -} - -inline char* WriteWrap::Storage() { - return storage_; -} - -inline size_t WriteWrap::StorageSize() const { - return storage_size_; +inline void WriteWrap::SetAllocatedStorage(AllocatedBuffer&& storage) { + CHECK_NULL(storage_.data()); + storage_ = std::move(storage); } inline void WriteWrap::OnDone(int status) { diff --git a/src/stream_base.cc b/src/stream_base.cc index a55cb60cfc5338..ebd9beb984eba3 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -111,9 +111,9 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { } } - MallocedBuffer storage; + AllocatedBuffer storage; if (storage_size > 0) - storage = MallocedBuffer(storage_size); + storage = env->AllocateManaged(storage_size); offset = 0; if (!all_buffers) { @@ -129,8 +129,8 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { // Write string CHECK_LE(offset, storage_size); - char* str_storage = storage.data + offset; - size_t str_size = storage_size - offset; + char* str_storage = storage.data() + offset; + size_t str_size = storage.size() - offset; Local string = chunk->ToString(env->context()).ToLocalChecked(); enum encoding encoding = ParseEncoding(env->isolate(), @@ -149,7 +149,7 @@ int StreamBase::Writev(const FunctionCallbackInfo& args) { StreamWriteResult res = Write(*bufs, count, nullptr, req_wrap_obj); SetWriteResult(res); if (res.wrap != nullptr && storage_size > 0) { - res.wrap->SetAllocatedStorage(storage.release(), storage_size); + res.wrap->SetAllocatedStorage(std::move(storage)); } return res.err; } @@ -239,18 +239,18 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_EQ(count, 1); } - MallocedBuffer data; + AllocatedBuffer data; if (try_write) { // Copy partial data - data = MallocedBuffer(buf.len); - memcpy(data.data, buf.base, buf.len); + data = env->AllocateManaged(buf.len); + memcpy(data.data(), buf.base, buf.len); data_size = buf.len; } else { // Write it - data = MallocedBuffer(storage_size); + data = env->AllocateManaged(storage_size); data_size = StringBytes::Write(env->isolate(), - data.data, + data.data(), storage_size, string, enc); @@ -258,7 +258,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { CHECK_LE(data_size, storage_size); - buf = uv_buf_init(data.data, data_size); + buf = uv_buf_init(data.data(), data_size); uv_stream_t* send_handle = nullptr; @@ -278,7 +278,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo& args) { SetWriteResult(res); if (res.wrap != nullptr) { - res.wrap->SetAllocatedStorage(data.release(), data_size); + res.wrap->SetAllocatedStorage(std::move(data)); } return res.err; @@ -343,35 +343,30 @@ void StreamResource::ClearError() { // No-op } - -uv_buf_t StreamListener::OnStreamAlloc(size_t suggested_size) { - return uv_buf_init(Malloc(suggested_size), suggested_size); +uv_buf_t EmitToJSStreamListener::OnStreamAlloc(size_t suggested_size) { + CHECK_NOT_NULL(stream_); + Environment* env = static_cast(stream_)->stream_env(); + return env->AllocateManaged(suggested_size).release(); } - -void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { +void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { CHECK_NOT_NULL(stream_); StreamBase* stream = static_cast(stream_); Environment* env = stream->stream_env(); HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); + AllocatedBuffer buf(env, buf_); if (nread <= 0) { - free(buf.base); if (nread < 0) stream->CallJSOnreadMethod(nread, Local()); return; } - CHECK_LE(static_cast(nread), buf.len); - char* base = Realloc(buf.base, nread); + CHECK_LE(static_cast(nread), buf.size()); + buf.Resize(nread); - Local obj = ArrayBuffer::New( - env->isolate(), - base, - nread, - v8::ArrayBufferCreationMode::kInternalized); // Transfer ownership to V8. - stream->CallJSOnreadMethod(nread, obj); + stream->CallJSOnreadMethod(nread, buf.ToArrayBuffer()); } diff --git a/src/stream_base.h b/src/stream_base.h index d5b2235f0e6f71..33cf62a0c861ba 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -74,24 +74,17 @@ class ShutdownWrap : public StreamReq { class WriteWrap : public StreamReq { public: - char* Storage(); - size_t StorageSize() const; - void SetAllocatedStorage(char* data, size_t size); + void SetAllocatedStorage(AllocatedBuffer&& storage); WriteWrap(StreamBase* stream, v8::Local req_wrap_obj) : StreamReq(stream, req_wrap_obj) { } - ~WriteWrap() override { - free(storage_); - } - // Call stream()->EmitAfterWrite() and dispose of this request wrap. void OnDone(int status) override; private: - char* storage_ = nullptr; - size_t storage_size_ = 0; + AllocatedBuffer storage_; }; @@ -115,7 +108,7 @@ class StreamListener { // It is not valid to return a zero-length buffer from this method. // It is not guaranteed that the corresponding `OnStreamRead()` call // happens in the same event loop turn as this call. - virtual uv_buf_t OnStreamAlloc(size_t suggested_size); + virtual uv_buf_t OnStreamAlloc(size_t suggested_size) = 0; // `OnStreamRead()` is called when data is available on the socket and has // been read into the buffer provided by `OnStreamAlloc()`. @@ -181,6 +174,7 @@ class ReportWritesToJSStreamListener : public StreamListener { // JS land via the handle’s .ondata method. class EmitToJSStreamListener : public ReportWritesToJSStreamListener { public: + uv_buf_t OnStreamAlloc(size_t suggested_size) override; void OnStreamRead(ssize_t nread, const uv_buf_t& buf) override; }; diff --git a/src/stream_pipe.cc b/src/stream_pipe.cc index 19d732d6592aaa..4c9b447a61c826 100644 --- a/src/stream_pipe.cc +++ b/src/stream_pipe.cc @@ -114,17 +114,17 @@ uv_buf_t StreamPipe::ReadableListener::OnStreamAlloc(size_t suggested_size) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); size_t size = std::min(suggested_size, pipe->wanted_data_); CHECK_GT(size, 0); - return uv_buf_init(Malloc(size), size); + return pipe->env()->AllocateManaged(size).release(); } void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, - const uv_buf_t& buf) { + const uv_buf_t& buf_) { StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this); + AllocatedBuffer buf(pipe->env(), buf_); AsyncScope async_scope(pipe); if (nread < 0) { // EOF or error; stop reading and pass the error to the previous listener // (which might end up in JS). - free(buf.base); pipe->is_eof_ = true; stream()->ReadStop(); CHECK_NOT_NULL(previous_listener_); @@ -138,19 +138,18 @@ void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread, return; } - pipe->ProcessData(nread, buf); + pipe->ProcessData(nread, std::move(buf)); } -void StreamPipe::ProcessData(size_t nread, const uv_buf_t& buf) { - uv_buf_t buffer = uv_buf_init(buf.base, nread); +void StreamPipe::ProcessData(size_t nread, AllocatedBuffer&& buf) { + uv_buf_t buffer = uv_buf_init(buf.data(), nread); StreamWriteResult res = sink()->Write(&buffer, 1); if (!res.async) { - free(buf.base); writable_listener_.OnStreamAfterWrite(nullptr, res.err); } else { is_writing_ = true; is_reading_ = false; - res.wrap->SetAllocatedStorage(buf.base, buf.len); + res.wrap->SetAllocatedStorage(std::move(buf)); if (source() != nullptr) source()->ReadStop(); } diff --git a/src/stream_pipe.h b/src/stream_pipe.h index ce0077749c868a..b8d0cca5962a22 100644 --- a/src/stream_pipe.h +++ b/src/stream_pipe.h @@ -42,7 +42,7 @@ class StreamPipe : public AsyncWrap { // `OnStreamWantsWrite()` support. size_t wanted_data_ = 0; - void ProcessData(size_t nread, const uv_buf_t& buf); + void ProcessData(size_t nread, AllocatedBuffer&& buf); class ReadableListener : public StreamListener { public: diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index be1f59d2c52863..5682e51478d5e9 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -462,25 +462,23 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) { void UDPWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - buf->base = node::Malloc(suggested_size); - buf->len = suggested_size; + UDPWrap* wrap = static_cast(handle->data); + *buf = wrap->env()->AllocateManaged(suggested_size).release(); } - void UDPWrap::OnRecv(uv_udp_t* handle, ssize_t nread, - const uv_buf_t* buf, + const uv_buf_t* buf_, const struct sockaddr* addr, unsigned int flags) { + UDPWrap* wrap = static_cast(handle->data); + Environment* env = wrap->env(); + + AllocatedBuffer buf(env, *buf_); if (nread == 0 && addr == nullptr) { - if (buf->base != nullptr) - free(buf->base); return; } - UDPWrap* wrap = static_cast(handle->data); - Environment* env = wrap->env(); - HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); @@ -493,14 +491,12 @@ void UDPWrap::OnRecv(uv_udp_t* handle, }; if (nread < 0) { - if (buf->base != nullptr) - free(buf->base); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); return; } - char* base = node::UncheckedRealloc(buf->base, nread); - argv[2] = Buffer::New(env, base, nread).ToLocalChecked(); + buf.Resize(nread); + argv[2] = buf.ToBuffer().ToLocalChecked(); argv[3] = AddressToJS(env, addr); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); }