[JSC] CallLinkInfo should clear Callee or CodeBlock even if it is unlinked by jettison
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Apr 2019 19:25:59 +0000 (19:25 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Apr 2019 19:25:59 +0000 (19:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196683

Reviewed by Saam Barati.

JSTests:

* stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js: Added.
(foo):

Source/JavaScriptCore:

In r243626, we stop repatching CallLinkInfo when the CallLinkInfo is held by jettisoned CodeBlock.
But we still need to clear the Callee or CodeBlock since they are now dead. Otherwise, CodeBlock's
visitWeak eventually accesses this dead cells and crashes because the owner CodeBlock of CallLinkInfo
can be still live.

We also move all repatching operations from CallLinkInfo.cpp to Repatch.cpp for consistency because the
other repatching operations in CallLinkInfo are implemented in Repatch.cpp side.

* bytecode/CallLinkInfo.cpp:
(JSC::CallLinkInfo::setCallee):
(JSC::CallLinkInfo::clearCallee):
* jit/Repatch.cpp:
(JSC::linkFor):
(JSC::revertCall):

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

JSTests/ChangeLog
JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CallLinkInfo.cpp
Source/JavaScriptCore/jit/Repatch.cpp

index e4b6796..35204b1 100644 (file)
@@ -1,3 +1,13 @@
+2019-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] CallLinkInfo should clear Callee or CodeBlock even if it is unlinked by jettison
+        https://bugs.webkit.org/show_bug.cgi?id=196683
+
+        Reviewed by Saam Barati.
+
+        * stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js: Added.
+        (foo):
+
 2019-04-05  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] OSRExit recovery for SpeculativeAdd does not consier "A = A + A" pattern
diff --git a/JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js b/JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js
new file mode 100644 (file)
index 0000000..303f6ce
--- /dev/null
@@ -0,0 +1,8 @@
+//@ runDefault("--osrExitCountForReoptimizationFromLoop=2", "--useFTLJIT=0", "--slowPathAllocsBetweenGCs=100", "--forceDebuggerBytecodeGeneration=1", "--forceEagerCompilation=1")
+
+function foo(x, y) {
+}
+for (var i = 0; i < 1000; ++i)
+    foo(0)
+for (var i = 0; i < 100000; ++i)
+    foo()
index 43cd884..a48b210 100644 (file)
@@ -1,3 +1,25 @@
+2019-04-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] CallLinkInfo should clear Callee or CodeBlock even if it is unlinked by jettison
+        https://bugs.webkit.org/show_bug.cgi?id=196683
+
+        Reviewed by Saam Barati.
+
+        In r243626, we stop repatching CallLinkInfo when the CallLinkInfo is held by jettisoned CodeBlock.
+        But we still need to clear the Callee or CodeBlock since they are now dead. Otherwise, CodeBlock's
+        visitWeak eventually accesses this dead cells and crashes because the owner CodeBlock of CallLinkInfo
+        can be still live.
+
+        We also move all repatching operations from CallLinkInfo.cpp to Repatch.cpp for consistency because the
+        other repatching operations in CallLinkInfo are implemented in Repatch.cpp side.
+
+        * bytecode/CallLinkInfo.cpp:
+        (JSC::CallLinkInfo::setCallee):
+        (JSC::CallLinkInfo::clearCallee):
+        * jit/Repatch.cpp:
+        (JSC::linkFor):
+        (JSC::revertCall):
+
 2019-04-05  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] OSRExit recovery for SpeculativeAdd does not consier "A = A + A" pattern
index c4605c1..b3b66c4 100644 (file)
@@ -31,7 +31,6 @@
 #include "DFGThunks.h"
 #include "FunctionCodeBlock.h"
 #include "JSCInlines.h"
-#include "MacroAssembler.h"
 #include "Opcode.h"
 #include "Repatch.h"
 #include <wtf/ListDump.h>
@@ -125,14 +124,12 @@ CodeLocationLabel<JSInternalPtrTag> CallLinkInfo::slowPathStart()
 void CallLinkInfo::setCallee(VM& vm, JSCell* owner, JSObject* callee)
 {
     RELEASE_ASSERT(!isDirect());
-    MacroAssembler::repatchPointer(hotPathBegin(), callee);
     m_calleeOrCodeBlock.set(vm, owner, callee);
 }
 
 void CallLinkInfo::clearCallee()
 {
     RELEASE_ASSERT(!isDirect());
-    MacroAssembler::repatchPointer(hotPathBegin(), nullptr);
     m_calleeOrCodeBlock.clear();
 }
 
index 17f5dea..9875444 100644 (file)
@@ -844,6 +844,7 @@ void linkFor(
 
     ASSERT(!callLinkInfo.isLinked());
     callLinkInfo.setCallee(vm, owner, callee);
+    MacroAssembler::repatchPointer(callLinkInfo.hotPathBegin(), callee);
     callLinkInfo.setLastSeenCallee(vm, owner, callee);
     if (shouldDumpDisassemblyFor(callerCodeBlock))
         dataLog("Linking call in ", FullCodeOrigin(callerCodeBlock, callLinkInfo.codeOrigin()), " to ", pointerDump(calleeCodeBlock), ", entrypoint at ", codePtr, "\n");
@@ -895,20 +896,23 @@ void linkSlowFor(
 
 static void revertCall(VM* vm, CallLinkInfo& callLinkInfo, MacroAssemblerCodeRef<JITStubRoutinePtrTag> codeRef)
 {
-    if (!callLinkInfo.clearedByJettison()) {
-        if (callLinkInfo.isDirect()) {
-            callLinkInfo.clearCodeBlock();
+    if (callLinkInfo.isDirect()) {
+        callLinkInfo.clearCodeBlock();
+        if (!callLinkInfo.clearedByJettison()) {
             if (callLinkInfo.callType() == CallLinkInfo::DirectTailCall)
                 MacroAssembler::repatchJump(callLinkInfo.patchableJump(), callLinkInfo.slowPathStart());
             else
                 MacroAssembler::repatchNearCall(callLinkInfo.hotPathOther(), callLinkInfo.slowPathStart());
-        } else {
+        }
+    } else {
+        if (!callLinkInfo.clearedByJettison()) {
             MacroAssembler::revertJumpReplacementToBranchPtrWithPatch(
                 MacroAssembler::startOfBranchPtrWithPatchOnRegister(callLinkInfo.hotPathBegin()),
                 static_cast<MacroAssembler::RegisterID>(callLinkInfo.calleeGPR()), 0);
             linkSlowFor(vm, callLinkInfo, codeRef);
-            callLinkInfo.clearCallee();
+            MacroAssembler::repatchPointer(callLinkInfo.hotPathBegin(), nullptr);
         }
+        callLinkInfo.clearCallee();
     }
     callLinkInfo.clearSeen();
     callLinkInfo.clearStub();