Attachment 268389 Details for Bug 152798 – the patch

[patch] the patch

blah.patch (text/plain), 5.74 KB, created by Filip Pizlo on 2016-01-06 12:31:42 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2016-01-06 12:31:42 PST

Size: 5.74 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 194652)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,27 @@
>+2016-01-06  Filip Pizlo  <fpizlo@apple.com>
>+
>+        Make sure that the basic throw-from-operation mode of throwing makes sense in FTL B3
>+        https://bugs.webkit.org/show_bug.cgi?id=152798
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This really just contains one change: we inline emitBranchToOSRExitIfWillCatchException()
>+        into callCheck(), since that was its only caller. This makes it a bit more clear what is
>+        going on.
>+
>+        It turns out that FTL B3 already handled this case properly. I added a test that I believe
>+        illustrates this. Note that although the test uses GetById, which ordinarily throws
>+        exceptions from inside a patchpoint, it uses it in such a way that the exception is thrown
>+        from the operation call for the non-cell bypass path of a GetById(UntypedUse:).
>+
>+        * ftl/FTLLowerDFGToLLVM.cpp:
>+        (JSC::FTL::DFG::LowerDFGToLLVM::callCheck):
>+        (JSC::FTL::DFG::LowerDFGToLLVM::appendOSRExitArgumentsForPatchpointIfWillCatchException):
>+        (JSC::FTL::DFG::LowerDFGToLLVM::lowBlock):
>+        (JSC::FTL::DFG::LowerDFGToLLVM::emitBranchToOSRExitIfWillCatchException): Deleted.
>+        * tests/stress/ftl-operation-exception.js: Added.
>+        (foo):
>+
> 2016-01-06  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Remove duplicate check
>Index: Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp	(revision 194649)
>+++ Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp	(working copy)
>@@ -10113,9 +10113,16 @@ private:
>         LValue exception = m_out.load64(m_out.absolute(vm().addressOfException()));
>         LValue hadException = m_out.notZero64(exception);
> 
>-        bool emittedExceptionHandlingCodeForOSRExit = emitBranchToOSRExitIfWillCatchException(hadException);
>-        if (emittedExceptionHandlingCodeForOSRExit) // It already took care of exception handling logic.
>+        CodeOrigin opCatchOrigin;
>+        HandlerInfo* exceptionHandler;
>+        if (m_graph.willCatchExceptionInMachineFrame(m_origin.forExit, opCatchOrigin, exceptionHandler)) {
>+            bool exitOK = true;
>+            bool isExceptionHandler = true;
>+            appendOSRExit(
>+                Uncountable, noValue(), nullptr, hadException,
>+                m_origin.withForExitAndExitOK(opCatchOrigin, exitOK), isExceptionHandler);
>             return;
>+        }
> 
>         LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("Exception check continuation"));
> 
>@@ -10148,18 +10155,6 @@ private:
>     }
> #endif // !FTL_USES_B3
> 
>-    bool emitBranchToOSRExitIfWillCatchException(LValue hadException)
>-    {
>-        CodeOrigin opCatchOrigin;
>-        HandlerInfo* exceptionHandler;
>-        bool willCatchException = m_graph.willCatchExceptionInMachineFrame(m_origin.forExit, opCatchOrigin, exceptionHandler); 
>-        if (!willCatchException)
>-            return false;
>-
>-        appendOSRExit(Uncountable, noValue(), nullptr, hadException, m_origin.withForExitAndExitOK(opCatchOrigin, true), true);
>-        return true;
>-    }
>-    
>     LBasicBlock lowBlock(DFG::BasicBlock* block)
>     {
>         return m_blocks.get(block);
>Index: Source/JavaScriptCore/tests/stress/ftl-operation-exception.js
>===================================================================
>--- Source/JavaScriptCore/tests/stress/ftl-operation-exception.js	(revision 0)
>+++ Source/JavaScriptCore/tests/stress/ftl-operation-exception.js	(working copy)
>@@ -0,0 +1,62 @@
>+function foo(o, p) {
>+    var x = 100;
>+    var result = 101;
>+    try {
>+        x = 102;
>+        p = 103;
>+        result = o.f;
>+        o = 104;
>+        p = 105;
>+        x = 106;
>+    } catch (e) {
>+        return {outcome: "exception", values: [o, p, x, result]};
>+    }
>+    return {outcome: "return", values: [o, p, x, result]};
>+}
>+
>+noInline(foo);
>+
>+// Warm up foo() with polymorphic objects and non-object types.
>+for (var i = 0; i < 100000; ++i) {
>+    var o;
>+    var isObject = i & 1;
>+    if (isObject) {
>+        o = {f:107};
>+        o["i" + i] = i; // Make it polymorphic.
>+    } else
>+        o = 42;
>+    var result = foo(o);
>+    if (result.outcome !== "return")
>+        throw "Error in loop: bad outcome: " + result.outcome;
>+    if (result.values.length !== 4)
>+        throw "Error in loop: bad number of values: " + result.values.length;
>+    if (result.values[0] !== 104)
>+        throw "Error in loop: bad values[0]: " + result.values[0];
>+    if (result.values[1] !== 105)
>+        throw "Error in loop: bad values[1]: " + result.values[1];
>+    if (result.values[2] !== 106)
>+        throw "Error in loop: bad values[2]: " + result.values[2];
>+    if (isObject) {
>+        if (result.values[3] !== 107)
>+            throw "Error in loop: bad values[3]: " + result.values[3];
>+    } else {
>+        if (result.values[3] !== void 0)
>+            throw "Error in loop: bad values[3]: " + result.values[3];
>+    }
>+}
>+
>+// Now throw an exception.
>+var result = foo(null, 108);
>+if (result.outcome !== "exception")
>+    throw "Error at end: bad outcome: " + result.outcome;
>+if (result.values.length !== 4)
>+    throw "Error at end: bad number of values: " + result.values.length;
>+if (result.values[0] !== null)
>+    throw "Error at end: bad values[0]: " + result.values[0];
>+if (result.values[1] !== 103)
>+    throw "Error at end: bad values[1]: " + result.values[1];
>+if (result.values[2] !== 102)
>+    throw "Error at end: bad values[2]: " + result.values[2];
>+if (result.values[3] !== 101)
>+    throw "Error at end: bad values[3]: " + result.values[3];
>+

Flags:

oliver: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 152798: 268389