Changeset 243966 in webkit

Timestamp:

Apr 7, 2019 12:25:59 PM (5 years ago)

Author:

ysuzuki@apple.com

Message:

[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.

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/bytecode/CallLinkInfo.cpp (modified) (3 diffs)
• Source/JavaScriptCore/jit/Repatch.cpp (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/JavaScriptCore/bytecode/CallLinkInfo.cpp

@@ -32 +32 @@
 #include "FunctionCodeBlock.h"
 #include "JSCInlines.h"
-#include "MacroAssembler.h"
 #include "Opcode.h"
 #include "Repatch.h"
@@ -126 +125 @@
 {
     RELEASE_ASSERT(!isDirect());
-    MacroAssembler::repatchPointer(hotPathBegin(), callee);
     m_calleeOrCodeBlock.set(vm, owner, callee);
 }
@@ -133 +131 @@
 {
     RELEASE_ASSERT(!isDirect());
-    MacroAssembler::repatchPointer(hotPathBegin(), nullptr);
     m_calleeOrCodeBlock.clear();
 }

trunk/Source/JavaScriptCore/jit/Repatch.cpp

@@ -845 +845 @@
     ASSERT(!callLinkInfo.isLinked());
     callLinkInfo.setCallee(vm, owner, callee);
+    MacroAssembler::repatchPointer(callLinkInfo.hotPathBegin(), callee);
     callLinkInfo.setLastSeenCallee(vm, owner, callee);
     if (shouldDumpDisassemblyFor(callerCodeBlock))
@@ -896 +897 @@
 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();