1287520 - Incorrect IsPackedArray optimization in Array.prototype.concat

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[André Bargull [:anba]]

The `IsPackedArray` optimization in ArrayConcat is not correct when proxies are used.


Test case:
---
var a = [1, 2, 3];
a.constructor = {
  [Symbol.species]: function(...args) {
    var p = new Proxy(new Array(...args), {
      defineProperty(target, propertyKey, receiver) {
        if (propertyKey === "0") delete a[1];
        return Reflect.defineProperty(target, propertyKey, receiver);
      }
    });
    return p;
  }
};

var p = a.concat();
print(1 in p);
---

Expected: Prints "false"
Actual: Prints "true"

Comment 1

[Tooru Fujisawa [:arai]]

Thanks :)

https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/js/src/builtin/Array.js#993
>             if (IsPackedArray(E)) {

So, we should check `IsPackedArray(A)` there too.

Will post a patch after performance check.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Check IsPackedArray for the result array in Array.prototype.concat.

Before entering optimized path for packed array, added a check for `IsPackedArray(A)`, to avoid entering it when the result array is a Proxy or some complicated thing.

I don't see any notable performance regression with this.

Comment 3

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b40a60e2a4e138d4b99805b5471bcff268817f9a
Bug 1287520 - Check IsPackedArray for the result array in Array.prototype.concat. r=efaust

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b40a60e2a4e1