WebInspector crashed while viewing Timeline when refreshing cnn.com while it was...
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Nov 2015 05:34:01 +0000 (05:34 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Nov 2015 05:34:01 +0000 (05:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=150745

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

During OSR exit, reifyInlinedCallFrames() was using the call kind from a tail call to
find the CallLinkInfo / StubInfo to find the return PC.  Instead we need to get the call
type of the true caller, that is the function we'll be returning to.

This can be found by remembering the last call type we find while walking up the inlined
frames in InlineCallFrame::getCallerSkippingDeadFrames().

We can also return directly back to a getter or setter callsite without using a thunk.

* bytecode/InlineCallFrame.h:
(JSC::InlineCallFrame::computeCallerSkippingDeadFrames):
(JSC::InlineCallFrame::getCallerSkippingDeadFrames):
* dfg/DFGOSRExitCompilerCommon.cpp:
(JSC::DFG::reifyInlinedCallFrames):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emit_op_get_by_id): Need to eliminate the stack pointer check, as it is wrong
for reified inlined frames created during OSR exit.
* jit/ThunkGenerators.cpp:
(JSC::baselineGetterReturnThunkGenerator): Deleted.
(JSC::baselineSetterReturnThunkGenerator): Deleted.
* jit/ThunkGenerators.h:

LayoutTests:

New regression tests.

* js/regress-150745-expected.txt: Added.
* js/regress-150745.html: Added.
* js/script-tests/regress-150745.js: Added.
(Test):
(Test.prototype.get sum):
(Test.prototype.doSum):
(getSum):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191937 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/js/regress-150745-expected.txt [new file with mode: 0644]
LayoutTests/js/regress-150745.html [new file with mode: 0644]
LayoutTests/js/script-tests/regress-150745.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/bytecode/InlineCallFrame.h
Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
Source/JavaScriptCore/jit/JITPropertyAccess.cpp
Source/JavaScriptCore/jit/ThunkGenerators.cpp
Source/JavaScriptCore/jit/ThunkGenerators.h
Source/WebCore/WebCore.xcodeproj/project.pbxproj

index 8baf6b093f6948e43923a847f8ee08ecf27691fa..dada8ae584e603c4464d8d29803b613a27bd4813 100644 (file)
@@ -1,3 +1,20 @@
+2015-11-02  Michael Saboff  <msaboff@apple.com>
+
+        WebInspector crashed while viewing Timeline when refreshing cnn.com while it was already loading
+        https://bugs.webkit.org/show_bug.cgi?id=150745
+
+        Reviewed by Geoffrey Garen.
+
+        New regression tests.
+
+        * js/regress-150745-expected.txt: Added.
+        * js/regress-150745.html: Added.
+        * js/script-tests/regress-150745.js: Added.
+        (Test):
+        (Test.prototype.get sum):
+        (Test.prototype.doSum):
+        (getSum):
+
 2015-11-02  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [Vertical Writing Mode] Rename "vertical-right" CSS value to match spec
diff --git a/LayoutTests/js/regress-150745-expected.txt b/LayoutTests/js/regress-150745-expected.txt
new file mode 100644 (file)
index 0000000..881c2c5
--- /dev/null
@@ -0,0 +1,10 @@
+Regression test for 150745
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Able to OSR exit from an inlined tail callee of a getter.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regress-150745.html b/LayoutTests/js/regress-150745.html
new file mode 100644 (file)
index 0000000..7861858
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/regress-150745.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/regress-150745.js b/LayoutTests/js/script-tests/regress-150745.js
new file mode 100644 (file)
index 0000000..c2f04b6
--- /dev/null
@@ -0,0 +1,47 @@
+description("Regression test for 150745");
+
+// We should be able to ORS exit from an inlined tail callee of a getter.  This test shouldn't crash.
+
+"use strict";
+
+class Test {
+    constructor(a, b)
+    {
+        this.a = a;
+        this.b = b;
+        this.callCount = 0;
+    }
+
+    get sum()
+    {
+        return this.doSum(1, 2);
+    }
+
+    doSum(dummy1, dummy2)
+    {
+        this.callCount++;
+
+        if (this.callCount == 49000)
+            this.dfgCompiled = true;
+
+        if (this.callCount == 199000)
+            this.ftlCompiled = true;
+
+        return this.a + this.b;
+    }
+}
+
+var testObj = new Test(40, 2);
+
+function getSum(o)
+{
+    return o.sum;
+}
+
+for (var i = 0; i < 500000; i++) {
+    var result = getSum(testObj);
+    if (result != 42)
+        testFailed("Expected 42 from \"sum\" getter, got " + result);
+}
+
+testPassed("Able to OSR exit from an inlined tail callee of a getter.");
index 4fadb24daa30c7e1b2ab408a3b9a91d11dac8749..0fb102a6985ce4f97b70588b3686365234be7aba 100644 (file)
@@ -1,3 +1,32 @@
+2015-11-02  Michael Saboff  <msaboff@apple.com>
+
+        WebInspector crashed while viewing Timeline when refreshing cnn.com while it was already loading
+        https://bugs.webkit.org/show_bug.cgi?id=150745
+
+        Reviewed by Geoffrey Garen.
+
+        During OSR exit, reifyInlinedCallFrames() was using the call kind from a tail call to
+        find the CallLinkInfo / StubInfo to find the return PC.  Instead we need to get the call
+        type of the true caller, that is the function we'll be returning to.
+
+        This can be found by remembering the last call type we find while walking up the inlined
+        frames in InlineCallFrame::getCallerSkippingDeadFrames().
+
+        We can also return directly back to a getter or setter callsite without using a thunk.
+
+        * bytecode/InlineCallFrame.h:
+        (JSC::InlineCallFrame::computeCallerSkippingDeadFrames):
+        (JSC::InlineCallFrame::getCallerSkippingDeadFrames):
+        * dfg/DFGOSRExitCompilerCommon.cpp:
+        (JSC::DFG::reifyInlinedCallFrames):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emit_op_get_by_id): Need to eliminate the stack pointer check, as it is wrong
+        for reified inlined frames created during OSR exit. 
+        * jit/ThunkGenerators.cpp:
+        (JSC::baselineGetterReturnThunkGenerator): Deleted.
+        (JSC::baselineSetterReturnThunkGenerator): Deleted.
+        * jit/ThunkGenerators.h:
+
 2015-11-02  Saam barati  <sbarati@apple.com>
 
         Wrong value recovery for DFG try/catch with a getter that throws during an IC miss
index 3ea143c3a80ca7ef1e4412f8dd664cbad814c20c..c3af112d35d7165f883709968a88cd8142b8b0b8 100644 (file)
                70B0A9D01A9B66200001306A /* RuntimeFlags.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RuntimeFlags.h; sourceTree = "<group>"; };
                70DC3E071B2DF2C700054299 /* IteratorPrototype.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = IteratorPrototype.cpp; sourceTree = "<group>"; };
                70DC3E081B2DF2C700054299 /* IteratorPrototype.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IteratorPrototype.h; sourceTree = "<group>"; };
-               70DE9A081BE7D670005D89D9 /* LLIntAssembly.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LLIntAssembly.h; path = LLIntAssembly.h; sourceTree = "<group>"; };
+               70DE9A081BE7D670005D89D9 /* LLIntAssembly.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LLIntAssembly.h; sourceTree = "<group>"; };
                70EC0EBC1AA0D7DA00B6AAFA /* JSStringIterator.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSStringIterator.cpp; sourceTree = "<group>"; };
                70EC0EBD1AA0D7DA00B6AAFA /* JSStringIterator.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSStringIterator.h; sourceTree = "<group>"; };
                70EC0EC01AA0D7DA00B6AAFA /* StringIteratorPrototype.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = StringIteratorPrototype.cpp; sourceTree = "<group>"; };
index 9eb7fb318a19aa37eb96bc017c8a1e9569ba3405..6d61b36ffcea3a5fd2e622284cb32ced6a77ccea 100644 (file)
@@ -146,23 +146,30 @@ struct InlineCallFrame {
         return isTail(static_cast<Kind>(kind));
     }
 
-    static CodeOrigin* computeCallerSkippingDeadFrames(InlineCallFrame* inlineCallFrame)
+    static CodeOrigin* computeCallerSkippingDeadFrames(InlineCallFrame* inlineCallFrame, Kind* callerCallKind = nullptr)
     {
         CodeOrigin* codeOrigin;
         bool tailCallee;
+        int callKind;
         do {
             tailCallee = inlineCallFrame->isTail();
+            callKind = inlineCallFrame->kind;
             codeOrigin = &inlineCallFrame->directCaller;
             inlineCallFrame = codeOrigin->inlineCallFrame;
         } while (inlineCallFrame && tailCallee);
+
         if (tailCallee)
             return nullptr;
+
+        if (callerCallKind)
+            *callerCallKind = static_cast<Kind>(callKind);
+
         return codeOrigin;
     }
 
-    CodeOrigin* getCallerSkippingDeadFrames()
+    CodeOrigin* getCallerSkippingDeadFrames(Kind* callerCallKind = nullptr)
     {
-        return computeCallerSkippingDeadFrames(this);
+        return computeCallerSkippingDeadFrames(this, callerCallKind);
     }
 
     InlineCallFrame* getCallerInlineFrameSkippingDeadFrames()
index c2248de0b9e8da95fa0842e2392adff685251ad8..0aacd0491c5c543d66d63e471a0154df32e155f5 100644 (file)
@@ -146,8 +146,8 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
     for (codeOrigin = &exit.m_codeOrigin; codeOrigin && codeOrigin->inlineCallFrame; codeOrigin = codeOrigin->inlineCallFrame->getCallerSkippingDeadFrames()) {
         InlineCallFrame* inlineCallFrame = codeOrigin->inlineCallFrame;
         CodeBlock* baselineCodeBlock = jit.baselineCodeBlockFor(*codeOrigin);
-        CodeOrigin* trueCaller = inlineCallFrame->getCallerSkippingDeadFrames();
-        void* trueReturnPC = nullptr;
+        InlineCallFrame::Kind trueCallerCallKind;
+        CodeOrigin* trueCaller = inlineCallFrame->getCallerSkippingDeadFrames(&trueCallerCallKind);
         GPRReg callerFrameGPR = GPRInfo::callFrameRegister;
 
         if (!trueCaller) {
@@ -161,7 +161,7 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
             unsigned callBytecodeIndex = trueCaller->bytecodeIndex;
             void* jumpTarget = nullptr;
 
-            switch (inlineCallFrame->kind) {
+            switch (trueCallerCallKind) {
             case InlineCallFrame::Call:
             case InlineCallFrame::Construct:
             case InlineCallFrame::CallVarargs:
@@ -182,22 +182,14 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
                     baselineCodeBlockForCaller->findStubInfo(CodeOrigin(callBytecodeIndex));
                 RELEASE_ASSERT(stubInfo);
 
-                switch (inlineCallFrame->kind) {
-                case InlineCallFrame::GetterCall:
-                    jumpTarget = jit.vm()->getCTIStub(baselineGetterReturnThunkGenerator).code().executableAddress();
-                    break;
-                case InlineCallFrame::SetterCall:
-                    jumpTarget = jit.vm()->getCTIStub(baselineSetterReturnThunkGenerator).code().executableAddress();
-                    break;
-                default:
-                    RELEASE_ASSERT_NOT_REACHED();
-                    break;
-                }
-
-                trueReturnPC = stubInfo->callReturnLocation.labelAtOffset(
+                jumpTarget = stubInfo->callReturnLocation.labelAtOffset(
                     stubInfo->patch.deltaCallToDone).executableAddress();
                 break;
-            } }
+            }
+
+            default:
+                RELEASE_ASSERT_NOT_REACHED();
+            }
 
             if (trueCaller->inlineCallFrame) {
                 jit.addPtr(
@@ -210,9 +202,6 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
             jit.storePtr(AssemblyHelpers::TrustedImmPtr(jumpTarget), AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
         }
 
-        if (trueReturnPC)
-            jit.storePtr(AssemblyHelpers::TrustedImmPtr(trueReturnPC), AssemblyHelpers::addressFor(inlineCallFrame->stackOffset + virtualRegisterForArgument(inlineCallFrame->arguments.size()).offset()));
-                         
         jit.storePtr(AssemblyHelpers::TrustedImmPtr(baselineCodeBlock), AssemblyHelpers::addressFor((VirtualRegister)(inlineCallFrame->stackOffset + JSStack::CodeBlock)));
 
         // Restore the inline call frame's callee save registers.
index 8b3bb8b022d7dbb4b78df3e905a5f5be68b2fec7..8c00827f23c06d7de447cfab4801e6590534607b 100644 (file)
@@ -561,7 +561,6 @@ void JIT::emit_op_get_by_id(Instruction* currentInstruction)
 
     emitValueProfilingSite();
     emitPutVirtualRegister(resultVReg);
-    assertStackPointerOffset();
 }
 
 void JIT::emitSlow_op_get_by_id(Instruction* currentInstruction, Vector<SlowCaseEntry>::iterator& iter)
index 2aec8552a06be966acf61b5ecb9d3cee99d99ffc..3d52d127280a435e7992cc60a2718366fff23a96 100644 (file)
@@ -530,83 +530,6 @@ MacroAssemblerCodeRef unreachableGenerator(VM* vm)
     return FINALIZE_CODE(patchBuffer, ("unreachable thunk"));
 }
 
-MacroAssemblerCodeRef baselineGetterReturnThunkGenerator(VM* vm)
-{
-    JSInterfaceJIT jit(vm);
-    
-#if USE(JSVALUE64)
-    jit.move(GPRInfo::returnValueGPR, GPRInfo::regT0);
-#else
-    jit.setupResults(GPRInfo::regT0, GPRInfo::regT1);
-#endif
-    
-    unsigned numberOfParameters = 0;
-    numberOfParameters++; // The 'this' argument.
-    numberOfParameters++; // The true return PC.
-    
-    unsigned numberOfRegsForCall =
-        JSStack::CallFrameHeaderSize + numberOfParameters;
-    
-    unsigned numberOfBytesForCall =
-        numberOfRegsForCall * sizeof(Register) - sizeof(CallerFrameAndPC);
-    
-    unsigned alignedNumberOfBytesForCall =
-        WTF::roundUpToMultipleOf(stackAlignmentBytes(), numberOfBytesForCall);
-            
-    // The real return address is stored above the arguments. We passed one argument, which is
-    // 'this'. So argument at index 1 is the return address.
-    jit.loadPtr(
-        AssemblyHelpers::Address(
-            AssemblyHelpers::stackPointerRegister,
-            (virtualRegisterForArgument(1).offset() - JSStack::CallerFrameAndPCSize) * sizeof(Register)),
-        GPRInfo::regT2);
-    
-    jit.addPtr(
-        AssemblyHelpers::TrustedImm32(alignedNumberOfBytesForCall),
-        AssemblyHelpers::stackPointerRegister);
-    
-    jit.jump(GPRInfo::regT2);
-
-    LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
-    return FINALIZE_CODE(patchBuffer, ("baseline getter return thunk"));
-}
-
-MacroAssemblerCodeRef baselineSetterReturnThunkGenerator(VM* vm)
-{
-    JSInterfaceJIT jit(vm);
-    
-    unsigned numberOfParameters = 0;
-    numberOfParameters++; // The 'this' argument.
-    numberOfParameters++; // The value to set.
-    numberOfParameters++; // The true return PC.
-    
-    unsigned numberOfRegsForCall =
-        JSStack::CallFrameHeaderSize + numberOfParameters;
-    
-    unsigned numberOfBytesForCall =
-        numberOfRegsForCall * sizeof(Register) - sizeof(CallerFrameAndPC);
-    
-    unsigned alignedNumberOfBytesForCall =
-        WTF::roundUpToMultipleOf(stackAlignmentBytes(), numberOfBytesForCall);
-            
-    // The real return address is stored above the arguments. We passed two arguments, so
-    // the argument at index 2 is the return address.
-    jit.loadPtr(
-        AssemblyHelpers::Address(
-            AssemblyHelpers::stackPointerRegister,
-            (virtualRegisterForArgument(2).offset() - JSStack::CallerFrameAndPCSize) * sizeof(Register)),
-        GPRInfo::regT2);
-    
-    jit.addPtr(
-        AssemblyHelpers::TrustedImm32(alignedNumberOfBytesForCall),
-        AssemblyHelpers::stackPointerRegister);
-    
-    jit.jump(GPRInfo::regT2);
-
-    LinkBuffer patchBuffer(*vm, jit, GLOBAL_THUNK_ID);
-    return FINALIZE_CODE(patchBuffer, ("baseline setter return thunk"));
-}
-
 static void stringCharLoad(SpecializedThunkJIT& jit, VM* vm)
 {
     // load string
index 4531b182b734c363fccf56684bad4ba43c1c23c6..ad4cddd6bdaf99050e3adec0a10db9a13047839d 100644 (file)
@@ -49,9 +49,6 @@ MacroAssemblerCodeRef nativeTailCallGenerator(VM*);
 MacroAssemblerCodeRef arityFixupGenerator(VM*);
 MacroAssemblerCodeRef unreachableGenerator(VM*);
 
-MacroAssemblerCodeRef baselineGetterReturnThunkGenerator(VM* vm);
-MacroAssemblerCodeRef baselineSetterReturnThunkGenerator(VM* vm);
-
 MacroAssemblerCodeRef charCodeAtThunkGenerator(VM*);
 MacroAssemblerCodeRef charAtThunkGenerator(VM*);
 MacroAssemblerCodeRef clz32ThunkGenerator(VM*);
index 1cecb5dc314b5573337762e1ce06730efae13a2a..95b1babab8657936d6e657eacb7bf8b783fa1e93 100644 (file)
                        runOnlyForDeploymentPostprocessing = 0;
                };
 /* End PBXFrameworksBuildPhase section */
+
 /* Begin PBXGroup section */
                00B9318013BA867F0035A948 /* parser */ = {
                        isa = PBXGroup;