buffer: pad aligned allocations by a multiple of 8

Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/65605
Fixes: https://github.com/nodejs/node/issues/65601
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>

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,

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);
   }
 }
 

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,

src/node_buffer.cc

@@ -1661,9 +1661,9 @@ void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& 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<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   CHECK_EQ(args.Length(), 2);

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);