Attachment 347392 Details for Bug 188707 – patch

[patch] patch

a-backup.diff (text/plain), 5.05 KB, created by Saam Barati on 2018-08-17 14:57:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-08-17 14:57:23 PDT

Size: 5.05 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 NOBODY (OOPS!).
>+
>+        We use the values in intersectionOfPastValuesAtHead to verify that we can
>+        OSR enter at the head of a block. We verify it's safe to OSR enter by checking
>+        that the incoming values are compatible with the 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 OSR entry effectively has an infinite structure set since
>+        it's clobbered. So, imagine we had code like this:
>+        ```
>+        ---> We OSR enter here
>+        InvalidationPoint
>+        GetByOffset(@base)
>+        ```
>+        
>+        The abstract value for @base inside intersectionOfPastValuesAtHead had a
>+        clobberred structure set, so we'd allow any incoming object with any
>+        structure. However, this is wrong because the invalidation point is no
>+        longer guaranteeing the structure that @base has.
>+        
>+        We fix this by filtering values in intersectionOfPastValuesAtHead as
>+        with values as if an invalidation point has occured. This places the
>+        stricter requirement that to OSR enter at a program point, the
>+        incoming values must be compatible as if an invalidation point has
>+        occurred.
>+
>+        * 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,16 @@ 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 an invalidation point has occurred.
>+                    // Otherwise, we may allow objects to OSR enter with the wrong
>+                    // structure, and an InvalidationPoint no longer guarantees
>+                    // the structure that a value may have.
>+                    value.m_structure.observeInvalidationPoint();
>+                    block->intersectionOfPastValuesAtHead[i].filter(value);
>+                }
>             }
>         }
>

Flags:

mark.lam: review+

Actions: View | Formatted Diff | Diff

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