diff --git a/doc/api/buffer.md b/doc/api/buffer.md index c4369716c6f9..a78254919ff9 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -5690,8 +5690,9 @@ since the extra bytes are not free. Because the address of a `Buffer`'s memory cannot be chosen directly, extra bytes have to be allocated or skipped to reach an aligned address. -[`Buffer.allocUnsafeSlow()`][] over-allocates up to `alignment - 1` bytes and -positions the returned `Buffer` at the first suitably aligned byte within them. +[`Buffer.allocUnsafeSlow()`][] over-allocates `alignment` bytes, or 8 when +`alignment` is smaller than that, and positions the returned `Buffer` at the +first suitably aligned byte within them. [`Buffer.allocUnsafe()`][] instead pads its offset into the shared internal pool, whose start is always aligned to 64 bytes, and only falls back to an allocation of its own when `alignment` is larger than that. Either way, diff --git a/lib/buffer.js b/lib/buffer.js index 19574064ad12..7b0e37239abb 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -141,6 +141,7 @@ const { addBufferPrototypeMethods, createUnsafeBuffer, createUnsafeAlignedBuffer, + alignmentPadding, asciiWrite, latin1Write, utf8Write, @@ -482,9 +483,9 @@ Buffer.allocUnsafe = function allocUnsafe(size, alignment) { * * If `alignment` is given, the memory backing the returned buffer starts at an * address that is a multiple of `alignment`, which is required by e.g. reads - * and writes on file descriptors opened with `O_DIRECT`. Note that up to - * `alignment - 1` extra bytes are allocated to satisfy the request, and that - * the returned buffer's `byteOffset` is therefore usually non-zero. + * and writes on file descriptors opened with `O_DIRECT`. Note that at least + * `alignment` extra bytes are allocated to satisfy the request, and that the + * returned buffer's `byteOffset` is therefore usually non-zero. * @param {number} size * @param {number} [alignment] A power of two, at most 2 ** 30 * @returns {FastBuffer} @@ -504,10 +505,10 @@ function validateAlignment(size, alignment) { throw new ERR_INVALID_ARG_VALUE( 'alignment', alignment, 'must be a power of two'); } - // Satisfying the alignment costs up to `alignment - 1` extra bytes. - if (size > kMaxLength - (alignment - 1)) { + const padding = alignmentPadding(alignment); + if (size > kMaxLength - padding) { throw new ERR_OUT_OF_RANGE( - 'size', `<= ${kMaxLength - (alignment - 1)}`, size); + 'size', `<= ${kMaxLength - padding}`, size); } } diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js index 5977bdf23768..325df6ca01ad 100644 --- a/lib/internal/buffer.js +++ b/lib/internal/buffer.js @@ -5,6 +5,7 @@ const { Float32Array, Float64Array, MathFloor, + MathMax, Number, Symbol, Uint8Array, @@ -1114,24 +1115,33 @@ function createUnsafeBuffer(size) { return new FastBuffer(createUnsafeArrayBuffer(size)); } +// Reaching an aligned address needs at most `alignment - 1` extra bytes, but a +// multiple of 8 is padded instead. Backing store addresses are 8 byte aligned +// too, so the bytes left over after the buffer then remain a whole number of +// elements of any typed array created over its ArrayBuffer. +function alignmentPadding(alignment) { + return MathMax(alignment, 8); +} + // Returns an uninitialized buffer of `size` bytes whose first byte is located at // a memory address that is a multiple of `alignment`. `alignment` must be a -// power of two, and `size + alignment - 1` must not exceed the maximum buffer -// length. Since the address of a backing store cannot be chosen, `alignment - 1` -// extra bytes are allocated and skipped, which leaves the returned buffer with a -// non-zero `byteOffset` into a larger ArrayBuffer. +// power of two, and `size` plus its padding must not exceed the maximum buffer +// length. Since the address of a backing store cannot be chosen, that padding is +// allocated and skipped, which leaves the returned buffer with a non-zero +// `byteOffset` into a larger ArrayBuffer. function createUnsafeAlignedBuffer(size, alignment) { if (size === 0) { return new FastBuffer(); } - const ab = createUnsafeArrayBuffer(size + alignment - 1); + const ab = createUnsafeArrayBuffer(size + alignmentPadding(alignment)); return new FastBuffer(ab, arrayBufferAlignedOffset(ab, alignment), size); } module.exports = { FastBuffer, addBufferPrototypeMethods, + alignmentPadding, markAsUntransferable, isMarkedAsUntransferable, createUnsafeBuffer, diff --git a/src/node_buffer.cc b/src/node_buffer.cc index a0453800d84a..1093b73e6838 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1661,9 +1661,9 @@ void CreateUnsafeArrayBuffer(const FunctionCallbackInfo& args) { // Returns the offset of the first byte of `arrayBuffer` that is located at a // memory address which is a multiple of `alignment`. V8 does not let us choose // the address of a backing store, so an aligned view is obtained by -// over-allocating `alignment - 1` bytes and skipping to that offset. The -// backing store of a non-resizable ArrayBuffer never moves, so the offset stays -// aligned for the lifetime of the ArrayBuffer. +// over-allocating and skipping to that offset. The backing store of a +// non-resizable ArrayBuffer never moves, so the offset stays aligned for the +// lifetime of the ArrayBuffer. void ArrayBufferAlignedOffset(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 2); diff --git a/test/parallel/test-buffer-alloc-alignment.js b/test/parallel/test-buffer-alloc-alignment.js index a1a3890ddc20..a55c3fe5fba8 100644 --- a/test/parallel/test-buffer-alloc-alignment.js +++ b/test/parallel/test-buffer-alloc-alignment.js @@ -111,6 +111,24 @@ for (const alloc of [Buffer.allocUnsafe, Buffer.allocUnsafeSlow]) { assert.strictEqual(buf.buffer.byteLength, 100); } +// The padding an aligned allocation needs does not leave a partial element +// behind it, so a typed array view running from the buffer to the end of its +// ArrayBuffer can be created without an explicit length. +{ + const bufs = [Buffer.allocUnsafe(8), Buffer.allocUnsafeSlow(4096, 4096)]; + for (const alignment of alignments) { + bufs.push(Buffer.allocUnsafe(8, alignment), + Buffer.allocUnsafeSlow(8, alignment)); + } + for (const buf of bufs) { + const rest = buf.buffer.byteLength - buf.byteOffset; + assert.strictEqual(new BigUint64Array(buf.buffer, buf.byteOffset).byteLength, + rest); + assert.strictEqual(new Uint32Array(buf.buffer, buf.byteOffset).byteLength, + rest); + } +} + // Buffer.allocUnsafeSlow() is never pooled, even when aligned. { const a = Buffer.allocUnsafeSlow(64, 64);