ecma262 icon indicating copy to clipboard operation
ecma262 copied to clipboard

Implementations do not agree on ArrayBuffer construction step order

Open gibson042 opened this issue 1 year ago • 2 comments

new ArrayBuffer(length, options = { maxByteLength }) is specified to execute steps in the following order (with each potential source of an abrupt completion prefixed with a circled digit):

  1. ➊ Let byteLength be ? ToIndex(length)
  2. Let requestedMaxByteLength be ? GetArrayBufferMaxByteLengthOption(options)
    1. ➋ Let maxByteLength be ? Get(options, "maxByteLength")
    2. ➌ Return ? ToIndex(maxByteLength)
  3. ? AllocateArrayBuffer(NewTarget as constructor, byteLength, requestedMaxByteLength as maxByteLength)
    1. ➍ If byteLength > maxByteLength, throw a RangeError exception.
    2. ? OrdinaryCreateFromConstructor(constructor, "%ArrayBuffer.prototype%" as intrinsicDefaultProto, slots as internalSlotsList)
      1. ? GetPrototypeFromConstructor(constructor, intrinsicDefaultProto)
        1. ➎ Let proto be ? Get(constructor, "prototype")
        2. If proto is not an Object, then… ? GetFunctionRealm(constructor as obj)
          1. ➏ If obj is a Proxy exotic object, then… ? ValidateNonRevokedProxy(obj)
    3. ? CreateByteDataBlock(byteLength as size)
      1. ➐ If it is impossible to create a new Data Block value consisting of size bytes, throw a RangeError exception.
    4. ➑ If it is not possible to create a Data Block block consisting of maxByteLength bytes, throw a RangeError exception.

However, all current implementations except SpiderMonkey observably deviate from this order. This part of the specification (including other call sites of AllocateArrayBuffer) should be revisited with implementers to settle on universally acceptable behavior and cover it in test262 (i.e., beyond the simple a vs. b before/after tests at https://github.com/tc39/test262/tree/main/test/built-ins/ArrayBuffer ).

gibson042 avatar Aug 12 '24 19:08 gibson042

Can provide some hints where SpiderMonkey works incorrectly? Some basic tests show spec compliance issues in JSC and V8, but I couldn't find a test which fails in SpiderMonkey.

function runTest(expected, {
  index: indexValue,
  maxByteLength: maxByteLengthValue,
  proto = ArrayBuffer.prototype,
}) {
  let log = [];

  let index = {
    valueOf() {
      log.push("index valueOf");
      return indexValue;
    }
  };

  let maxByteLength = {
    valueOf() {
      log.push("maxByteLength valueOf");
      return maxByteLengthValue;
    }
  };

  let options = {
    get maxByteLength() {
      log.push("get maxByteLength");
      if (maxByteLengthValue === undefined) {
        return undefined;
      }
      return maxByteLength;
    }
  };

  let {proxy: newTarget, revoke} = Proxy.revocable(function(){}, {
    get(t, pk, r) {
      log.push("proxy-get " + String(pk));
      if (proto === null) {
        revoke();
        return undefined;
      }
      return proto;
    }
  });

  try {
    let ab = Reflect.construct(ArrayBuffer, [index, options], newTarget);

    if (Object.getPrototypeOf(ab) !== ArrayBuffer.prototype) {
      log.push("wrong prototype");
    }
  } catch (e) {
    log.push("caught " + e.name);
  }

  if (log.toString() !== expected.toString()) {
    (console?.log ?? print)(`${log} != ${expected}`);
  }
}

// conversions and property accesses
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "proxy-get prototype",
], {
  index: 0,
  maxByteLength: 0,
});

// index > maxByteLength
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "caught RangeError",
], {
  index: 10,
  maxByteLength: 0,
});

// index < 0
runTest([
  "index valueOf",
  "caught RangeError",
], {
  index: -1,
  maxByteLength: 0,
});

// maxByteLength < 0
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "caught RangeError",
], {
  index: 0,
  maxByteLength: -1,
});

// index too large for ToIndex
runTest([
  "index valueOf",
  "caught RangeError",
], {
  index: 2**53,
  maxByteLength: 0,
});

// maxByteLength too large for ToIndex
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "caught RangeError",
], {
  index: 0,
  maxByteLength: 2**53,
});

// index too large to allocate
runTest([
  "index valueOf",
  "get maxByteLength",
  "proxy-get prototype",
  "caught RangeError",
], {
  index: 2**52,
  maxByteLength: undefined,
});

// maxByteLength too large to allocate
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "proxy-get prototype",
  "caught RangeError",
], {
  index: 0,
  maxByteLength: 2**52,
});

// constructor.prototype is non-object and constructor is revoked
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "proxy-get prototype",
  "caught TypeError",
], {
  index: 0,
  maxByteLength: 0,
  proto: null,
});

// constructor.prototype is non-object and constructor is revoked and
// index too large to allocate
runTest([
  "index valueOf",
  "get maxByteLength",
  "proxy-get prototype",
  "caught TypeError",
], {
  index: 2**52,
  maxByteLength: undefined,
  proto: null,
});

// constructor.prototype is non-object and constructor is revoked and
// maxByteLength too large to allocate
runTest([
  "index valueOf",
  "get maxByteLength",
  "maxByteLength valueOf",
  "proxy-get prototype",
  "caught TypeError",
], {
  index: 0,
  maxByteLength: 2**52,
  proto: null,
});

anba avatar Aug 13 '24 14:08 anba

@anba I just updated my esvu SpiderMonkey, and confirmed that it implements the spec as written. Thanks for keeping me honest.

gibson042 avatar Aug 13 '24 21:08 gibson042