- Revision
- 236136
- Author
- carlo...@webkit.org
- Date
- 2018-09-18 08:40:09 -0700 (Tue, 18 Sep 2018)
Log Message
Merge r235605 - The watchdog sometimes fails to terminate a script.
https://bugs.webkit.org/show_bug.cgi?id=189227
<rdar://problem/39932857>
Reviewed by Saam Barati.
JSTests:
* stress/regress-189227-watchdog-on-infinite-loop.js: Added.
Source/_javascript_Core:
Consider the following scenario:
1. We have an infinite loop bytecode sequence as follows:
[ 13] loop_hint
[ 14] check_traps
[ 15] jmp -2(->13)
2. The VM tiers up from LLInt -> BaselineJIT -> DFG -> FTL.
Note that op_check_traps is represented as a CheckTraps node in the DFG and FTL.
When we're not using pollingTraps (JSC_usePollingTraps is false by default),
we emit no code for CheckTraps, but only record an InvalidationPoint there.
3. The watchdog fires, and invalidates all InvalidationPoints in the FTL CodeBlock.
InvalidationPoints OSR exits to the next instruction by design. In this case,
that means the VM will resumes executing at the op_jmp, which jumps to the
op_loop_hint opcode. At the loop_hint, the VM discovers that the function is
already hot, and attempts to tier up. It immediately discovers that a replacement
CodeBlock is available because we still haven't jettisoned the DFG CodeBlock
nor the FTL CodeBlock that was previously compiled for this function.
Note that jettisoning a CodeBlock necessarily means the VM will invalidate
its InvalidationPoints (if the CodeBlock is DFG/FTL). However, the reverse
is not true: merely invalidating the InvalidationPoints does not necessarily
mean that the CodeBlock is jettisoned.
VMTraps::tryInstallTrapBreakpoints() runs from a separate thread. Hence,
it is only safe for it to invalidate a CodeBlock's InvalidationPoints. It
is not safe for the CodeBlock to be jettisoned from another thread. Instead,
the VMTraps mechanism relies on the script thread running to an op_check_traps
in the baseline JIT code where it will do the necessary jettisoning of optimized
CodeBlocks.
Since the op_check_traps never get executed, the VM will perpetually tier up in
the op_loop_hint, OSR exit to the op_jmp, jump to the op_loop_hint, and repeat.
Consequently, the watchdog fails to terminate this script.
In this patch, we fix this by making the DFG BytecodeParser emit an InvalidationPoint
node directly (when the VM is not configured to use polling traps). This ensures
that the check traps invalidation point will OSR exit to the op_check_traps opcode
in the baseline JIT.
In this patch, we also change VMTraps::tryInstallTrapBreakpoints() to use
CallFrame::unsafeCodeBlock() instead of CallFrame::codeBlock(). This is because
we don't really know if the frame is properly set up. We're just conservatively
probing the stack. ASAN does not like this probing. Using unsafeCodeBlock() here
will suppress the false positive ASAN complaint.
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCheckTraps):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
* runtime/VMTraps.cpp:
(JSC::VMTraps::tryInstallTrapBreakpoints):
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/JSTests/ChangeLog 2018-09-18 15:40:09 UTC (rev 236136)
@@ -1,3 +1,13 @@
+2018-09-03 Mark Lam <mark....@apple.com>
+
+ The watchdog sometimes fails to terminate a script.
+ https://bugs.webkit.org/show_bug.cgi?id=189227
+ <rdar://problem/39932857>
+
+ Reviewed by Saam Barati.
+
+ * stress/regress-189227-watchdog-on-infinite-loop.js: Added.
+
2018-08-24 Yusuke Suzuki <yusukesuz...@slowstart.org>
Function object should convert params to string before throw a parsing error
Added: releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189227-watchdog-on-infinite-loop.js (0 => 236136)
--- releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189227-watchdog-on-infinite-loop.js (rev 0)
+++ releases/WebKitGTK/webkit-2.22/JSTests/stress/regress-189227-watchdog-on-infinite-loop.js 2018-09-18 15:40:09 UTC (rev 236136)
@@ -0,0 +1,4 @@
+//@ requireOptions("--watchdog=20", "--jitPolicyScale=0", "--watchdog-exception-ok")
+
+// This test should not time out.
+while (1) { }
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ChangeLog 2018-09-18 15:40:09 UTC (rev 236136)
@@ -1,5 +1,81 @@
2018-09-03 Mark Lam <mark....@apple.com>
+ The watchdog sometimes fails to terminate a script.
+ https://bugs.webkit.org/show_bug.cgi?id=189227
+ <rdar://problem/39932857>
+
+ Reviewed by Saam Barati.
+
+ Consider the following scenario:
+
+ 1. We have an infinite loop bytecode sequence as follows:
+
+ [ 13] loop_hint
+ [ 14] check_traps
+ [ 15] jmp -2(->13)
+
+ 2. The VM tiers up from LLInt -> BaselineJIT -> DFG -> FTL.
+
+ Note that op_check_traps is represented as a CheckTraps node in the DFG and FTL.
+ When we're not using pollingTraps (JSC_usePollingTraps is false by default),
+ we emit no code for CheckTraps, but only record an InvalidationPoint there.
+
+ 3. The watchdog fires, and invalidates all InvalidationPoints in the FTL CodeBlock.
+
+ InvalidationPoints OSR exits to the next instruction by design. In this case,
+ that means the VM will resumes executing at the op_jmp, which jumps to the
+ op_loop_hint opcode. At the loop_hint, the VM discovers that the function is
+ already hot, and attempts to tier up. It immediately discovers that a replacement
+ CodeBlock is available because we still haven't jettisoned the DFG CodeBlock
+ nor the FTL CodeBlock that was previously compiled for this function.
+
+ Note that jettisoning a CodeBlock necessarily means the VM will invalidate
+ its InvalidationPoints (if the CodeBlock is DFG/FTL). However, the reverse
+ is not true: merely invalidating the InvalidationPoints does not necessarily
+ mean that the CodeBlock is jettisoned.
+
+ VMTraps::tryInstallTrapBreakpoints() runs from a separate thread. Hence,
+ it is only safe for it to invalidate a CodeBlock's InvalidationPoints. It
+ is not safe for the CodeBlock to be jettisoned from another thread. Instead,
+ the VMTraps mechanism relies on the script thread running to an op_check_traps
+ in the baseline JIT code where it will do the necessary jettisoning of optimized
+ CodeBlocks.
+
+ Since the op_check_traps never get executed, the VM will perpetually tier up in
+ the op_loop_hint, OSR exit to the op_jmp, jump to the op_loop_hint, and repeat.
+ Consequently, the watchdog fails to terminate this script.
+
+ In this patch, we fix this by making the DFG BytecodeParser emit an InvalidationPoint
+ node directly (when the VM is not configured to use polling traps). This ensures
+ that the check traps invalidation point will OSR exit to the op_check_traps opcode
+ in the baseline JIT.
+
+ In this patch, we also change VMTraps::tryInstallTrapBreakpoints() to use
+ CallFrame::unsafeCodeBlock() instead of CallFrame::codeBlock(). This is because
+ we don't really know if the frame is properly set up. We're just conservatively
+ probing the stack. ASAN does not like this probing. Using unsafeCodeBlock() here
+ will suppress the false positive ASAN complaint.
+
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::compileCheckTraps):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ * runtime/VMTraps.cpp:
+ (JSC::VMTraps::tryInstallTrapBreakpoints):
+
+2018-09-03 Mark Lam <mark....@apple.com>
+
CallFrame::unsafeCallee() should use an ASAN suppressed Register::asanUnsafePointer().
https://bugs.webkit.org/show_bug.cgi?id=189247
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -6405,7 +6405,7 @@
}
case op_check_traps: {
- addToGraph(CheckTraps);
+ addToGraph(Options::usePollingTraps() ? CheckTraps : InvalidationPoint);
NEXT_OPCODE(op_check_traps);
}
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGClobberize.h (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGClobberize.h 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGClobberize.h 2018-09-18 15:40:09 UTC (rev 236136)
@@ -471,11 +471,8 @@
return;
case CheckTraps:
- if (Options::usePollingTraps()) {
- read(InternalState);
- write(InternalState);
- } else
- write(Watchpoint_fire);
+ read(InternalState);
+ write(InternalState);
return;
case InvalidationPoint:
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -1612,7 +1612,6 @@
case CheckTierUpInLoop:
case CheckTierUpAtReturn:
case CheckTierUpAndOSREnter:
- case InvalidationPoint:
case CheckArray:
case CheckInBounds:
case ConstantStoragePointer:
@@ -2241,6 +2240,7 @@
case FilterGetByIdStatus:
case FilterPutByIdStatus:
case FilterInByIdStatus:
+ case InvalidationPoint:
break;
#else
default:
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -1105,7 +1105,6 @@
case CheckTierUpInLoop:
case CheckTierUpAtReturn:
case CheckTierUpAndOSREnter:
- case InvalidationPoint:
case CheckInBounds:
case ValueToInt32:
case DoubleRep:
@@ -1230,6 +1229,7 @@
case FilterInByIdStatus:
case ClearCatchLocals:
case DataViewSet:
+ case InvalidationPoint:
break;
// This gets ignored because it only pretends to produce a value.
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -2028,7 +2028,7 @@
}
}
-void SpeculativeJIT::compileCheckTraps(Node*)
+void SpeculativeJIT::compileCheckTraps(Node* node)
{
ASSERT(Options::usePollingTraps());
GPRTemporary unused(this);
@@ -2038,6 +2038,7 @@
JITCompiler::AbsoluteAddress(m_jit.vm()->needTrapHandlingAddress()));
addSlowPathGenerator(slowPathCall(needTrapHandling, this, operationHandleTraps, unusedGPR));
+ noResult(node);
}
void SpeculativeJIT::compileDoublePutByVal(Node* node, SpeculateCellOperand& base, SpeculateStrictInt32Operand& property)
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -3964,10 +3964,7 @@
break;
case CheckTraps:
- if (Options::usePollingTraps())
- compileCheckTraps(node);
- else
- noResult(node); // This is a no-op.
+ compileCheckTraps(node);
break;
case CountExecution:
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -4445,10 +4445,7 @@
break;
case CheckTraps:
- if (Options::usePollingTraps())
- compileCheckTraps(node);
- else
- noResult(node); // This is a no-op.
+ compileCheckTraps(node);
break;
case Phantom:
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -1216,8 +1216,7 @@
compileMaterializeCreateActivation();
break;
case CheckTraps:
- if (Options::usePollingTraps())
- compileCheckTraps();
+ compileCheckTraps();
break;
case CreateRest:
compileCreateRest();
Modified: releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/VMTraps.cpp (236135 => 236136)
--- releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/VMTraps.cpp 2018-09-18 15:40:00 UTC (rev 236135)
+++ releases/WebKitGTK/webkit-2.22/Source/_javascript_Core/runtime/VMTraps.cpp 2018-09-18 15:40:09 UTC (rev 236136)
@@ -126,7 +126,7 @@
if (!isSaneFrame(callFrame, calleeFrame, entryFrame, stackBounds))
return; // Let the SignalSender try again later.
- CodeBlock* candidateCodeBlock = callFrame->codeBlock();
+ CodeBlock* candidateCodeBlock = callFrame->unsafeCodeBlock();
if (candidateCodeBlock && vm.heap.codeBlockSet().contains(codeBlockSetLocker, candidateCodeBlock)) {
foundCodeBlock = candidateCodeBlock;
break;
