Bug 145358 - [JSC] indexed property doesn't work well
Summary: [JSC] indexed property doesn't work well
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 145360 144252
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-24 10:39 PDT by Yusuke Suzuki
Modified: 2015-08-11 23:57 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2015-05-24 10:39:33 PDT
STEP TO REPRODUCE:

var object = {
    length: 5,
    0: 0,
    get 1() {
        return 1;
    },
    set 1(value) {
        print(value);
        throw new Error(2);
    },
    2: 2,
    3: 3,
};

print(JSON.stringify(Object.getOwnPropertyDescriptor(object, 1)));

EXPECTED:
'{"enumerable":true,"configurable":true}'

ACTUAL:
undefined
Comment 1 Yusuke Suzuki 2015-05-24 10:47:06 PDT
The following issue might be related to this issue.

var object = {
    get 2() {
        return 1;
    },
    set 2(value) {
        throw new Error(2);
    },
    2: 2,  // Throw new Error(2)
};

Is this expected behavior?
Comment 2 Yusuke Suzuki 2015-05-24 11:38:42 PDT
The following code will fail with assertions.

(function () {
    Object.defineProperty(Object.prototype, 0, {
        get() {
            print("Get");
        },
        set() {
            print("Set");
        }
    });
    var object = {
        length: 5,
        0: 0,
        get 1() {
            return 1;
        },
        set 1(value) {
            throw new Error(2);
        },
        2: 2,
        3: 3,
    };
}());
Comment 3 Yusuke Suzuki 2015-05-24 12:29:22 PDT
The following should throw an error, but don't.

Object.defineProperty(Object.prototype, 2, {
    set: function () {
        throw new Error("out");
    }
}); 
var obj = {};
obj[2] = 'hello';
Comment 4 Yusuke Suzuki 2015-05-24 12:30:23 PDT
Hm, it seems that current JSC has serious issues about indexed properties.
Comment 5 Yusuke Suzuki 2015-05-24 12:35:10 PDT
(In reply to comment #1)
> The following issue might be related to this issue.
> 
> var object = {
>     get 2() {
>         return 1;
>     },
>     set 2(value) {
>         throw new Error(2);
>     },
>     2: 2,  // Throw new Error(2)
> };
> 
> Is this expected behavior?

https://bugs.webkit.org/show_bug.cgi?id=145360 fixes it.
But the other 3 issues remain.
Comment 6 Yusuke Suzuki 2015-05-24 12:37:38 PDT
Make the first step :D
https://bugs.webkit.org/show_bug.cgi?id=145360

And change it to meta bug.
Comment 7 Yusuke Suzuki 2015-06-09 13:14:13 PDT
After investigating the issue, I found that storage type is accidentally changed.
I'll investigate more to fix it.
Comment 8 Yusuke Suzuki 2015-08-11 23:48:06 PDT
https://bugs.webkit.org/show_bug.cgi?id=144252 this also fixes the one of the issue listed in this bug.
Comment 9 Yusuke Suzuki 2015-08-11 23:57:32 PDT
(In reply to comment #2)
> The following code will fail with assertions.
> 
> (function () {
>     Object.defineProperty(Object.prototype, 0, {
>         get() {
>             print("Get");
>         },
>         set() {
>             print("Set");
>         }
>     });
>     var object = {
>         length: 5,
>         0: 0,
>         get 1() {
>             return 1;
>         },
>         set 1(value) {
>             throw new Error(2);
>         },
>         2: 2,
>         3: 3,
>     };
> }());

The remaining issue is this.

1. JSObject has 2 storage, vector and map. And map has 2 types, non-sparse and sparse (dictionary mode)
2. If the JSObject is the dictionary mode, there's no vector
3. If the map of the JSObject is non-sparse, there may be the vector. But the ranges of these storages are not overlapped.
4. And JSObject stores the accessor into the map that is not marked as the sparse.
5. But in the other place (like JSArray), they assume that the map does not contain the accessors if the map is not marked as the sparse.

The simplest solution is, "when storing the indexed accessor, always make the object the dictionary mode". But one concern is the performance regression.