chakra-core / ChakraCore

ChakraCore is an open source Javascript engine with a C API.
MIT License
9.13k stars 1.2k forks source link

A question about Array.prototype.push #6582

Closed YiWen-y closed 3 years ago

YiWen-y commented 3 years ago
Version

chakra-1.11.24

Test case
var arr = [];

Object.defineProperty(Array.prototype, '0', {
    get: function() {
        print('get');
        return 30;
    },
    set: function() {
        print('set');
        return 60;
    }
});

arr.push(1);
print('arr:', arr);
print('arr:', arr[0]);
Execution steps
./ChakraCore/out/Release/ch testcase.js
Output
arr: 1
arr: 1
Expected behavior
set
arr: get
30
get
arr: 30
Description

When executing the above test case, when executing arr.push in line 14, other engines (such as v8, SpiderMonkey, JavascriptCore, etc.) all call the set method defined in line 8, and then execute the get method defined in line 4. However, chakra does not seem to do this. Is this a bug in chakra implementation?

rhuanjl commented 3 years ago

This definitely looks like a bug - we're not checking the prototype for the setter.

Looking at it our Push implementation doesn't check for setters at all on prototype or the object itself.

Easiest fix would be to reimplement push as self-hosted javascript - not sure what the performance impact would be though.

rhuanjl commented 3 years ago

I'll look at doing this after #6583

rhuanjl commented 3 years ago

I've written a fix BUT it slaughtered the performance on some of our benchmarks need to either look at conditional ways to optimise JS builtins or do a native fix rather than a self-hosted JS fix.