- Revision
- 243626
- Author
- [email protected]
- Date
- 2019-03-28 15:05:34 -0700 (Thu, 28 Mar 2019)
Log Message
CodeBlock::jettison() should disallow repatching its own calls
https://bugs.webkit.org/show_bug.cgi?id=196359
<rdar://problem/48973663>
Reviewed by Saam Barati.
JSTests:
* stress/call-link-info-osrexit-repatch.js: Added.
(foo):
Source/_javascript_Core:
CodeBlock::jettison() calls CommonData::invalidate, which replaces the `hlt`
instruction with the jump to OSR exit. However, if the `hlt` was immediately
followed by a call to the CodeBlock being jettisoned, we would write over the
OSR exit address while unlinking all the incoming CallLinkInfos later in
CodeBlock::jettison().
Change it so that we set a flag, `clearedByJettison`, in all the CallLinkInfos
owned by the CodeBlock being jettisoned. If the flag is set, we will avoid
repatching the call during unlinking. This is safe because this call will never
be reachable again after the CodeBlock is jettisoned.
* bytecode/CallLinkInfo.cpp:
(JSC::CallLinkInfo::CallLinkInfo):
(JSC::CallLinkInfo::setCallee):
(JSC::CallLinkInfo::clearCallee):
(JSC::CallLinkInfo::setCodeBlock):
(JSC::CallLinkInfo::clearCodeBlock):
* bytecode/CallLinkInfo.h:
(JSC::CallLinkInfo::clearedByJettison):
(JSC::CallLinkInfo::setClearedByJettison):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::jettison):
* jit/Repatch.cpp:
(JSC::revertCall):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (243625 => 243626)
--- trunk/JSTests/ChangeLog 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/JSTests/ChangeLog 2019-03-28 22:05:34 UTC (rev 243626)
@@ -1,3 +1,14 @@
+2019-03-28 Tadeu Zagallo <[email protected]>
+
+ CodeBlock::jettison() should disallow repatching its own calls
+ https://bugs.webkit.org/show_bug.cgi?id=196359
+ <rdar://problem/48973663>
+
+ Reviewed by Saam Barati.
+
+ * stress/call-link-info-osrexit-repatch.js: Added.
+ (foo):
+
2019-03-28 Yusuke Suzuki <[email protected]>
[JSC] imports-oom.js intermittently fails
Added: trunk/JSTests/stress/call-link-info-osrexit-repatch.js (0 => 243626)
--- trunk/JSTests/stress/call-link-info-osrexit-repatch.js (rev 0)
+++ trunk/JSTests/stress/call-link-info-osrexit-repatch.js 2019-03-28 22:05:34 UTC (rev 243626)
@@ -0,0 +1,18 @@
+//@ runFTLEager("--watchdog=1000", "--watchdog-exception-ok")
+
+function foo(a, b) {
+ 'use strict';
+ if (a === 0) {
+ return;
+ }
+ if (a === 0) {
+ return foo(a + 0);
+ }
+ if (a === 0) {
+ return foo(+a, 0);
+ }
+ return foo(b / 1, a, 0);
+ 0 === 0
+}
+
+foo(1, 5);
Modified: trunk/Source/_javascript_Core/ChangeLog (243625 => 243626)
--- trunk/Source/_javascript_Core/ChangeLog 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-03-28 22:05:34 UTC (rev 243626)
@@ -1,3 +1,36 @@
+2019-03-28 Tadeu Zagallo <[email protected]>
+
+ CodeBlock::jettison() should disallow repatching its own calls
+ https://bugs.webkit.org/show_bug.cgi?id=196359
+ <rdar://problem/48973663>
+
+ Reviewed by Saam Barati.
+
+ CodeBlock::jettison() calls CommonData::invalidate, which replaces the `hlt`
+ instruction with the jump to OSR exit. However, if the `hlt` was immediately
+ followed by a call to the CodeBlock being jettisoned, we would write over the
+ OSR exit address while unlinking all the incoming CallLinkInfos later in
+ CodeBlock::jettison().
+
+ Change it so that we set a flag, `clearedByJettison`, in all the CallLinkInfos
+ owned by the CodeBlock being jettisoned. If the flag is set, we will avoid
+ repatching the call during unlinking. This is safe because this call will never
+ be reachable again after the CodeBlock is jettisoned.
+
+ * bytecode/CallLinkInfo.cpp:
+ (JSC::CallLinkInfo::CallLinkInfo):
+ (JSC::CallLinkInfo::setCallee):
+ (JSC::CallLinkInfo::clearCallee):
+ (JSC::CallLinkInfo::setCodeBlock):
+ (JSC::CallLinkInfo::clearCodeBlock):
+ * bytecode/CallLinkInfo.h:
+ (JSC::CallLinkInfo::clearedByJettison):
+ (JSC::CallLinkInfo::setClearedByJettison):
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::jettison):
+ * jit/Repatch.cpp:
+ (JSC::revertCall):
+
2019-03-27 Yusuke Suzuki <[email protected]>
[JSC] Drop VM and Context cache map in _javascript_Core.framework
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (243625 => 243626)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-03-28 22:05:34 UTC (rev 243626)
@@ -61,7 +61,7 @@
, m_clearedByGC(false)
, m_clearedByVirtual(false)
, m_allowStubs(true)
- , m_isLinked(false)
+ , m_clearedByJettison(false)
, m_callType(None)
, m_calleeGPR(255)
, m_maxNumArguments(0)
@@ -127,7 +127,6 @@
RELEASE_ASSERT(!isDirect());
MacroAssembler::repatchPointer(hotPathBegin(), callee);
m_calleeOrCodeBlock.set(vm, owner, callee);
- m_isLinked = true;
}
void CallLinkInfo::clearCallee()
@@ -135,7 +134,6 @@
RELEASE_ASSERT(!isDirect());
MacroAssembler::repatchPointer(hotPathBegin(), nullptr);
m_calleeOrCodeBlock.clear();
- m_isLinked = false;
}
JSObject* CallLinkInfo::callee()
@@ -148,7 +146,6 @@
{
RELEASE_ASSERT(isDirect());
m_calleeOrCodeBlock.setMayBeNull(vm, owner, codeBlock);
- m_isLinked = true;
}
void CallLinkInfo::clearCodeBlock()
@@ -155,7 +152,6 @@
{
RELEASE_ASSERT(isDirect());
m_calleeOrCodeBlock.clear();
- m_isLinked = false;
}
FunctionCodeBlock* CallLinkInfo::codeBlock()
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h (243625 => 243626)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-03-28 22:05:34 UTC (rev 243626)
@@ -270,10 +270,20 @@
return m_clearedByVirtual;
}
+ bool clearedByJettison()
+ {
+ return m_clearedByJettison;
+ }
+
void setClearedByVirtual()
{
m_clearedByVirtual = true;
}
+
+ void setClearedByJettison()
+ {
+ m_clearedByJettison = true;
+ }
void setCallType(CallType callType)
{
@@ -350,7 +360,7 @@
bool m_clearedByGC : 1;
bool m_clearedByVirtual : 1;
bool m_allowStubs : 1;
- bool m_isLinked : 1;
+ bool m_clearedByJettison : 1;
unsigned m_callType : 4; // CallType
unsigned m_calleeGPR : 8;
uint32_t m_maxNumArguments; // For varargs: the profiled maximum number of arguments. For direct: the number of stack slots allocated for arguments.
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (243625 => 243626)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-03-28 22:05:34 UTC (rev 243626)
@@ -2049,6 +2049,16 @@
if (vm.heap.isCurrentThreadBusy() && !vm.heap.isMarked(ownerExecutable()))
return;
+#if ENABLE(JIT)
+ {
+ ConcurrentJSLocker locker(m_lock);
+ if (JITData* jitData = m_jitData.get()) {
+ for (CallLinkInfo* callLinkInfo : jitData->m_callLinkInfos)
+ callLinkInfo->setClearedByJettison();
+ }
+ }
+#endif
+
// This accomplishes (2).
ownerExecutable()->installCode(vm, alternative(), codeType(), specializationKind());
Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (243625 => 243626)
--- trunk/Source/_javascript_Core/jit/Repatch.cpp 2019-03-28 22:03:40 UTC (rev 243625)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp 2019-03-28 22:05:34 UTC (rev 243626)
@@ -895,18 +895,20 @@
static void revertCall(VM* vm, CallLinkInfo& callLinkInfo, MacroAssemblerCodeRef<JITStubRoutinePtrTag> codeRef)
{
- if (callLinkInfo.isDirect()) {
- callLinkInfo.clearCodeBlock();
- if (callLinkInfo.callType() == CallLinkInfo::DirectTailCall)
- MacroAssembler::repatchJump(callLinkInfo.patchableJump(), callLinkInfo.slowPathStart());
- else
- MacroAssembler::repatchNearCall(callLinkInfo.hotPathOther(), callLinkInfo.slowPathStart());
- } else {
- MacroAssembler::revertJumpReplacementToBranchPtrWithPatch(
- MacroAssembler::startOfBranchPtrWithPatchOnRegister(callLinkInfo.hotPathBegin()),
- static_cast<MacroAssembler::RegisterID>(callLinkInfo.calleeGPR()), 0);
- linkSlowFor(vm, callLinkInfo, codeRef);
- callLinkInfo.clearCallee();
+ if (!callLinkInfo.clearedByJettison()) {
+ if (callLinkInfo.isDirect()) {
+ callLinkInfo.clearCodeBlock();
+ if (callLinkInfo.callType() == CallLinkInfo::DirectTailCall)
+ MacroAssembler::repatchJump(callLinkInfo.patchableJump(), callLinkInfo.slowPathStart());
+ else
+ MacroAssembler::repatchNearCall(callLinkInfo.hotPathOther(), callLinkInfo.slowPathStart());
+ } else {
+ MacroAssembler::revertJumpReplacementToBranchPtrWithPatch(
+ MacroAssembler::startOfBranchPtrWithPatchOnRegister(callLinkInfo.hotPathBegin()),
+ static_cast<MacroAssembler::RegisterID>(callLinkInfo.calleeGPR()), 0);
+ linkSlowFor(vm, callLinkInfo, codeRef);
+ callLinkInfo.clearCallee();
+ }
}
callLinkInfo.clearSeen();
callLinkInfo.clearStub();
