Attachment 347414 Details for Bug 188707 – patch for landing

[patch] patch for landing

a-backup.diff (text/plain), 5.16 KB, created by Saam Barati on 2018-08-17 16:49:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-08-17 16:49:48 PDT

Size: 5.16 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 234996)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2018-08-17  Saam barati  <sbarati@apple.com>
>+
>+        intersectionOfPastValuesAtHead must filter values after they've observed an invalidation point
>+        https://bugs.webkit.org/show_bug.cgi?id=188707
>+        <rdar://problem/43015442>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/cfa-expected-values-must-set-clobbered-to-false.js: Added.
>+        (foo):
>+        (let.comp.valueOf):
>+        (result):
>+
> 2018-08-10  Keith Miller  <keith_miller@apple.com>
> 
>         Slicing an ArrayBuffer with a long number returns an ArrayBuffer with byteLength zero
>Index: JSTests/stress/cfa-expected-values-must-set-clobbered-to-false.js
>===================================================================
>--- JSTests/stress/cfa-expected-values-must-set-clobbered-to-false.js	(nonexistent)
>+++ JSTests/stress/cfa-expected-values-must-set-clobbered-to-false.js	(working copy)
>@@ -0,0 +1,38 @@
>+//@ runDefault("--useFTLJIT=0", "--useConcurrentJIT=false")
>+
>+let num = 150;
>+
>+function foo(comp, o, b) {
>+    let sum = o.f;
>+    if (b)
>+        OSRExit();
>+    for (let i = 0; i < comp; ++i) {
>+        sum += o.f;
>+    }
>+    return sum;
>+}
>+noInline(foo);
>+
>+let o = {f:25};
>+let o2 = {f:25, g:44};
>+o2.f = 45;
>+o2.f = 45;
>+o2.f = 45;
>+o2.f = 45;
>+let comp = {
>+    valueOf() { return num; }
>+}
>+
>+foo(comp, o2, true);
>+foo(comp, o2, true);
>+for (let i = 0; i < 500; ++i) {
>+    foo(comp, o2, false);
>+}
>+
>+let o3 = {g:24, f:73};
>+num = 10000000;
>+let result = foo(comp, o3, false);
>+
>+if (result !== (num + 1)*73) {
>+    throw new Error("Bad: " + result);
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 234991)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,39 @@
>+2018-08-17  Saam barati  <sbarati@apple.com>
>+
>+        intersectionOfPastValuesAtHead must filter values after they've observed an invalidation point
>+        https://bugs.webkit.org/show_bug.cgi?id=188707
>+        <rdar://problem/43015442>
>+
>+        Reviewed by Mark Lam.
>+
>+        We use the values in intersectionOfPastValuesAtHead to verify that it is safe to
>+        OSR enter at the head of a block. We verify it's safe to OSR enter by checking
>+        that each incoming value is compatible with its corresponding AbstractValue.
>+        
>+        The bug is that we were sometimes filtering the intersectionOfPastValuesAtHead
>+        with abstract values that were clobbererd. This meant that the value we're
>+        verifying with at OSR entry effectively has an infinite structure set because
>+        it's clobbered. So, imagine we have code like this:
>+        ```
>+        ---> We OSR enter here, and we're clobbered here
>+        InvalidationPoint
>+        GetByOffset(@base)
>+        ```
>+        
>+        The abstract value for @base inside intersectionOfPastValuesAtHead has a
>+        clobberred structure set, so we'd allow an incoming object with any
>+        structure. However, this is wrong because the invalidation point is no
>+        longer fulfilling its promise that it filters the structure that @base has.
>+        
>+        We fix this by filtering the AbstractValues in intersectionOfPastValuesAtHead
>+        as if the incoming value may be live past an InvalidationPoint.
>+        This places a stricter requirement that to safely OSR enter at any basic
>+        block, all incoming values must be compatible as if they lived past
>+        the execution of an invalidation point.
>+
>+        * dfg/DFGCFAPhase.cpp:
>+        (JSC::DFG::CFAPhase::run):
>+
> 2018-08-17  Yusuke Suzuki  <yusukesuzuki@slowstart.org> and Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [JSC] Add GPRReg::InvalidGPRReg and FPRReg::InvalidFPRReg
>Index: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGCFAPhase.cpp	(revision 234991)
>+++ Source/JavaScriptCore/dfg/DFGCFAPhase.cpp	(working copy)
>@@ -141,8 +141,15 @@ public:
>                     continue;
>                 
>                 block->intersectionOfCFAHasVisited &= block->cfaHasVisited;
>-                for (unsigned i = block->intersectionOfPastValuesAtHead.size(); i--;)
>-                    block->intersectionOfPastValuesAtHead[i].filter(block->valuesAtHead[i]);
>+                for (unsigned i = block->intersectionOfPastValuesAtHead.size(); i--;) {
>+                    AbstractValue value = block->valuesAtHead[i];
>+                    // We need to guarantee that when we do an OSR entry, we validate the incoming
>+                    // value as if it could be live past an invalidation point. Otherwise, we may
>+                    // OSR enter with a value with the wrong structure, and an InvalidationPoint's
>+                    // promise of filtering the structure set of certain values is no longer upheld.
>+                    value.m_structure.observeInvalidationPoint();
>+                    block->intersectionOfPastValuesAtHead[i].filter(value);
>+                }
>             }
>         }
>

Flags:

commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 188707: 347392 | 347398 | 347412 | 347414