buffer: throw a consistent range error for length > kMaxLength

• Version: v8.0.0-pre, or master as of Dec.6
• Platform: multiple platforms
• Subsystem: buffer

Background

https://github.com/nodejs/node/pull/9924

When trying to make the error messages thrown for buffer length exceeding the maximum limit, CI failed on some platforms and succeeded on others. As @addaleax pointed out the error could be Array buffer allocation failed on some machines, while being Invalid typed array length on others.

const len = 1422561062959;
const lenLimitMsg = new RegExp('^RangeError: (Invalid typed array length' +
                               '|Array buffer allocation failed)$');
assert.throws(() => Buffer(len).toString('utf8'), lenLimitMsg);

Why this happens

When the buffer allocation methods receive a relatively large length as the sole argument, they all go to createUnsafeBuffer eventually.

function createUnsafeBuffer(size) {
  return new FastBuffer(createUnsafeArrayBuffer(size));
}

function createUnsafeArrayBuffer(size) {
  zeroFill[0] = 0;
  try {
    return new ArrayBuffer(size);
  } finally {
    zeroFill[0] = 1;
  }
}

And when the length is extrodinarily large, this could throw three types of errors:

1. RangeError: Invalid array buffer length
   • This one gets checked first in v8::internal::Builtin_Impl_ArrayBufferConstructor_ConstructStub, the stub for new ArrayBuffer
   • The check fails when the length is even larger than std::numeric_limits<size_t>::max(), and v8 can't convert this into a size_t
2. RangeError: Array buffer allocation failed
   • This one gets checked later in v8::internal::Builtin_Impl_ArrayBufferConstructor_ConstructStub
   • The check could fail in node::MultiplyWithOverflowCheck, when the byte length(buffer length * 8) that will get passed to realloc and alike has an overflow as size_t(i.g. length * 8 > std::numeric_limits<size_t>::max()). * Or it could fail in node::UncheckedRealloc-like functions, when the machine don't have enough memory and realloc-like calls fail. The limit here depends on the machine's current available memory.
3. RangeError: Invalid typed array length
   • This gets checked the last because it happens when the ArrayBuffer is created and gets passed to new FastBuffer aka Uint8Array.
   • The checks are performed in v8::internal::Runtime_TypedArrayInitialize and thrown there. Since V8 can't handle a typed array with length larger than what a SMI can handle(v8::Smi::kMaxValue, or just node::kMaxLength/buffer.kMaxLength), when the length is larger than that, this error will be thrown.

This is what happens on a 64-bit mac, where std::numeric_limits<size_t>::max() is 2^64-1 and v8::Smi::kMaxValue is 2^32-1:

But on other platforms, the limits can vary, hence the error message is platform-specific.

What the docs say

Basically the API docs for all the allocation methods say:

The size must be less than or equal to the value of [buffer.kMaxLength]. Otherwise, a [RangeError] is thrown.

What the C++ methods do

node::Buffer::New and node::Buffer::Copy do:

if (length > kMaxLength) {
  return Local<Object>();
}

What is the old behavior

Before the new buffer implementation came along(https://github.com/nodejs/node/pull/1825), a manual checking is performed and throw a range error in the JS land of Node(as of https://github.com/nodejs/node/blob/9cd44bb2b683446531306bbcff8739fc3526d02c/lib/buffer.js)

// Note: cannot use `length < kMaxLength` here because that fails when
// length is NaN (which is otherwise coerced to zero.)
if (length >= kMaxLength) {
  throw new RangeError('Attempt to allocate Buffer larger than maximum ' +
                        'size: 0x' + kMaxLength.toString(16) + ' bytes');
}

What is the issue

• When the length is larger than v8::Smi::kMaxValue, the allocation would fail, but we need to get into the C++ land of V8 and go through a lot of steps to throw an error out. This could have been avoided by doing a fast check in the JS land first, as the old implementation did.
• The error message is platform-specific when a large length is passed to the allocation methods. The value of v8::Smi::kMaxValue and std::numeric_limits<size_t>::max() could be very different on different platforms, so the tests can not test for a consistent error message. But as I understand we are trying to add exact regexp match to assert.throws in tests, so we need to do a lot of /^RangeError: (Invalid typed array length|Array buffer allocation failed|Invalid array buffer length)$/. That looks weird.
• These different error messages could be confusing to Node.js beginners. A consistent error message like the one in the old implementation is much friendlier.

Possible fix

Currently when a large length is passed into the allocation methods, the call path looks like this:

Buffer ->
  Buffer.allocUnsafe ->
    assertSize
    allocate ->
      createUnsafeBuffer ->
        createUnsafeArrayBuffer ->
          new ArrayBuffer
        FastBuffer
SlowBuffer ->
  // no assertSize here
  createUnsafeBuffer ->
    createUnsafeArrayBuffer ->
      new ArrayBuffer
    FastBuffer
Buffer.alloc ->
  assertSize
  createUnsafeBuffer ->
    createUnsafeArrayBuffer ->
      new ArrayBuffer
    FastBuffer
  FastBuffer
Buffer.allocUnsafeSlow ->
  assertSize
  createUnsafeBuffer ->
    createUnsafeArrayBuffer ->
      new ArrayBuffer
    FastBuffer

So I believe the most suitable place for this check should be in assertSize, and ifassertSize is added to SlowBuffer(), all allocations will get checked before they go into new ArrayBuffer and enter V8 C++ land.

I've only fixed the test that fails on my machine as of now. If @nodejs/buffer think this approach is plausible then I will try to fix more tests with matching regexps.