Skip to content
Permalink
Browse files

Fix missing edge cases with JSGlobalObjects having a bad time.

https://bugs.webkit.org/show_bug.cgi?id=189028
<rdar://problem/45204939>

Reviewed by Saam Barati.

JSTests:

* stress/regress-189028.js: Added.

Source/JavaScriptCore:

Consider the following scenario:

    let object O1 (of global G1) have an indexing type that is not SlowPut.
    let global G2 have a bad time.
    let object O2 (of global G2) be set as the prototype of O1.
    let object O3 (of global G2) have indexed accessors.

In the existing code, if we set O3 as O2's prototype, we'll have a bug where
O1 will not be made aware that that there are indexed accessors in its prototype
chain.

In this patch, we solve this issue by introducing a new invariant:

    A prototype chain is considered to possibly have indexed accessors if any
    object in the chain belongs to a global object that is having a bad time.

We apply this invariant as follows:

1. Enhance JSGlobalObject::haveABadTime() to also check if other global objects are
   affected by it having a bad time.  If so, it also ensures that those affected
   global objects have a bad time.

   The original code for JSGlobalObject::haveABadTime() uses a ObjectsWithBrokenIndexingFinder
   to find all objects affected by the global object having a bad time.  We enhance
   ObjectsWithBrokenIndexingFinder to also check for the possibility that any global
   objects may be affected by other global objects having a bad time i.e.

        let g1 = global1
        let g2 = global2
        let o1 = an object in g1
        let o2 = an object in g2

        let g1 have a bad time
        g2 is affected if
            o1 is in the prototype chain of o2,
            and o2 may be a prototype.

   If the ObjectsWithBrokenIndexingFinder does find the possibility of other global
   objects being affected, it will abort its heap scan and let haveABadTime() take
   a slow path to do a more complete multi global object scan.

   The slow path works as follows:

   1. Iterate the heap and record the graph of all global object dependencies.

      For each global object, record the list of other global objects that are
      affected by it.

   2. Compute a list of global objects that need to have a bad time using the
      current global object dependency graph.

   3. For each global object in the list of affected global objects, fire their
      HaveABadTime watchpoint and convert all their array structures to the
      SlowPut alternatives.

   4. Re-run ObjectsWithBrokenIndexingFinder to find all objects that are affected
      by any of the globals in the list from (2).

2. Enhance Structure::mayInterceptIndexedAccesses() to also return true if the
   structure's global object is having a bad time.

Note: there are 3 scenarios that we need to consider:

    let g1 = global1
    let g2 = global2
    let o1 = an object in g1
    let o2 = an object in g2

    Scenario 1: o2 is a prototype, and
                g1 has a bad time after o1 is inserted into the o2's prototype chain.

    Scenario 2: o2 is a prototype, and
                o1 is inserted into the o2's prototype chain after g1 has a bad time.

    Scenario 3: o2 is NOT a prototype, and
                o1 is inserted into the o2's prototype chain after g1 has a bad time.

    For scenario 1, when g1 has a bad time, we need to also make sure g2 has
    a bad time.  This is handled by enhancement 1 above.

    For scenario 2, when o1 is inserted into o2's prototype chain, we need to check
    if o1's global object has a bad time.  If so, then we need to make sure o2's
    global also has a bad time (because o2 is a prototype) and convert o2's
    storage type to SlowPut.  This is handled by enhancement 2 above in conjunction
    with JSObject::setPrototypeDirect().

    For scenario 3, when o1 is inserted into o2's prototype chain, we need to check
    if o1's global object has a bad time.  If so, then we only need to convert o2's
    storage type to SlowPut (because o2 is NOT a prototype).  This is handled by
    enhancement 2 above.

3. Also add $vm.isHavingABadTime(), $vm.createGlobalObject() to enable us to
   write some tests for this issue.

* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):
(JSC::JSGlobalObject::haveABadTime):
* runtime/JSGlobalObject.h:
* runtime/JSObject.h:
(JSC::JSObject::mayInterceptIndexedAccesses): Deleted.
* runtime/JSObjectInlines.h:
(JSC::JSObject::mayInterceptIndexedAccesses):
* runtime/Structure.h:
* runtime/StructureInlines.h:
(JSC::Structure::mayInterceptIndexedAccesses const):
* tools/JSDollarVM.cpp:
(JSC::functionHaveABadTime):
(JSC::functionIsHavingABadTime):
(JSC::functionCreateGlobalObject):
(JSC::JSDollarVM::finishCreation):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@237469 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information...
mark.lam@apple.com
mark.lam@apple.com committed Oct 26, 2018
1 parent 37d55b8 commit 8deb8bd96f4a27bf8bb60334c9247cc14ceab2eb
@@ -1,3 +1,13 @@
2018-10-26 Mark Lam <mark.lam@apple.com>

Fix missing edge cases with JSGlobalObjects having a bad time.
https://bugs.webkit.org/show_bug.cgi?id=189028
<rdar://problem/45204939>

Reviewed by Saam Barati.

* stress/regress-189028.js: Added.

2018-10-22 Mark Lam <mark.lam@apple.com>

DFGAbstractValue::m_arrayModes expects IndexingMode values, not IndexingType.
@@ -0,0 +1,242 @@
function assert(x, y) {
if (x != y) {
$vm.print("actual: ", x);
$vm.print("expected: ", y);
throw "FAILED\n" + new Error().stack;
}
}

(function() {
let arr = [1.1, 2.2];
let arr2 = [1.1, 2.2];

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "CopyOnWriteArrayWithDouble");

let o = $vm.createGlobalObject();

$vm.haveABadTime(o);

let proto = new o.Object();
assert($vm.isHavingABadTime(o), true);
assert($vm.isHavingABadTime(proto), true);

arr2.__proto__ = proto;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "ArrayWithSlowPutArrayStorage");
})();

gc();

(function() {
let arr = [1.1, 2.2];
let arr2 = [1.1, 2.2];

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "CopyOnWriteArrayWithDouble");

let o = $vm.createGlobalObject();

let proto = new o.Object();
assert($vm.isHavingABadTime(o), false);
assert($vm.isHavingABadTime(proto), false);

arr2.__proto__ = proto;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "ArrayWithDouble");

$vm.haveABadTime(o);

assert($vm.isHavingABadTime(o), true);
assert($vm.isHavingABadTime(proto), true);

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "ArrayWithSlowPutArrayStorage");
})();

gc();

(function() {
let arr = [1.1, 2.2];
let arr2 = {};

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArray");

let o = $vm.createGlobalObject();

$vm.haveABadTime(o);

let proto = new o.Object();
assert($vm.isHavingABadTime(o), true);
assert($vm.isHavingABadTime(proto), true);

arr2.__proto__ = proto;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArray");

arr2[0] = 1.1;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArrayWithSlowPutArrayStorage");
})();

gc();

(function() {
let arr = [1.1, 2.2];
let arr2 = {};

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArray");

let o = $vm.createGlobalObject();
let proto = new o.Object();

assert($vm.isHavingABadTime(o), false);
assert($vm.isHavingABadTime(proto), false);

arr2.__proto__ = proto;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArray");

arr2[0] = 1.1;

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArrayWithDouble");

$vm.haveABadTime(o);

assert($vm.isHavingABadTime(o), true);
assert($vm.isHavingABadTime(proto), true);

assert($vm.isHavingABadTime(arr), false);
assert($vm.indexingMode(arr), "CopyOnWriteArrayWithDouble");
assert($vm.isHavingABadTime(arr2), false);
assert($vm.indexingMode(arr2), "NonArrayWithSlowPutArrayStorage");
})();

gc();

(function() {
let g0 = $vm.createGlobalObject();
let o0 = new g0.Object();
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(o0), false);

let g1 = $vm.createGlobalObject();
let o1 = new g1.Object();
assert($vm.isHavingABadTime(g1), false);
assert($vm.isHavingABadTime(o1), false);

let g2 = $vm.createGlobalObject();
assert($vm.isHavingABadTime(g2), false);

$vm.haveABadTime(g1);
assert($vm.isHavingABadTime(g1), true);

o1.__proto__ = null;
g2.Array.prototype.__proto__ = o1;
o0.__proto__ = o1;

assert($vm.indexingMode(o0), "NonArray");
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(g2), true);
})();

gc();

(function() {
let g0 = $vm.createGlobalObject();
let o0 = new g0.Object();
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(o0), false);

let g1 = $vm.createGlobalObject();
let o1 = new g1.Object();
assert($vm.isHavingABadTime(g1), false);
assert($vm.isHavingABadTime(o1), false);

let g2 = $vm.createGlobalObject();
assert($vm.isHavingABadTime(g2), false);

o1.__proto__ = null;
g2.Array.prototype.__proto__ = o1;
o0.__proto__ = o1;
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(g1), false);
assert($vm.isHavingABadTime(g2), false);

$vm.haveABadTime(g1);

assert($vm.indexingMode(o0), "NonArray");
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(g1), true);
assert($vm.isHavingABadTime(g2), true);
})();

gc();

(function() {
let g0 = $vm.createGlobalObject();
let o0 = new g0.Object();
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(o0), false);

let g1 = $vm.createGlobalObject();
let o1 = new g1.Object();
assert($vm.isHavingABadTime(g1), false);
assert($vm.isHavingABadTime(o1), false);

let g2 = $vm.createGlobalObject();
let o2 = new g2.Object();
assert($vm.isHavingABadTime(g2), false);
assert($vm.isHavingABadTime(o2), false);

let g3 = $vm.createGlobalObject();
assert($vm.isHavingABadTime(g3), false);

o1.__proto__ = null;
g2.Array.prototype.__proto__ = o1;
o2.__proto__ = o1;
g3.Array.prototype.__proto__ = o2;
o0.__proto__ = o1;
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(g1), false);
assert($vm.isHavingABadTime(g2), false);
assert($vm.isHavingABadTime(g3), false);

$vm.haveABadTime(g1);

assert($vm.indexingMode(o0), "NonArray");
assert($vm.isHavingABadTime(g0), false);
assert($vm.isHavingABadTime(g1), true);
assert($vm.isHavingABadTime(g2), true);
assert($vm.isHavingABadTime(g2), true);
})();
@@ -1,3 +1,122 @@
2018-10-26 Mark Lam <mark.lam@apple.com>

Fix missing edge cases with JSGlobalObjects having a bad time.
https://bugs.webkit.org/show_bug.cgi?id=189028
<rdar://problem/45204939>

Reviewed by Saam Barati.

Consider the following scenario:

let object O1 (of global G1) have an indexing type that is not SlowPut.
let global G2 have a bad time.
let object O2 (of global G2) be set as the prototype of O1.
let object O3 (of global G2) have indexed accessors.

In the existing code, if we set O3 as O2's prototype, we'll have a bug where
O1 will not be made aware that that there are indexed accessors in its prototype
chain.

In this patch, we solve this issue by introducing a new invariant:

A prototype chain is considered to possibly have indexed accessors if any
object in the chain belongs to a global object that is having a bad time.

We apply this invariant as follows:

1. Enhance JSGlobalObject::haveABadTime() to also check if other global objects are
affected by it having a bad time. If so, it also ensures that those affected
global objects have a bad time.

The original code for JSGlobalObject::haveABadTime() uses a ObjectsWithBrokenIndexingFinder
to find all objects affected by the global object having a bad time. We enhance
ObjectsWithBrokenIndexingFinder to also check for the possibility that any global
objects may be affected by other global objects having a bad time i.e.

let g1 = global1
let g2 = global2
let o1 = an object in g1
let o2 = an object in g2

let g1 have a bad time
g2 is affected if
o1 is in the prototype chain of o2,
and o2 may be a prototype.

If the ObjectsWithBrokenIndexingFinder does find the possibility of other global
objects being affected, it will abort its heap scan and let haveABadTime() take
a slow path to do a more complete multi global object scan.

The slow path works as follows:

1. Iterate the heap and record the graph of all global object dependencies.

For each global object, record the list of other global objects that are
affected by it.

2. Compute a list of global objects that need to have a bad time using the
current global object dependency graph.

3. For each global object in the list of affected global objects, fire their
HaveABadTime watchpoint and convert all their array structures to the
SlowPut alternatives.

4. Re-run ObjectsWithBrokenIndexingFinder to find all objects that are affected
by any of the globals in the list from (2).

2. Enhance Structure::mayInterceptIndexedAccesses() to also return true if the
structure's global object is having a bad time.

Note: there are 3 scenarios that we need to consider:

let g1 = global1
let g2 = global2
let o1 = an object in g1
let o2 = an object in g2

Scenario 1: o2 is a prototype, and
g1 has a bad time after o1 is inserted into the o2's prototype chain.

Scenario 2: o2 is a prototype, and
o1 is inserted into the o2's prototype chain after g1 has a bad time.

Scenario 3: o2 is NOT a prototype, and
o1 is inserted into the o2's prototype chain after g1 has a bad time.

For scenario 1, when g1 has a bad time, we need to also make sure g2 has
a bad time. This is handled by enhancement 1 above.

For scenario 2, when o1 is inserted into o2's prototype chain, we need to check
if o1's global object has a bad time. If so, then we need to make sure o2's
global also has a bad time (because o2 is a prototype) and convert o2's
storage type to SlowPut. This is handled by enhancement 2 above in conjunction
with JSObject::setPrototypeDirect().

For scenario 3, when o1 is inserted into o2's prototype chain, we need to check
if o1's global object has a bad time. If so, then we only need to convert o2's
storage type to SlowPut (because o2 is NOT a prototype). This is handled by
enhancement 2 above.

3. Also add $vm.isHavingABadTime(), $vm.createGlobalObject() to enable us to
write some tests for this issue.

* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):
(JSC::JSGlobalObject::haveABadTime):
* runtime/JSGlobalObject.h:
* runtime/JSObject.h:
(JSC::JSObject::mayInterceptIndexedAccesses): Deleted.
* runtime/JSObjectInlines.h:
(JSC::JSObject::mayInterceptIndexedAccesses):
* runtime/Structure.h:
* runtime/StructureInlines.h:
(JSC::Structure::mayInterceptIndexedAccesses const):
* tools/JSDollarVM.cpp:
(JSC::functionHaveABadTime):
(JSC::functionIsHavingABadTime):
(JSC::functionCreateGlobalObject):
(JSC::JSDollarVM::finishCreation):

2018-10-26 Keith Miller <keith_miller@apple.com>

JSC xcconfig should set DEFINES_MODULE

0 comments on commit 8deb8bd

Please sign in to comment.
You can’t perform that action at this time.