Diff
Modified: trunk/JSTests/ChangeLog (242191 => 242192)
--- trunk/JSTests/ChangeLog 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/JSTests/ChangeLog 2019-02-28 06:25:23 UTC (rev 242192)
@@ -1,3 +1,18 @@
+2019-02-27 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] mustHandleValues for dead bytecode locals should be ignored in DFG phases
+ https://bugs.webkit.org/show_bug.cgi?id=195144
+ <rdar://problem/47595961>
+
+ Reviewed by Mark Lam.
+
+ * stress/read-dead-bytecode-locals-in-must-handle-values1.js: Added.
+ (bar):
+ (foo):
+ * stress/read-dead-bytecode-locals-in-must-handle-values2.js: Added.
+ (bar):
+ (foo):
+
2019-02-26 Yusuke Suzuki <ysuz...@apple.com>
REGRESSION: stress/regress-178386.js is timing out on JSC debug bot
Added: trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values1.js (0 => 242192)
--- trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values1.js (rev 0)
+++ trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values1.js 2019-02-28 06:25:23 UTC (rev 242192)
@@ -0,0 +1,18 @@
+//@ runDefault("--useMaximalFlushInsertionPhase=1", "--useConcurrentJIT=0")
+function bar(x) {
+ if (x) {
+ return;
+ }
+ x = x ** 0;
+ x = x * 2;
+}
+
+function foo() {
+ bar();
+ for (let i = 0; i < 10; ++i) {
+ }
+}
+
+for (var i = 0; i < 10000; ++i) {
+ foo();
+}
Added: trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values2.js (0 => 242192)
--- trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values2.js (rev 0)
+++ trunk/JSTests/stress/read-dead-bytecode-locals-in-must-handle-values2.js 2019-02-28 06:25:23 UTC (rev 242192)
@@ -0,0 +1,19 @@
+//@ runDefault("--useMaximalFlushInsertionPhase=1", "--useConcurrentJIT=0")
+function bar(c) {
+ if (c > 1) {
+ bar(parseInt(c * 2))
+ }
+ c *= 2;
+}
+
+function foo() {
+ var i = 0;
+ if (''.match(/^/)) {
+ while(i < 1e8) {
+ bar(2);
+ ++i;
+ }
+ }
+}
+
+foo();
Modified: trunk/Source/_javascript_Core/ChangeLog (242191 => 242192)
--- trunk/Source/_javascript_Core/ChangeLog 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-02-28 06:25:23 UTC (rev 242192)
@@ -1,3 +1,48 @@
+2019-02-27 Yusuke Suzuki <ysuz...@apple.com>
+
+ [JSC] mustHandleValues for dead bytecode locals should be ignored in DFG phases
+ https://bugs.webkit.org/show_bug.cgi?id=195144
+ <rdar://problem/47595961>
+
+ Reviewed by Mark Lam.
+
+ DFGMaximalFlushInsertionPhase inserts Flush for all the locals at the end of basic blocks. This enlarges the live ranges of
+ locals in DFG, and it sometimes makes DFG value live while it is dead in bytecode. The issue happens when we use mustHandleValues
+ to widen AbstractValue in CFAPhase. At that time, DFG tells "this value is live in DFG", but it may be dead in the bytecode level.
+ At that time, we attempt to merge AbstractValue with dead mustHandleValue, which is cleared as jsUndefined() in
+ DFG::Plan::cleanMustHandleValuesIfNecessary before start compilation, and crash because jsUndefined() may be irrelevant to the FlushFormat
+ in VariableAccessData.
+
+ This patch makes the type of mustHandleValues Operands<Optional<JSValue>>. We clear dead JSValues in DFG::Plan::cleanMustHandleValuesIfNecessary.
+ And we skip handling dead mustHandleValue in DFG phases.
+
+ * bytecode/Operands.h:
+ (JSC::Operands::isLocal const):
+ (JSC::Operands::isVariable const): Deleted.
+ * dfg/DFGCFAPhase.cpp:
+ (JSC::DFG::CFAPhase::injectOSR):
+ * dfg/DFGDriver.cpp:
+ (JSC::DFG::compileImpl):
+ (JSC::DFG::compile):
+ * dfg/DFGDriver.h:
+ * dfg/DFGJITCode.cpp:
+ (JSC::DFG::JITCode::reconstruct):
+ * dfg/DFGJITCode.h:
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::Plan):
+ (JSC::DFG::Plan::checkLivenessAndVisitChildren):
+ (JSC::DFG::Plan::cleanMustHandleValuesIfNecessary):
+ * dfg/DFGPlan.h:
+ (JSC::DFG::Plan::mustHandleValues const):
+ * dfg/DFGPredictionInjectionPhase.cpp:
+ (JSC::DFG::PredictionInjectionPhase::run):
+ * dfg/DFGTypeCheckHoistingPhase.cpp:
+ (JSC::DFG::TypeCheckHoistingPhase::disableHoistingAcrossOSREntries):
+ * ftl/FTLOSREntry.cpp:
+ (JSC::FTL::prepareOSREntry):
+ * jit/JITOperations.cpp:
+
2019-02-27 Simon Fraser <simon.fra...@apple.com>
Roll out r242014; it caused crashes in compositing logging (webkit.org/b/195141)
Modified: trunk/Source/_javascript_Core/bytecode/Operands.h (242191 => 242192)
--- trunk/Source/_javascript_Core/bytecode/Operands.h 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/bytecode/Operands.h 2019-02-28 06:25:23 UTC (rev 242192)
@@ -220,7 +220,7 @@
T& operator[](size_t index) { return at(index); }
bool isArgument(size_t index) const { return index < m_numArguments; }
- bool isVariable(size_t index) const { return !isArgument(index); }
+ bool isLocal(size_t index) const { return !isArgument(index); }
int operandForIndex(size_t index) const
{
if (index < numberOfArguments())
Modified: trunk/Source/_javascript_Core/dfg/DFGCFAPhase.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGCFAPhase.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGCFAPhase.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -163,10 +163,15 @@
dataLog(" Found must-handle block: ", *block, "\n");
bool changed = false;
- const Operands<JSValue>& mustHandleValues = m_graph.m_plan.mustHandleValues();
+ const Operands<Optional<JSValue>>& mustHandleValues = m_graph.m_plan.mustHandleValues();
for (size_t i = mustHandleValues.size(); i--;) {
int operand = mustHandleValues.operandForIndex(i);
- JSValue value = mustHandleValues[i];
+ Optional<JSValue> value = mustHandleValues[i];
+ if (!value) {
+ if (m_verbose)
+ dataLog(" Not live in bytecode: ", VirtualRegister(operand), "\n");
+ continue;
+ }
Node* node = block->variablesAtHead.operand(operand);
if (!node) {
if (m_verbose)
@@ -175,12 +180,12 @@
}
if (m_verbose)
- dataLog(" Widening ", VirtualRegister(operand), " with ", value, "\n");
+ dataLog(" Widening ", VirtualRegister(operand), " with ", value.value(), "\n");
AbstractValue& target = block->valuesAtHead.operand(operand);
- changed |= target.mergeOSREntryValue(m_graph, value);
+ changed |= target.mergeOSREntryValue(m_graph, value.value());
target.fixTypeForRepresentation(
- m_graph, resultFor(node->variableAccessData()->flushFormat()));
+ m_graph, resultFor(node->variableAccessData()->flushFormat()), node);
}
if (changed || !block->cfaHasVisited) {
Modified: trunk/Source/_javascript_Core/dfg/DFGDriver.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGDriver.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGDriver.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -70,7 +70,7 @@
static CompilationResult compileImpl(
VM& vm, CodeBlock* codeBlock, CodeBlock* profiledDFGCodeBlock, CompilationMode mode,
- unsigned osrEntryBytecodeIndex, const Operands<JSValue>& mustHandleValues,
+ unsigned osrEntryBytecodeIndex, const Operands<Optional<JSValue>>& mustHandleValues,
Ref<DeferredCompilationCallback>&& callback)
{
if (!Options::bytecodeRangeToDFGCompile().isInRange(codeBlock->instructionCount())
@@ -116,7 +116,7 @@
}
#else // ENABLE(DFG_JIT)
static CompilationResult compileImpl(
- VM&, CodeBlock*, CodeBlock*, CompilationMode, unsigned, const Operands<JSValue>&,
+ VM&, CodeBlock*, CodeBlock*, CompilationMode, unsigned, const Operands<Optional<JSValue>>&,
Ref<DeferredCompilationCallback>&&)
{
return CompilationFailed;
@@ -125,7 +125,7 @@
CompilationResult compile(
VM& vm, CodeBlock* codeBlock, CodeBlock* profiledDFGCodeBlock, CompilationMode mode,
- unsigned osrEntryBytecodeIndex, const Operands<JSValue>& mustHandleValues,
+ unsigned osrEntryBytecodeIndex, const Operands<Optional<JSValue>>& mustHandleValues,
Ref<DeferredCompilationCallback>&& callback)
{
CompilationResult result = compileImpl(
Modified: trunk/Source/_javascript_Core/dfg/DFGDriver.h (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGDriver.h 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGDriver.h 2019-02-28 06:25:23 UTC (rev 242192)
@@ -42,7 +42,7 @@
// compile. Even if we do a synchronous compile, we call the callback with the result.
CompilationResult compile(
VM&, CodeBlock*, CodeBlock* profiledDFGCodeBlock, CompilationMode,
- unsigned osrEntryBytecodeIndex, const Operands<JSValue>& mustHandleValues,
+ unsigned osrEntryBytecodeIndex, const Operands<Optional<JSValue>>& mustHandleValues,
Ref<DeferredCompilationCallback>&&);
} } // namespace JSC::DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCode.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGJITCode.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCode.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -78,12 +78,12 @@
void JITCode::reconstruct(
ExecState* exec, CodeBlock* codeBlock, CodeOrigin codeOrigin, unsigned streamIndex,
- Operands<JSValue>& result)
+ Operands<Optional<JSValue>>& result)
{
Operands<ValueRecovery> recoveries;
reconstruct(codeBlock, codeOrigin, streamIndex, recoveries);
- result = Operands<JSValue>(OperandsLike, recoveries);
+ result = Operands<Optional<JSValue>>(OperandsLike, recoveries);
for (size_t i = result.size(); i--;)
result[i] = recoveries[i].recover(exec);
}
Modified: trunk/Source/_javascript_Core/dfg/DFGJITCode.h (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGJITCode.h 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCode.h 2019-02-28 06:25:23 UTC (rev 242192)
@@ -98,7 +98,7 @@
// stack. Currently, it also has the restriction that the values must be in their
// bytecode-designated stack slots.
void reconstruct(
- ExecState*, CodeBlock*, CodeOrigin, unsigned streamIndex, Operands<JSValue>& result);
+ ExecState*, CodeBlock*, CodeOrigin, unsigned streamIndex, Operands<Optional<JSValue>>& result);
#if ENABLE(FTL_JIT)
// NB. All of these methods take CodeBlock* because they may want to use
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -3108,7 +3108,7 @@
// We need to compile the code.
compile(
*vm, codeBlock->newReplacement(), codeBlock, FTLMode, UINT_MAX,
- Operands<JSValue>(), ToFTLDeferredCompilationCallback::create());
+ Operands<Optional<JSValue>>(), ToFTLDeferredCompilationCallback::create());
// If we reached here, the counter has not be reset. Do that now.
jitCode->setOptimizationThresholdBasedOnCompilationResult(
@@ -3346,7 +3346,7 @@
JITCode::TriggerReason* triggerAddress = &(triggerIterator->value);
- Operands<JSValue> mustHandleValues;
+ Operands<Optional<JSValue>> mustHandleValues;
unsigned streamIndex = jitCode->bytecodeIndexToStreamIndex.get(originBytecodeIndex);
jitCode->reconstruct(
exec, codeBlock, CodeOrigin(originBytecodeIndex), streamIndex, mustHandleValues);
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -135,7 +135,7 @@
Plan::Plan(CodeBlock* passedCodeBlock, CodeBlock* profiledDFGCodeBlock,
CompilationMode mode, unsigned osrEntryBytecodeIndex,
- const Operands<JSValue>& mustHandleValues)
+ const Operands<Optional<JSValue>>& mustHandleValues)
: m_vm(passedCodeBlock->vm())
, m_codeBlock(passedCodeBlock)
, m_profiledDFGCodeBlock(profiledDFGCodeBlock)
@@ -639,8 +639,11 @@
return;
cleanMustHandleValuesIfNecessary();
- for (unsigned i = m_mustHandleValues.size(); i--;)
- visitor.appendUnbarriered(m_mustHandleValues[i]);
+ for (unsigned i = m_mustHandleValues.size(); i--;) {
+ Optional<JSValue> value = m_mustHandleValues[i];
+ if (value)
+ visitor.appendUnbarriered(value.value());
+ }
m_recordedStatuses.markIfCheap(visitor);
@@ -715,7 +718,7 @@
for (unsigned local = m_mustHandleValues.numberOfLocals(); local--;) {
if (!liveness[local])
- m_mustHandleValues.local(local) = jsUndefined();
+ m_mustHandleValues.local(local) = WTF::nullopt;
}
}
Modified: trunk/Source/_javascript_Core/dfg/DFGPlan.h (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGPlan.h 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGPlan.h 2019-02-28 06:25:23 UTC (rev 242192)
@@ -57,7 +57,7 @@
Plan(
CodeBlock* codeBlockToCompile, CodeBlock* profiledDFGCodeBlock,
CompilationMode, unsigned osrEntryBytecodeIndex,
- const Operands<JSValue>& mustHandleValues);
+ const Operands<Optional<JSValue>>& mustHandleValues);
~Plan();
void compileInThread(ThreadData*);
@@ -88,8 +88,7 @@
bool isFTL() const { return DFG::isFTL(m_mode); }
CompilationMode mode() const { return m_mode; }
unsigned osrEntryBytecodeIndex() const { return m_osrEntryBytecodeIndex; }
- const Operands<JSValue>& mustHandleValues() const { return m_mustHandleValues; }
-
+ const Operands<Optional<JSValue>>& mustHandleValues() const { return m_mustHandleValues; }
ThreadData* threadData() const { return m_threadData; }
Profiler::Compilation* compilation() const { return m_compilation.get(); }
@@ -138,7 +137,7 @@
CompilationMode m_mode;
const unsigned m_osrEntryBytecodeIndex;
- Operands<JSValue> m_mustHandleValues;
+ Operands<Optional<JSValue>> m_mustHandleValues;
bool m_mustHandleValuesMayIncludeGarbage { true };
Lock m_mustHandleValueCleaningLock;
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionInjectionPhase.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -70,15 +70,17 @@
continue;
if (block->bytecodeBegin != m_graph.m_plan.osrEntryBytecodeIndex())
continue;
- const Operands<JSValue>& mustHandleValues = m_graph.m_plan.mustHandleValues();
+ const Operands<Optional<JSValue>>& mustHandleValues = m_graph.m_plan.mustHandleValues();
for (size_t i = 0; i < mustHandleValues.size(); ++i) {
int operand = mustHandleValues.operandForIndex(i);
+ Optional<JSValue> value = mustHandleValues[i];
+ if (!value)
+ continue;
Node* node = block->variablesAtHead.operand(operand);
if (!node)
continue;
ASSERT(node->accessesStack(m_graph));
- node->variableAccessData()->predict(
- speculationFromValue(mustHandleValues[i]));
+ node->variableAccessData()->predict(speculationFromValue(value.value()));
}
}
Modified: trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -444,7 +444,7 @@
continue;
if (block->bytecodeBegin != m_graph.m_plan.osrEntryBytecodeIndex())
continue;
- const Operands<JSValue>& mustHandleValues = m_graph.m_plan.mustHandleValues();
+ const Operands<Optional<JSValue>>& mustHandleValues = m_graph.m_plan.mustHandleValues();
for (size_t i = 0; i < mustHandleValues.size(); ++i) {
int operand = mustHandleValues.operandForIndex(i);
Node* node = block->variablesAtHead.operand(operand);
@@ -456,8 +456,8 @@
continue;
if (!TypeCheck::isValidToHoist(iter->value))
continue;
- JSValue value = mustHandleValues[i];
- if (!value || !value.isCell() || TypeCheck::isContravenedByValue(iter->value, value)) {
+ Optional<JSValue> value = mustHandleValues[i];
+ if (!value || !value.value() || !value.value().isCell() || TypeCheck::isContravenedByValue(iter->value, value.value())) {
TypeCheck::disableHoisting(iter->value);
continue;
}
Modified: trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/ftl/FTLOSREntry.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -64,7 +64,7 @@
return 0;
}
- Operands<JSValue> values;
+ Operands<Optional<JSValue>> values;
dfgCode->reconstruct(
exec, dfgCodeBlock, CodeOrigin(bytecodeIndex), streamIndex, values);
@@ -73,8 +73,8 @@
for (int argument = values.numberOfArguments(); argument--;) {
JSValue valueOnStack = exec->r(virtualRegisterForArgument(argument).offset()).asanUnsafeJSValue();
- JSValue reconstructedValue = values.argument(argument);
- if (valueOnStack == reconstructedValue || !argument)
+ Optional<JSValue> reconstructedValue = values.argument(argument);
+ if ((reconstructedValue && valueOnStack == reconstructedValue.value()) || !argument)
continue;
dataLog("Mismatch between reconstructed values and the value on the stack for argument arg", argument, " for ", *entryCodeBlock, " at bc#", bytecodeIndex, ":\n");
dataLog(" Value on stack: ", valueOnStack, "\n");
@@ -88,8 +88,13 @@
EncodedJSValue* scratch = static_cast<EncodedJSValue*>(
entryCode->entryBuffer()->dataBuffer());
- for (int local = values.numberOfLocals(); local--;)
- scratch[local] = JSValue::encode(values.local(local));
+ for (int local = values.numberOfLocals(); local--;) {
+ Optional<JSValue> value = values.local(local);
+ if (value)
+ scratch[local] = JSValue::encode(value.value());
+ else
+ scratch[local] = JSValue::encode(JSValue());
+ }
int stackFrameSize = entryCode->common.requiredRegisterCountForExecutionAndExit();
if (UNLIKELY(!vm.ensureStackCapacityFor(&exec->registers()[virtualRegisterForLocal(stackFrameSize - 1).offset()]))) {
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (242191 => 242192)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2019-02-28 05:27:43 UTC (rev 242191)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2019-02-28 06:25:23 UTC (rev 242192)
@@ -1542,7 +1542,7 @@
numVarsWithValues = codeBlock->numCalleeLocals();
else
numVarsWithValues = 0;
- Operands<JSValue> mustHandleValues(codeBlock->numParameters(), numVarsWithValues);
+ Operands<Optional<JSValue>> mustHandleValues(codeBlock->numParameters(), numVarsWithValues);
int localsUsedForCalleeSaves = static_cast<int>(CodeBlock::llintBaselineCalleeSaveSpaceAsVirtualRegisters());
for (size_t i = 0; i < mustHandleValues.size(); ++i) {
int operand = mustHandleValues.operandForIndex(i);
