diff --git a/common.gypi b/common.gypi index 26c2ef3675fd0e..36b4b1138dbfd2 100644 --- a/common.gypi +++ b/common.gypi @@ -40,7 +40,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.20', + 'v8_embedder_string': '-node.21', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/include/v8-array-buffer.h b/deps/v8/include/v8-array-buffer.h index 4b55c445376530..f70c6dd57b36b0 100644 --- a/deps/v8/include/v8-array-buffer.h +++ b/deps/v8/include/v8-array-buffer.h @@ -338,6 +338,17 @@ class V8_EXPORT ArrayBuffer : public Object { */ bool IsImmutable() const; + /** + * Copy up to |bytes_to_copy| bytes from this ArrayBuffer starting at + * position |source_start| to the target ArrayBuffer starting at position + * |target_start|. Nothing is copied if the source ArrayBuffer is detached, + * or if the target ArrayBuffer is detached or immutable. + * Returns the number of bytes actually copied. + */ + size_t CopyArrayBufferBytes(size_t source_start, size_t bytes_to_copy, + Local target, + size_t target_start) const; + /** * Detaches this ArrayBuffer and all its views (typed arrays). * Detaching sets the byte length of the buffer and all typed arrays to zero, @@ -605,6 +616,16 @@ class V8_EXPORT SharedArrayBuffer : public Object { */ void* Data() const; + /** + * Copy up to |bytes_to_copy| bytes from this SharedArrayBuffer starting at + * position |source_start| to the target SharedArrayBuffer starting at + * position |target_start|. + * Returns the number of bytes actually copied. + */ + size_t CopyArrayBufferBytes(size_t source_start, size_t bytes_to_copy, + Local target, + size_t target_start) const; + V8_INLINE static SharedArrayBuffer* Cast(Value* value) { #ifdef V8_ENABLE_CHECKS CheckCast(value); diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 9ef4e3b4a66006..bdb9f715de95b4 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -4221,6 +4221,44 @@ void* v8::SharedArrayBuffer::Data() const { return Utils::OpenDirectHandle(this)->backing_store(); } +template +static size_t CopyArrayBufferBytesImpl(const void* source_buffer, + size_t source_start, + size_t source_length, + void* target_buffer, size_t target_start, + size_t target_length, + size_t bytes_to_copy) { + source_start = std::min(source_start, source_length); + target_start = std::min(target_start, target_length); + size_t source_size = source_length - source_start; + size_t target_size = target_length - target_start; + bytes_to_copy = std::min({bytes_to_copy, source_size, target_size}); + if (bytes_to_copy == 0) return 0; + const char* src = static_cast(source_buffer) + source_start; + char* dst = static_cast(target_buffer) + target_start; + if (is_shared) { + base::Relaxed_Memmove(reinterpret_cast(dst), + reinterpret_cast(src), + bytes_to_copy); + } else { + std::memmove(dst, src, bytes_to_copy); + } + return bytes_to_copy; +} + +size_t v8::SharedArrayBuffer::CopyArrayBufferBytes( + size_t source_start, size_t bytes_to_copy, Local target, + size_t target_start) const { + i::DisallowGarbageCollection no_gc; + auto self = Utils::OpenDirectHandle(this); + auto that = Utils::OpenDirectHandle(*target); + DCHECK(!that->is_immutable()); + return CopyArrayBufferBytesImpl(self->backing_store(), source_start, + self->GetByteLength(), + that->backing_store(), target_start, + that->GetByteLength(), bytes_to_copy); +} + void v8::ArrayBuffer::CheckCast(Value* that) { auto obj = *Utils::OpenDirectHandle(that); Utils::ApiCheck( @@ -8907,6 +8945,21 @@ bool v8::ArrayBuffer::IsImmutable() const { return Utils::OpenDirectHandle(this)->is_immutable(); } +size_t v8::ArrayBuffer::CopyArrayBufferBytes(size_t source_start, + size_t bytes_to_copy, + Local target, + size_t target_start) const { + i::DisallowGarbageCollection no_gc; + auto self = Utils::OpenDirectHandle(this); + auto that = Utils::OpenDirectHandle(*target); + if (self->was_detached()) return 0; + if (that->was_detached() || that->is_immutable()) return 0; + return CopyArrayBufferBytesImpl(self->backing_store(), source_start, + self->GetByteLength(), + that->backing_store(), target_start, + that->GetByteLength(), bytes_to_copy); +} + namespace { std::shared_ptr ToInternal( std::shared_ptr backing_store) { diff --git a/deps/v8/test/cctest/test-api-array-buffer.cc b/deps/v8/test/cctest/test-api-array-buffer.cc index 955639f73650fd..e66b216f0751f1 100644 --- a/deps/v8/test/cctest/test-api-array-buffer.cc +++ b/deps/v8/test/cctest/test-api-array-buffer.cc @@ -1301,3 +1301,53 @@ TEST(ArrayBuffer_ImmutableBackingStore) { CHECK(ab->IsImmutable()); } + +TEST(ArrayBuffer_CopyArrayBufferBytes) { + LocalContext env; + v8::Isolate* isolate = env.isolate(); + v8::HandleScope scope(isolate); + auto ab1 = v8::ArrayBuffer::New(isolate, 6); + auto ab2 = v8::ArrayBuffer::New(isolate, 4); + std::memcpy(ab1->Data(), "123456", 6); + std::memcpy(ab2->Data(), "ABCD", 4); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 0, ab2, 0)); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(6, 0, ab2, 6)); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 4, ab2, 6)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "ABCD", 4)); + CHECK_EQ(4, ab1->CopyArrayBufferBytes(0, 6, ab2, 0)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "1234", 4)); + CHECK_EQ(2, ab1->CopyArrayBufferBytes(0, 6, ab2, 2)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "1212", 4)); + ab2->Detach(v8::Local()).Check(); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 6, ab2, 0)); + std::unique_ptr backing_store = + v8::ArrayBuffer::NewBackingStore(isolate, 6); + CHECK(backing_store); + v8::internal::BackingStore* i_backing_store = + reinterpret_cast(backing_store.get()); + i_backing_store->set_is_immutable(true); + CHECK(i_backing_store->is_immutable()); + std::shared_ptr shared_backing_store = + std::move(backing_store); + auto ab3 = v8::ArrayBuffer::New(isolate, shared_backing_store); + CHECK(ab3->IsImmutable()); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 6, ab3, 0)); +} + +TEST(SharedArrayBuffer_CopyArrayBufferBytes) { + LocalContext env; + v8::Isolate* isolate = env.isolate(); + v8::HandleScope scope(isolate); + auto ab1 = v8::SharedArrayBuffer::New(isolate, 6); + auto ab2 = v8::SharedArrayBuffer::New(isolate, 4); + std::memcpy(ab1->Data(), "123456", 6); + std::memcpy(ab2->Data(), "ABCD", 4); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 0, ab2, 0)); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(6, 0, ab2, 6)); + CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 4, ab2, 6)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "ABCD", 4)); + CHECK_EQ(4, ab1->CopyArrayBufferBytes(0, 6, ab2, 0)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "1234", 4)); + CHECK_EQ(2, ab1->CopyArrayBufferBytes(0, 6, ab2, 2)); + CHECK_EQ(0, std::memcmp(ab2->Data(), "1212", 4)); +} diff --git a/lib/buffer.js b/lib/buffer.js index 4377b53d865f65..8c17b158222672 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -267,7 +267,7 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) { return _copyActual(source, target, targetStart, sourceStart, sourceEnd); } -function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint8Copy = false) { +function _copyActual(source, target, targetStart, sourceStart, sourceEnd) { if (sourceEnd - sourceStart > target.byteLength - targetStart) sourceEnd = sourceStart + target.byteLength - targetStart; @@ -279,13 +279,10 @@ function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint if (nb <= 0) return 0; - if (sourceStart === 0 && nb === sourceLen && (isUint8Copy || isUint8Array(target))) { - TypedArrayPrototypeSet(target, source, targetStart); - } else { - _copy(source, target, targetStart, sourceStart, nb); - } - - return nb; + // `_copy` returns the number of bytes actually copied, which is `nb` except + // when the target is backed by a detached or immutable ArrayBuffer, in which + // case nothing is copied and it returns 0. + return _copy(source, target, targetStart, sourceStart, nb); } /** diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 6c65c07bdcd2e4..9adb02517efc42 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -597,44 +597,78 @@ void StringSlice(const FunctionCallbackInfo& args) { } } -void CopyImpl(Local source_obj, - Local target_obj, - const uint32_t target_start, - const uint32_t source_start, - const uint32_t to_copy) { - ArrayBufferViewContents source(source_obj); - SPREAD_BUFFER_ARG(target_obj, target); - - memmove(target_data + target_start, source.data() + source_start, to_copy); +// Returns the number of bytes actually copied. This is normally |to_copy|, but +// V8 copies nothing (and returns 0) when the target is backed by a detached or +// immutable ArrayBuffer. +size_t CopyImpl(Local source_obj, + Local target_obj, + const size_t target_start, + const size_t source_start, + const size_t to_copy) { + Local source = source_obj.As(); + Local target = target_obj.As(); + + Local source_ab = source->Buffer(); + Local target_ab = target->Buffer(); + + const size_t source_offset = source->ByteOffset() + source_start; + const size_t target_offset = target->ByteOffset() + target_start; + + // Defer byte-range clamping and detached/immutable handling to V8. When both + // sides are backed by a SharedArrayBuffer the relaxed atomic overload is + // used, which honors the SharedArrayBuffer memory model. Any other + // combination (both regular, or one of each) goes through the ArrayBuffer + // overload: it operates on the underlying backing store regardless of + // shared-ness, so a plain memmove is performed (matching the historical + // behavior for SharedArrayBuffer-backed buffers). The V8 API has no overload + // that mixes ArrayBuffer and SharedArrayBuffer, so the two must never be + // cross-cast. + if (source_ab->IsSharedArrayBuffer() && target_ab->IsSharedArrayBuffer()) { + return source_ab.As()->CopyArrayBufferBytes( + source_offset, + to_copy, + target_ab.As(), + target_offset); + } + return source_ab->CopyArrayBufferBytes( + source_offset, to_copy, target_ab, target_offset); } // Assume caller has properly validated args. void SlowCopy(const FunctionCallbackInfo& args) { Local source_obj = args[0]; Local target_obj = args[1]; - const uint32_t target_start = args[2].As()->Value(); - const uint32_t source_start = args[3].As()->Value(); - const uint32_t to_copy = args[4].As()->Value(); - - CopyImpl(source_obj, target_obj, target_start, source_start, to_copy); - - args.GetReturnValue().Set(to_copy); + // Byte offsets and lengths can exceed uint32 for buffers larger than 4 GiB, + // so they are passed and returned as doubles (exact for integers < 2^53). + const size_t target_start = + static_cast(args[2].As()->Value()); + const size_t source_start = + static_cast(args[3].As()->Value()); + const size_t to_copy = static_cast(args[4].As()->Value()); + + const size_t copied = + CopyImpl(source_obj, target_obj, target_start, source_start, to_copy); + + args.GetReturnValue().Set(static_cast(copied)); } // Assume caller has properly validated args. -uint32_t FastCopy(Local receiver, - Local source_obj, - Local target_obj, - uint32_t target_start, - uint32_t source_start, - uint32_t to_copy, - // NOLINTNEXTLINE(runtime/references) - FastApiCallbackOptions& options) { +double FastCopy(Local receiver, + Local source_obj, + Local target_obj, + double target_start, + double source_start, + double to_copy, + // NOLINTNEXTLINE(runtime/references) + FastApiCallbackOptions& options) { + TRACK_V8_FAST_API_CALL("buffer.copy"); HandleScope scope(options.isolate); - CopyImpl(source_obj, target_obj, target_start, source_start, to_copy); - - return to_copy; + return static_cast(CopyImpl(source_obj, + target_obj, + static_cast(target_start), + static_cast(source_start), + static_cast(to_copy))); } static CFunction fast_copy(CFunction::Make(FastCopy)); diff --git a/test/parallel/test-buffer-copy-immutable.js b/test/parallel/test-buffer-copy-immutable.js new file mode 100644 index 00000000000000..b97e23ebdd1a72 --- /dev/null +++ b/test/parallel/test-buffer-copy-immutable.js @@ -0,0 +1,48 @@ +// Flags: --js-immutable-arraybuffer +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +// transferToImmutable is gated behind --js-immutable-arraybuffer (set above). +// Skip if this V8 build does not expose the API even with the flag set. +if (typeof ArrayBuffer.prototype.transferToImmutable !== 'function') + common.skip('ArrayBuffer.prototype.transferToImmutable is not available'); + +// Copying *into* a buffer backed by an immutable ArrayBuffer must not write to +// the read-only backing store. The copy is a no-op and reports 0 bytes copied. +{ + const ab = new ArrayBuffer(8); + new Uint8Array(ab).set([1, 2, 3, 4, 5, 6, 7, 8]); + const target = Buffer.from(ab.transferToImmutable()); + const source = Buffer.from([9, 9, 9, 9, 9, 9, 9, 9]); + + assert.strictEqual(source.copy(target), 0); + assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]); + + // A partial / offset copy is also a no-op. + assert.strictEqual(source.copy(target, 2, 0, 4), 0); + assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]); +} + +// Copying *from* a buffer backed by an immutable ArrayBuffer is allowed (reads +// do not require a writable backing store) and reports the bytes copied. +{ + const ab = new ArrayBuffer(8); + new Uint8Array(ab).set([10, 20, 30, 40, 50, 60, 70, 80]); + const source = Buffer.from(ab.transferToImmutable()); + const target = Buffer.alloc(8); + + assert.strictEqual(source.copy(target), 8); + assert.deepStrictEqual([...target], [10, 20, 30, 40, 50, 60, 70, 80]); +} + +// A mutable Uint8Array view onto an immutable ArrayBuffer is still a read-only +// target through Buffer.prototype.copy. +{ + const ab = new ArrayBuffer(4); + new Uint8Array(ab).set([100, 101, 102, 103]); + const target = new Uint8Array(ab.transferToImmutable()); + + assert.strictEqual(Buffer.from([1, 2, 3, 4]).copy(target), 0); + assert.deepStrictEqual([...target], [100, 101, 102, 103]); +} diff --git a/test/parallel/test-buffer-copy.js b/test/parallel/test-buffer-copy.js index 9864fc7c551715..2c64f87a4441c5 100644 --- a/test/parallel/test-buffer-copy.js +++ b/test/parallel/test-buffer-copy.js @@ -251,3 +251,54 @@ assert.deepStrictEqual(c, b.slice(0, c.length)); // No copying took place: assert.deepStrictEqual(c.toString(), 'C'.repeat(c.length)); } + +// Copying to/from SharedArrayBuffer-backed buffers. The relaxed-atomic copy +// path is used only when both sides are backed by a SharedArrayBuffer; mixed +// copies (one shared, one not) go through the regular non-atomic overload. +{ + // SharedArrayBuffer -> SharedArrayBuffer. + const src = Buffer.from(new SharedArrayBuffer(512)).fill(0x61); + const dst = Buffer.from(new SharedArrayBuffer(512)).fill(0x62); + const copied = src.copy(dst, 0, 0, 512); + assert.strictEqual(copied, 512); + assert.deepStrictEqual(Buffer.from(dst), Buffer.from(src)); +} + +{ + // SharedArrayBuffer source -> regular Buffer target (mixed). + const src = Buffer.from(new SharedArrayBuffer(256)).fill(0x63); + const dst = Buffer.allocUnsafe(256).fill(0x64); + assert.strictEqual(src.copy(dst), 256); + assert.deepStrictEqual(dst, Buffer.from(src)); +} + +{ + // Regular Buffer source -> SharedArrayBuffer target (mixed). + const src = Buffer.allocUnsafe(256).fill(0x65); + const dst = Buffer.from(new SharedArrayBuffer(256)).fill(0x66); + assert.strictEqual(src.copy(dst), 256); + assert.deepStrictEqual(Buffer.from(dst), src); +} + +{ + // Views with a non-zero byteOffset over a SharedArrayBuffer. The native copy + // is relative to the underlying ArrayBuffer, so the view's byteOffset must be + // accounted for. + const sab = new SharedArrayBuffer(16); + const whole = Buffer.from(sab); + for (let i = 0; i < 16; i++) whole[i] = i; + const src = Buffer.from(sab, 4, 8); // sab bytes 4..11 + const dst = Buffer.from(sab, 12, 4); // sab bytes 12..15 + assert.strictEqual(src.copy(dst, 0, 0, 4), 4); + assert.deepStrictEqual( + [...whole], + [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 4, 5, 6, 7]); +} + +{ + // Overlapping copy within a single SharedArrayBuffer uses memmove semantics. + const buf = Buffer.from(new SharedArrayBuffer(8)); + for (let i = 0; i < 8; i++) buf[i] = i + 1; // [1,2,3,4,5,6,7,8] + assert.strictEqual(buf.copy(buf, 2, 0, 6), 6); + assert.deepStrictEqual([...buf], [1, 2, 1, 2, 3, 4, 5, 6]); +} diff --git a/test/pummel/test-buffer-large-size-copy.js b/test/pummel/test-buffer-large-size-copy.js new file mode 100644 index 00000000000000..27dc9d38fab137 --- /dev/null +++ b/test/pummel/test-buffer-large-size-copy.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); + +// Buffer.prototype.copy with offsets and a byte count > 2 ** 32. Regression +// test for the .copy() side of https://github.com/nodejs/node/issues/55422, +// where the byte count and the return value were truncated to 32 bits. +common.skipIf32Bits(); + +const assert = require('node:assert'); + +// A little past the 2 ** 32 boundary, with headroom for the shift below. +const size = 2 ** 32 + 16; + +let buf; +try { + buf = Buffer.alloc(size); +} catch (e) { + if ( + e.code === 'ERR_MEMORY_ALLOCATION_FAILED' || + /Array buffer allocation failed/.test(e.message) + ) { + common.skip('insufficient memory for Buffer.alloc'); + } + + throw e; +} + +// Place a marker near the end of the source range, at an index > 2 ** 32. +const marker = 0x42; +buf[size - 9] = marker; + +// Shift the whole buffer right by 8 bytes within itself. `to_copy` is +// size - 8 (> 2 ** 32), so a truncated count would copy only 8 bytes. The +// source byte at size - 9 must land at size - 1. +const copied = buf.copy(buf, 8, 0, size - 8); + +// The return value must not be truncated to 32 bits ... +assert.strictEqual(copied, size - 8); +// ... and the byte must actually have moved across the 2 ** 32 boundary. +assert.strictEqual(buf[size - 1], marker);