Attachment 350139 Details for Bug 189757 – patch

[patch] patch

c-backup.diff (text/plain), 3.92 KB, created by Saam Barati on 2018-09-19 12:02:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-09-19 12:02:10 PDT

Size: 3.92 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 236213)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2018-09-19  Saam barati  <sbarati@apple.com>
>+
>+        AI rule for MultiPutByOffset executes its effects in the wrong order
>+        https://bugs.webkit.org/show_bug.cgi?id=189757
>+        <rdar://problem/43535257>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/multi-put-by-offset-must-filter-value-before-filtering-base.js: Added.
>+        (foo):
>+        (Foo):
>+        (g):
>+
> 2018-09-17  Mark Lam  <mark.lam@apple.com>
> 
>         Ensure that ForInContexts are invalidated if their loop local is over-written.
>Index: JSTests/stress/multi-put-by-offset-must-filter-value-before-filtering-base.js
>===================================================================
>--- JSTests/stress/multi-put-by-offset-must-filter-value-before-filtering-base.js	(nonexistent)
>+++ JSTests/stress/multi-put-by-offset-must-filter-value-before-filtering-base.js	(working copy)
>@@ -0,0 +1,25 @@
>+//@ runDefault("--collectContinuously=1", "--useConcurrentJIT=0", "--useConcurrentGC=1")
>+
>+function foo(oo) {
>+    oo.x = 4;
>+    oo.y = 4;
>+    oo.e = oo;
>+    oo.e = 7;
>+    oo.f = 8;
>+}
>+noInline(foo);
>+
>+function Foo() {
>+    foo(this);
>+}
>+
>+for (var i = 0; i < 100000; i++) {
>+    g();
>+}
>+
>+function g(){
>+    foo({f:8});
>+    new Foo();
>+    new Foo();
>+    new Foo();
>+}
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 236149)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-09-19  Saam barati  <sbarati@apple.com>
>+
>+        AI rule for MultiPutByOffset executes its effects in the wrong order
>+        https://bugs.webkit.org/show_bug.cgi?id=189757
>+        <rdar://problem/43535257>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The AI rule for MultiPutByOffset was executing effects in the wrong order.
>+        It first executed the transition effects and the effects on the base, and
>+        then executed the filtering effects on the value being stored. However, you
>+        can end up with the wrong type when the base and the value being stored
>+        are the same. E.g, in a program like `o.f = o`. These effects need to happen
>+        in the opposite order, modeling what happens in the runtime executing of
>+        MultiPutByOffset.
>+
>+        * dfg/DFGAbstractInterpreterInlines.h:
>+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
>+
> 2018-09-17  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: generate CSSKeywordCompletions from backend values
>Index: Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(revision 236149)
>+++ Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h	(working copy)
>@@ -3299,12 +3299,17 @@ bool AbstractInterpreter<AbstractStateTy
>             }
>         }
>         
>-        observeTransitions(clobberLimit, transitions);
>-        if (forNode(node->child1()).changeStructure(m_graph, newSet) == Contradiction)
>-            m_state.setIsValid(false);
>+        // We need to order AI executing these effects in the same order as they're executed
>+        // at runtime. This is critical when you have JS code like `o.f = o;`. We first
>+        // filter types on o, then transition o. Not the other way around. If we got
>+        // this ordering wrong, we could end up with the wrong type representing o.
>         setForNode(node->child2(), resultingValue);
>         if (!!originalValue && !resultingValue)
>             m_state.setIsValid(false);
>+
>+        observeTransitions(clobberLimit, transitions);
>+        if (forNode(node->child1()).changeStructure(m_graph, newSet) == Contradiction)
>+            m_state.setIsValid(false);
>         break;
>     }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 189757: 350139