Skip to content

Commit f099b34

Browse files
jasnelljasnell
committed
src: make a number of minor improvements to buffer
When using Buffer::New with an externally allocated buffer and v8 sandbox enabled, defer to copy instead of wrapping the buffer. Also, when immediately memcpy'ing into the full buffer, allocate the buffer with kUninitialized as a slight perf optimization.
1 parent 9eb9c26 commit f099b34

9 files changed

+55
-17
lines changed

‎src/crypto/crypto_cipher.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,10 @@ CipherBase::UpdateResult CipherBase::Update(
708708
*out = ArrayBuffer::NewBackingStore(env()->isolate(), 0);
709709
} else if (static_cast<size_t>(buf_len) != (*out)->ByteLength()) {
710710
std::unique_ptr<BackingStore> old_out = std::move(*out);
711-
*out = ArrayBuffer::NewBackingStore(env()->isolate(), buf_len);
711+
*out = ArrayBuffer::NewBackingStore(
712+
env()->isolate(),
713+
buf_len,
714+
BackingStoreInitializationMode::kUninitialized);
712715
memcpy((*out)->Data(), old_out->Data(), buf_len);
713716
}
714717

@@ -804,7 +807,10 @@ bool CipherBase::Final(std::unique_ptr<BackingStore>* out) {
804807
*out = ArrayBuffer::NewBackingStore(env()->isolate(), 0);
805808
} else if (static_cast<size_t>(out_len) != (*out)->ByteLength()) {
806809
std::unique_ptr<BackingStore> old_out = std::move(*out);
807-
*out = ArrayBuffer::NewBackingStore(env()->isolate(), out_len);
810+
*out = ArrayBuffer::NewBackingStore(
811+
env()->isolate(),
812+
out_len,
813+
BackingStoreInitializationMode::kUninitialized);
808814
memcpy((*out)->Data(), old_out->Data(), out_len);
809815
}
810816

@@ -880,7 +886,10 @@ bool PublicKeyCipher::Cipher(
880886
if (buf.size() == 0) {
881887
*out = ArrayBuffer::NewBackingStore(env->isolate(), 0);
882888
} else {
883-
*out = ArrayBuffer::NewBackingStore(env->isolate(), buf.size());
889+
*out = ArrayBuffer::NewBackingStore(
890+
env->isolate(),
891+
buf.size(),
892+
BackingStoreInitializationMode::kUninitialized);
884893
memcpy((*out)->Data(), buf.get(), buf.size());
885894
}
886895

‎src/crypto/crypto_sig.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ std::unique_ptr<BackingStore> Node_SignFinal(Environment* env,
9999
[[likely]] {
100100
CHECK_LE(sig_buf.len, sig->ByteLength());
101101
if (sig_buf.len < sig->ByteLength()) {
102-
auto new_sig = ArrayBuffer::NewBackingStore(env->isolate(), sig_buf.len);
102+
auto new_sig = ArrayBuffer::NewBackingStore(
103+
env->isolate(),
104+
sig_buf.len,
105+
BackingStoreInitializationMode::kUninitialized);
103106
if (sig_buf.len > 0) [[likely]] {
104107
memcpy(new_sig->Data(), sig->Data(), sig_buf.len);
105108
}

‎src/node_blob.cc

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ using v8::Array;
2121
using v8::ArrayBuffer;
2222
using v8::ArrayBufferView;
2323
using v8::BackingStore;
24+
using v8::BackingStoreInitializationMode;
2425
using v8::Context;
2526
using v8::Function;
2627
using v8::FunctionCallbackInfo;
@@ -82,8 +83,8 @@ void Concat(const FunctionCallbackInfo<Value>& args) {
8283
}
8384
}
8485

85-
std::shared_ptr<BackingStore> store =
86-
ArrayBuffer::NewBackingStore(env->isolate(), total);
86+
std::shared_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
87+
env->isolate(), total, BackingStoreInitializationMode::kUninitialized);
8788
uint8_t* ptr = static_cast<uint8_t*>(store->Data());
8889
for (size_t n = 0; n < views.size(); n++) {
8990
uint8_t* from =
@@ -210,8 +211,8 @@ void Blob::New(const FunctionCallbackInfo<Value>& args) {
210211
}
211212

212213
// If the ArrayBuffer is not detachable, we will copy from it instead.
213-
std::shared_ptr<BackingStore> store =
214-
ArrayBuffer::NewBackingStore(isolate, byte_length);
214+
std::shared_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
215+
isolate, byte_length, BackingStoreInitializationMode::kUninitialized);
215216
uint8_t* ptr = static_cast<uint8_t*>(buf->Data()) + byte_offset;
216217
std::copy(ptr, ptr + byte_length, static_cast<uint8_t*>(store->Data()));
217218
return DataQueue::CreateInMemoryEntryFromBackingStore(
@@ -375,8 +376,10 @@ void Blob::Reader::Pull(const FunctionCallbackInfo<Value>& args) {
375376
size_t total = 0;
376377
for (size_t n = 0; n < count; n++) total += vecs[n].len;
377378

378-
std::shared_ptr<BackingStore> store =
379-
ArrayBuffer::NewBackingStore(env->isolate(), total);
379+
std::shared_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
380+
env->isolate(),
381+
total,
382+
BackingStoreInitializationMode::kUninitialized);
380383
auto ptr = static_cast<uint8_t*>(store->Data());
381384
for (size_t n = 0; n < count; n++) {
382385
std::copy(vecs[n].base, vecs[n].base + vecs[n].len, ptr);

‎src/node_buffer.cc

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,8 @@ MaybeLocal<Object> New(Isolate* isolate,
329329
if (actual > 0) [[likely]] {
330330
if (actual < length) {
331331
std::unique_ptr<BackingStore> old_store = std::move(store);
332-
store = ArrayBuffer::NewBackingStore(isolate, actual);
332+
store = ArrayBuffer::NewBackingStore(
333+
isolate, actual, BackingStoreInitializationMode::kUninitialized);
333334
memcpy(store->Data(), old_store->Data(), actual);
334335
}
335336
Local<ArrayBuffer> buf = ArrayBuffer::New(isolate, std::move(store));
@@ -416,7 +417,7 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
416417

417418
CHECK(bs);
418419

419-
memcpy(bs->Data(), data, length);
420+
if (length > 0) memcpy(bs->Data(), data, length);
420421

421422
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, std::move(bs));
422423

@@ -506,6 +507,17 @@ MaybeLocal<Object> New(Environment* env,
506507
}
507508
}
508509

510+
#if defined(V8_ENABLE_SANDBOX)
511+
// When v8 sandbox is enabled, external backing stores are not supported
512+
// since all arraybuffer allocations are expected to be done by the isolate.
513+
// Since this violates the contract of this function, let's free the data and
514+
// throw an error.
515+
free(data);
516+
THROW_ERR_OPERATION_FAILED(
517+
env->isolate(),
518+
"Wrapping external data is not supported when the v8 sandbox is enabled");
519+
return MaybeLocal<Object>();
520+
#else
509521
EscapableHandleScope handle_scope(env->isolate());
510522

511523
auto free_callback = [](void* data, size_t length, void* deleter_data) {
@@ -520,6 +532,7 @@ MaybeLocal<Object> New(Environment* env,
520532
if (Buffer::New(env, ab, 0, length).ToLocal(&obj))
521533
return handle_scope.Escape(obj);
522534
return Local<Object>();
535+
#endif
523536
}
524537

525538
namespace {

‎src/node_http2.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2104,7 +2104,10 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
21042104
[[likely]] {
21052105
// Shrink to the actual amount of used data.
21062106
std::unique_ptr<BackingStore> old_bs = std::move(bs);
2107-
bs = ArrayBuffer::NewBackingStore(env()->isolate(), nread);
2107+
bs = ArrayBuffer::NewBackingStore(
2108+
env()->isolate(),
2109+
nread,
2110+
BackingStoreInitializationMode::kUninitialized);
21082111
memcpy(bs->Data(), old_bs->Data(), nread);
21092112
} else {
21102113
// This is a very unlikely case, and should only happen if the ReadStart()

‎src/node_sqlite.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ namespace sqlite {
1919

2020
using v8::Array;
2121
using v8::ArrayBuffer;
22+
using v8::BackingStoreInitializationMode;
2223
using v8::BigInt;
2324
using v8::Boolean;
2425
using v8::ConstructorBehavior;
@@ -103,7 +104,8 @@ using v8::Value;
103104
static_cast<size_t>(sqlite3_##from##_bytes(__VA_ARGS__)); \
104105
auto data = reinterpret_cast<const uint8_t*>( \
105106
sqlite3_##from##_blob(__VA_ARGS__)); \
106-
auto store = ArrayBuffer::NewBackingStore((isolate), size); \
107+
auto store = ArrayBuffer::NewBackingStore( \
108+
(isolate), size, BackingStoreInitializationMode::kUninitialized); \
107109
memcpy(store->Data(), data, size); \
108110
auto ab = ArrayBuffer::New((isolate), std::move(store)); \
109111
(result) = Uint8Array::New(ab, 0, size); \

‎src/quic/streams.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ namespace node {
1818
using v8::Array;
1919
using v8::ArrayBuffer;
2020
using v8::ArrayBufferView;
21+
using v8::BackingStoreInitializationMode;
2122
using v8::BigInt;
2223
using v8::FunctionCallbackInfo;
2324
using v8::FunctionTemplate;
@@ -1198,7 +1199,8 @@ void Stream::ReceiveData(const uint8_t* data,
11981199

11991200
STAT_INCREMENT_N(Stats, bytes_received, len);
12001201
STAT_RECORD_TIMESTAMP(Stats, received_at);
1201-
auto backing = ArrayBuffer::NewBackingStore(env()->isolate(), len);
1202+
auto backing = ArrayBuffer::NewBackingStore(
1203+
env()->isolate(), len, BackingStoreInitializationMode::kUninitialized);
12021204
memcpy(backing->Data(), data, len);
12031205
inbound_->append(DataQueue::CreateInMemoryEntryFromBackingStore(
12041206
std::move(backing), 0, len));

‎src/stream_base.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,8 @@ void EmitToJSStreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
708708
CHECK_LE(static_cast<size_t>(nread), bs->ByteLength());
709709
if (static_cast<size_t>(nread) != bs->ByteLength()) {
710710
std::unique_ptr<BackingStore> old_bs = std::move(bs);
711-
bs = ArrayBuffer::NewBackingStore(isolate, nread);
711+
bs = ArrayBuffer::NewBackingStore(
712+
isolate, nread, BackingStoreInitializationMode::kUninitialized);
712713
memcpy(bs->Data(), old_bs->Data(), nread);
713714
}
714715

‎src/udp_wrap.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ using errors::TryCatchScope;
3535
using v8::Array;
3636
using v8::ArrayBuffer;
3737
using v8::BackingStore;
38+
using v8::BackingStoreInitializationMode;
3839
using v8::Boolean;
3940
using v8::Context;
4041
using v8::DontDelete;
@@ -759,7 +760,8 @@ void UDPWrap::OnRecv(ssize_t nread,
759760
} else if (static_cast<size_t>(nread) != bs->ByteLength()) {
760761
CHECK_LE(static_cast<size_t>(nread), bs->ByteLength());
761762
std::unique_ptr<BackingStore> old_bs = std::move(bs);
762-
bs = ArrayBuffer::NewBackingStore(isolate, nread);
763+
bs = ArrayBuffer::NewBackingStore(
764+
isolate, nread, BackingStoreInitializationMode::kUninitialized);
763765
memcpy(bs->Data(), old_bs->Data(), nread);
764766
}
765767

0 commit comments

Comments
 (0)