Diff
Modified: branches/safari-605-branch/JSTests/ChangeLog (227023 => 227024)
--- branches/safari-605-branch/JSTests/ChangeLog 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/JSTests/ChangeLog 2018-01-17 05:04:07 UTC (rev 227024)
@@ -1,5 +1,23 @@
2018-01-16 Jason Marcell <[email protected]>
+ Cherry-pick r226907. rdar://problem/36567949
+
+ 2018-01-12 Saam Barati <[email protected]>
+
+ CheckStructure can be incorrectly subsumed by CheckStructureOrEmpty
+ https://bugs.webkit.org/show_bug.cgi?id=181177
+ <rdar://problem/36205704>
+
+ Reviewed by Yusuke Suzuki.
+
+ * stress/check-structure-ir-ensures-empty-does-not-flow-through.js: Added.
+ (runNearStackLimit.t):
+ (runNearStackLimit):
+ (test.f):
+ (test):
+
+2018-01-16 Jason Marcell <[email protected]>
+
Cherry-pick r226881. rdar://problem/36567948
2018-01-12 Saam Barati <[email protected]>
Added: branches/safari-605-branch/JSTests/stress/check-structure-ir-ensures-empty-does-not-flow-through.js (0 => 227024)
--- branches/safari-605-branch/JSTests/stress/check-structure-ir-ensures-empty-does-not-flow-through.js (rev 0)
+++ branches/safari-605-branch/JSTests/stress/check-structure-ir-ensures-empty-does-not-flow-through.js 2018-01-17 05:04:07 UTC (rev 227024)
@@ -0,0 +1,26 @@
+function runNearStackLimit(f) {
+ function t() {
+ try {
+ return t();
+ } catch (e) {
+ return f();
+ }
+ }
+ return t()
+}
+
+function test() {
+ function f(arg) {
+ let loc = arg;
+ try {
+ loc.p = 0;
+ } catch (e) {}
+ arg.p = 1;
+ }
+ let obj = {};
+ runNearStackLimit(() => {
+ return f(obj);
+ });
+}
+for (let i = 0; i < 50; ++i)
+ test();
Modified: branches/safari-605-branch/Source/_javascript_Core/ChangeLog (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/ChangeLog 2018-01-17 05:04:07 UTC (rev 227024)
@@ -1,5 +1,95 @@
2018-01-16 Jason Marcell <[email protected]>
+ Cherry-pick r226907. rdar://problem/36567949
+
+ 2018-01-12 Saam Barati <[email protected]>
+
+ CheckStructure can be incorrectly subsumed by CheckStructureOrEmpty
+ https://bugs.webkit.org/show_bug.cgi?id=181177
+ <rdar://problem/36205704>
+
+ Reviewed by Yusuke Suzuki.
+
+ The semantics of CheckStructure are such that it does not allow the empty value to flow through it.
+ However, we may eliminate a CheckStructure if it's preceded by a CheckStructureOrEmpty. This doesn't
+ have semantic consequences when validation is turned off. However, with validation on, this trips up
+ our OSR exit machinery that says when an exit is allowed to happen.
+
+ Consider the following IR:
+
+ a: GetClosureVar // Or any other node that produces BytecodeTop
+ ...
+ c: CheckStructure(Cell:@a, {s2})
+ d: PutByOffset(KnownCell:@a, KnownCell:@a, @value)
+
+ In the TypeCheckHoistingPhase, we may insert CheckStructureOrEmptys like this:
+ a: GetClosureVar
+ e: CheckStructureOrEmpty(@a, {s1})
+ ...
+ f: CheckStructureOrEmpty(@a, {s2})
+ c: CheckStructure(Cell:@a, {s2})
+ d: PutByOffset(KnownCell:@a, KnownCell:@a, @value)
+
+ This will cause constant folding to change the IR to:
+ a: GetClosureVar
+ e: CheckStructureOrEmpty(@a, {s1})
+ ...
+ f: CheckStructureOrEmpty(@a, {s2})
+ d: PutByOffset(KnownCell:@a, KnownCell:@a, @value)
+
+ Our mayExit analysis determines that the PutByOffset should not exit. Note
+ that AI will determine the only value the PutByOffset can see in @a is
+ the empty value. Because KnownCell filters SpecCell and not SpecCellCheck,
+ when lowering the PutByOffset, we reach a contradiction in AI and emit
+ an OSR exit. However, because mayExit said we couldn't exit, we assert.
+
+ Note that if we did not run the TypeCheckHoistingPhase on this IR, AI
+ would have determined we would OSR exit at the second CheckStructure.
+
+ This patch makes it so constant folding produces the following IR:
+ a: GetClosureVar
+ e: CheckStructureOrEmpty(@a, {s1})
+ g: AssertNotEmpty(@a)
+ ...
+ f: CheckStructureOrEmpty(@a, {s2})
+ h: AssertNotEmpty(@a)
+ d: PutByOffset(KnownCell:@a, KnownCell:@a, @value)
+
+ This modification will cause AI to know we will OSR exit before even reaching
+ the PutByOffset. Note that in the original IR, the GetClosureVar won't
+ actually produce the TDZ value. If it did, bytecode would have caused us
+ to emit a CheckNotEmpty before the CheckStructure/PutByOffset combo. That's
+ why this bug is about IR bookkeeping and not an actual error in IR analysis.
+ This patch introduces AssertNotEmpty instead of using CheckNotEmpty to be
+ more congruous with CheckStructure's semantics of crashing on the empty value
+ as input (on 64 bit platforms).
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGConstantFoldingPhase.cpp:
+ (JSC::DFG::ConstantFoldingPhase::foldConstants):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ (JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty):
+
+2018-01-16 Jason Marcell <[email protected]>
+
Cherry-pick r226895. rdar://problem/36568085
2018-01-12 Joseph Pecoraro <[email protected]>
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2018-01-17 05:04:07 UTC (rev 227024)
@@ -2995,6 +2995,7 @@
break;
}
+ case AssertNotEmpty:
case CheckNotEmpty: {
AbstractValue& value = forNode(node->child1());
if (!(value.m_type & SpecEmpty)) {
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGClobberize.h 2018-01-17 05:04:07 UTC (rev 227024)
@@ -417,6 +417,10 @@
def(PureValue(CheckNotEmpty, AdjacencyList(AdjacencyList::Fixed, node->child1())));
return;
+ case AssertNotEmpty:
+ write(SideState);
+ return;
+
case CheckStringIdent:
def(PureValue(CheckStringIdent, AdjacencyList(AdjacencyList::Fixed, node->child1()), node->uidOperand()));
return;
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGConstantFoldingPhase.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -175,8 +175,13 @@
RegisteredStructureSet set;
if (node->op() == ArrayifyToStructure)
set = node->structure();
- else
+ else {
set = node->structureSet();
+ if ((SpecCellCheck & SpecEmpty) && node->child1().useKind() == CellUse && m_state.forNode(node->child1()).m_type & SpecEmpty) {
+ m_insertionSet.insertNode(
+ indexInBlock, SpecNone, AssertNotEmpty, node->origin, Edge(node->child1().node(), UntypedUse));
+ }
+ }
if (value.m_structure.isSubsetOf(set)) {
m_interpreter.execute(indexInBlock); // Catch the fact that we may filter on cell.
node->remove();
@@ -293,6 +298,7 @@
break;
}
+ case AssertNotEmpty:
case CheckNotEmpty: {
if (m_state.forNode(node->child1()).m_type & SpecEmpty)
break;
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGDoesGC.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -134,6 +134,7 @@
case PutGlobalVariable:
case CheckCell:
case CheckNotEmpty:
+ case AssertNotEmpty:
case CheckStringIdent:
case RegExpExec:
case RegExpTest:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -2138,6 +2138,7 @@
case ForceOSRExit:
case CheckBadCell:
case CheckNotEmpty:
+ case AssertNotEmpty:
case CheckTraps:
case Unreachable:
case ExtractOSREntryLocal:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGNodeType.h 2018-01-17 05:04:07 UTC (rev 227024)
@@ -239,6 +239,7 @@
macro(RecordRegExpCachedResult, NodeMustGenerate | NodeHasVarArgs) \
macro(CheckCell, NodeMustGenerate) \
macro(CheckNotEmpty, NodeMustGenerate) \
+ macro(AssertNotEmpty, NodeMustGenerate) \
macro(CheckBadCell, NodeMustGenerate) \
macro(CheckInBounds, NodeMustGenerate) \
macro(CheckStringIdent, NodeMustGenerate) \
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -1162,6 +1162,7 @@
case CheckStructure:
case CheckCell:
case CheckNotEmpty:
+ case AssertNotEmpty:
case CheckStringIdent:
case CheckBadCell:
case PutStructure:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSafeToExecute.h 2018-01-17 05:04:07 UTC (rev 227024)
@@ -258,6 +258,7 @@
case CheckCell:
case CheckBadCell:
case CheckNotEmpty:
+ case AssertNotEmpty:
case CheckStringIdent:
case RegExpExec:
case RegExpTest:
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -5181,6 +5181,7 @@
case InitializeEntrypointArguments:
case EntrySwitch:
case CPUIntrinsic:
+ case AssertNotEmpty:
DFG_CRASH(m_jit.graph(), node, "unexpected node in DFG backend");
break;
}
Modified: branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -4215,6 +4215,18 @@
break;
}
+ case AssertNotEmpty: {
+ if (validationEnabled()) {
+ JSValueOperand operand(this, node->child1());
+ GPRReg input = operand.gpr();
+ auto done = m_jit.branchTest64(MacroAssembler::NonZero, input);
+ m_jit.breakpoint();
+ done.link(&m_jit);
+ }
+ noResult(node);
+ break;
+ }
+
case CheckStringIdent:
compileCheckStringIdent(node);
break;
Modified: branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/ftl/FTLCapabilities.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -136,6 +136,7 @@
case CheckCell:
case CheckBadCell:
case CheckNotEmpty:
+ case AssertNotEmpty:
case CheckStringIdent:
case CheckTraps:
case StringCharCodeAt:
Modified: branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (227023 => 227024)
--- branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-17 05:04:02 UTC (rev 227023)
+++ branches/safari-605-branch/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2018-01-17 05:04:07 UTC (rev 227024)
@@ -670,6 +670,9 @@
case CheckNotEmpty:
compileCheckNotEmpty();
break;
+ case AssertNotEmpty:
+ compileAssertNotEmpty();
+ break;
case CheckBadCell:
compileCheckBadCell();
break;
@@ -2892,6 +2895,23 @@
speculate(TDZFailure, noValue(), nullptr, m_out.isZero64(lowJSValue(m_node->child1())));
}
+ void compileAssertNotEmpty()
+ {
+ if (!validationEnabled())
+ return;
+
+ B3::PatchpointValue* patchpoint = m_out.patchpoint(Void);
+ patchpoint->appendSomeRegister(lowJSValue(m_node->child1()));
+ patchpoint->setGenerator(
+ [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ AllowMacroScratchRegisterUsage allowScratch(jit);
+ GPRReg input = params[0].gpr();
+ CCallHelpers::Jump done = jit.branchTest64(CCallHelpers::NonZero, input);
+ jit.breakpoint();
+ done.link(&jit);
+ });
+ }
+
void compileCheckStringIdent()
{
UniquedStringImpl* uid = m_node->uidOperand();
