Extended TypedArray reverts to parent class.

[AaronAsAChimp]

After creating several instances (~135 on my machine) an extended TypedArray reverts to the TypedArray it was extended from.

Gist: https://gist.github.com/AaronAsAChimp/354374a7e1cdc68c4d9f
Output:

[snipped ...]
The method "double" is function
Is it still an ExtendedArray? true
Is it still an Float64Array? true
The method "double" is undefined
Is it still an ExtendedArray? false
Is it still an Float64Array? true
/folder/array-extend.js:15
        throw new Error('Method "double" should not be undefined');
        ^

Error: Method "double" should not be undefined
    at Object.<anonymous> (/folder/array-extend.js:15:9)
    at Module._compile (module.js:435:26)
    at Object.Module._extensions..js (module.js:442:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Function.Module.runMain (module.js:467:10)
    at startup (node.js:134:18)
    at node.js:961:3

This was tested against Node v4.2.1

[targos]

I cannot reproduce this in d8 4.5.103.35

[skomski]

The reason it is not showing in vanilla d8 is the v8 command flag --typed_array_max_size_in_heap=0 that node passes unconditionally. Reproduces with d8 from v8 4.8 master.

Edit: But the overall issue is in NAMEConstructByLength in v8 (https://github.com/v8/v8/blob/8fafb2916c8ef046b8e841f2fa12b46a8ac8772a/src/js/typedarray.js#L95). It seems that %_TypedArrayInitialize is the culprit.

[trevnorris]

Unfortunately said flag is necessary for Node to work. Otherwise Buffer data access from the native side may crash unexpectedly.

[skomski]

@trevnorris The flag is not important. It only was the reason why it was not reproducible with the given example in vanilla d8. Needed change: new ExtendedArray(9);

The underlying bug is an flawed interaction between new GlobalArrayBuffer() and a runtime function with multiple args because %_TypedArrayInitialize can be completely empty and it still reproduces.

function NAMEConstructByLength(obj, length) {
  %_TypedArrayInitialize(obj, ARRAY_ID, new GlobalArrayBuffer(72), 0, 0, true);
}
RUNTIME_FUNCTION(Runtime_TypedArrayInitialize) {
  return isolate->heap()->undefined_value();
}

With --always-opt it crashes immediately which makes it clear crankshaft is doing some bad optimization.

[bnoordhuis]

Did someone file a V8 bug yet? If yes, can you post the link?

[skomski]

@bnoordhuis https://code.google.com/p/v8/issues/detail?id=4503

[jasnell]

@trevnorris @bnoordhuis ... do we need to keep this one open?

[bnoordhuis]

It seems to be fixed in V8 4.9 but not in 4.5 and 4.6; node v4 and v5 are still affected. If there is a single bug fix commit, I wasn't able to find it - the delta between 4.6 and 4.9 is pretty big - but it may be one of the patches @littledan landed late last year.

[littledan]

Actually I'm not sure what fixed this. I've definitely made a bunch of changes to that code. But I didn't change what happens in Crankshaft or C++, and I don't see any changes with respect to TypedArrayInitialize between 4.6 and 4.9. So if the fix was on the JS side, it might be not too hard to backport. /cc @ofrobots

[ofrobots]

This was fixed in V8 4.8.196 by v8/v8@4490ce8. See associated bug: https://bugs.chromium.org/p/v8/issues/detail?id=4419.

I fear that the change is a bit too complex to be back-ported to V8 4.5 and 4.6.

[jasnell]

this appears to be fixed in master and may be too complex to backport, I'm not sure if there's anything else than can be done with this one relative to v4 and v5. Recommend closing.