- Revision
- 248215
- Author
- [email protected]
- Date
- 2019-08-03 20:22:28 -0700 (Sat, 03 Aug 2019)
Log Message
Merge r243626 - 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: releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/JSTests/ChangeLog 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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-06-21 Mark Lam <[email protected]>
ArraySlice needs to keep the source array alive.
Added: releases/WebKitGTK/webkit-2.24/JSTests/stress/call-link-info-osrexit-repatch.js (0 => 248215)
--- releases/WebKitGTK/webkit-2.24/JSTests/stress/call-link-info-osrexit-repatch.js (rev 0)
+++ releases/WebKitGTK/webkit-2.24/JSTests/stress/call-link-info-osrexit-repatch.js 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/ChangeLog 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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-20 Michael Saboff <[email protected]>
JSC test crash: stress/dont-strength-reduce-regexp-with-compile-error.js.default
Modified: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.h (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CodeBlock.cpp (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/bytecode/CodeBlock.cpp 2019-08-04 03:22:28 UTC (rev 248215)
@@ -2044,6 +2044,16 @@
if (vm.heap.isCurrentThreadBusy() && !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: releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/jit/Repatch.cpp (248214 => 248215)
--- releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/jit/Repatch.cpp 2019-08-04 03:22:24 UTC (rev 248214)
+++ releases/WebKitGTK/webkit-2.24/Source/_javascript_Core/jit/Repatch.cpp 2019-08-04 03:22:28 UTC (rev 248215)
@@ -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();
