- Revision
- 243966
- Author
- [email protected]
- Date
- 2019-04-07 12:25:59 -0700 (Sun, 07 Apr 2019)
Log 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/_javascript_Core:
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):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (243965 => 243966)
--- trunk/JSTests/ChangeLog 2019-04-07 03:55:59 UTC (rev 243965)
+++ trunk/JSTests/ChangeLog 2019-04-07 19:25:59 UTC (rev 243966)
@@ -1,3 +1,13 @@
+2019-04-07 Yusuke Suzuki <[email protected]>
+
+ [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 <[email protected]>
[JSC] OSRExit recovery for SpeculativeAdd does not consier "A = A + A" pattern
Added: trunk/JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js (0 => 243966)
--- trunk/JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js (rev 0)
+++ trunk/JSTests/stress/clear-callee-or-codeblock-in-calllinkinfo-even-cleared-by-jettison.js 2019-04-07 19:25:59 UTC (rev 243966)
@@ -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()
Modified: trunk/Source/_javascript_Core/ChangeLog (243965 => 243966)
--- trunk/Source/_javascript_Core/ChangeLog 2019-04-07 03:55:59 UTC (rev 243965)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-04-07 19:25:59 UTC (rev 243966)
@@ -1,3 +1,25 @@
+2019-04-07 Yusuke Suzuki <[email protected]>
+
+ [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 <[email protected]>
[JSC] OSRExit recovery for SpeculativeAdd does not consier "A = A + A" pattern
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (243965 => 243966)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-04-07 03:55:59 UTC (rev 243965)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-04-07 19:25:59 UTC (rev 243966)
@@ -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,7 +124,6 @@
void CallLinkInfo::setCallee(VM& vm, JSCell* owner, JSObject* callee)
{
RELEASE_ASSERT(!isDirect());
- MacroAssembler::repatchPointer(hotPathBegin(), callee);
m_calleeOrCodeBlock.set(vm, owner, callee);
}
@@ -132,7 +130,6 @@
void CallLinkInfo::clearCallee()
{
RELEASE_ASSERT(!isDirect());
- MacroAssembler::repatchPointer(hotPathBegin(), nullptr);
m_calleeOrCodeBlock.clear();
}
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (243965 => 243966)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2019-04-07 03:55:59 UTC (rev 243965)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2019-04-07 19:25:59 UTC (rev 243966)
@@ -844,6 +844,7 @@
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 @@
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();
