- Revision
- 201936
- Author
- [email protected]
- Date
- 2016-06-10 12:56:18 -0700 (Fri, 10 Jun 2016)
Log Message
The backend should be happy to compile Unreachable even if AI didn't prove it to be unreachable
https://bugs.webkit.org/show_bug.cgi?id=158631
Reviewed by Keith Miller.
We've been slowly making the DFG Unreachable opcode behave like a grown-up. When we first
added it, it was a hack for Throw, and we could always rely on AI proving that Unreachable
was not reachable. But then we started using Unreachable as a proper Unreachable opcode,
like Oops in B3 for example, which has a more nuanced meaning: you use it whenever you
emit code that *you* know will not return, and you need some way of terminating the basic
block. The DFG is not a proof-carrying compiler, and it never will be. So, when you have
proved that something is not reachable, you should be able to use Unreachable even if
there is no guarantee that the compiler will later be able to replicate your proof. This
means that the backend may find itself compiling Unreachable because AI did not prove that
it was unreachable.
Prior to this change, we would crash compiling Unreachable because we would rely on AI
preventing us from reaching Unreachable in the backend. But that's silly! We don't want
users of Unreachable to have to also convince AI that their Unreachable is really
Unreachable.
This fixes crashes on real websites. I couldn't work out how to turn them into a reduced
test.
* assembler/AbortReason.h:
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::emitInvalidationPoint):
(JSC::DFG::SpeculativeJIT::unreachable):
(JSC::DFG::SpeculativeJIT::terminateSpeculativeExecution):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compilePutDynamicVar):
(JSC::FTL::DFG::LowerDFGToB3::compileUnreachable):
(JSC::FTL::DFG::LowerDFGToB3::compareEqObjectOrOtherToObject):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (201935 => 201936)
--- trunk/Source/_javascript_Core/ChangeLog 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-06-10 19:56:18 UTC (rev 201936)
@@ -1,3 +1,45 @@
+2016-06-10 Filip Pizlo <[email protected]>
+
+ The backend should be happy to compile Unreachable even if AI didn't prove it to be unreachable
+ https://bugs.webkit.org/show_bug.cgi?id=158631
+
+ Reviewed by Keith Miller.
+
+ We've been slowly making the DFG Unreachable opcode behave like a grown-up. When we first
+ added it, it was a hack for Throw, and we could always rely on AI proving that Unreachable
+ was not reachable. But then we started using Unreachable as a proper Unreachable opcode,
+ like Oops in B3 for example, which has a more nuanced meaning: you use it whenever you
+ emit code that *you* know will not return, and you need some way of terminating the basic
+ block. The DFG is not a proof-carrying compiler, and it never will be. So, when you have
+ proved that something is not reachable, you should be able to use Unreachable even if
+ there is no guarantee that the compiler will later be able to replicate your proof. This
+ means that the backend may find itself compiling Unreachable because AI did not prove that
+ it was unreachable.
+
+ Prior to this change, we would crash compiling Unreachable because we would rely on AI
+ preventing us from reaching Unreachable in the backend. But that's silly! We don't want
+ users of Unreachable to have to also convince AI that their Unreachable is really
+ Unreachable.
+
+ This fixes crashes on real websites. I couldn't work out how to turn them into a reduced
+ test.
+
+ * assembler/AbortReason.h:
+ * dfg/DFGSpeculativeJIT.cpp:
+ (JSC::DFG::SpeculativeJIT::emitInvalidationPoint):
+ (JSC::DFG::SpeculativeJIT::unreachable):
+ (JSC::DFG::SpeculativeJIT::terminateSpeculativeExecution):
+ * dfg/DFGSpeculativeJIT.h:
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ (JSC::FTL::DFG::LowerDFGToB3::compilePutDynamicVar):
+ (JSC::FTL::DFG::LowerDFGToB3::compileUnreachable):
+ (JSC::FTL::DFG::LowerDFGToB3::compareEqObjectOrOtherToObject):
+
2016-06-09 Alex Christensen <[email protected]>
Clean up _javascript_Core.vcxproj directory after switching to CMake.
Modified: trunk/Source/_javascript_Core/assembler/AbortReason.h (201935 => 201936)
--- trunk/Source/_javascript_Core/assembler/AbortReason.h 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/assembler/AbortReason.h 2016-06-10 19:56:18 UTC (rev 201936)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -57,6 +57,7 @@
DFGNegativeStringLength = 200,
DFGSlowPathGeneratorFellThrough = 210,
DFGUnreachableBasicBlock = 220,
+ DFGUnreachableNode = 225,
DFGUnreasonableOSREntryJumpDestination = 230,
DFGVarargsThrowingPathDidNotThrow = 235,
JITDidReturnFromTailCall = 237,
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (201935 => 201936)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp 2016-06-10 19:56:18 UTC (rev 201936)
@@ -307,6 +307,12 @@
noResult(node);
}
+void SpeculativeJIT::unreachable(Node* node)
+{
+ m_compileOkay = false;
+ m_jit.abortWithReason(DFGUnreachableNode, node->op());
+}
+
void SpeculativeJIT::terminateSpeculativeExecution(ExitKind kind, JSValueRegs jsValueRegs, Node* node)
{
if (!m_compileOkay)
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (201935 => 201936)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2016-06-10 19:56:18 UTC (rev 201936)
@@ -2661,6 +2661,8 @@
void emitInvalidationPoint(Node*);
+ void unreachable(Node*);
+
// Called when we statically determine that a speculation will fail.
void terminateSpeculativeExecution(ExitKind, JSValueRegs, Node*);
void terminateSpeculativeExecution(ExitKind, JSValueRegs, Edge);
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (201935 => 201936)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2016-06-10 19:56:18 UTC (rev 201936)
@@ -5273,7 +5273,7 @@
}
case Unreachable:
- RELEASE_ASSERT_NOT_REACHED();
+ unreachable(node);
break;
case LastNodeType:
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (201935 => 201936)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2016-06-10 19:56:18 UTC (rev 201936)
@@ -4801,7 +4801,7 @@
break;
case Unreachable:
- DFG_CRASH(m_jit.graph(), node, "Unexpected Unreachable node");
+ unreachable(node);
break;
case StoreBarrier: {
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (201935 => 201936)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-06-10 19:42:37 UTC (rev 201935)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-06-10 19:56:18 UTC (rev 201936)
@@ -1000,6 +1000,9 @@
case PutDynamicVar:
compilePutDynamicVar();
break;
+ case Unreachable:
+ compileUnreachable();
+ break;
case PhantomLocal:
case LoopHint:
@@ -7684,6 +7687,18 @@
m_callFrame, lowCell(m_node->child1()), lowJSValue(m_node->child2()), m_out.constIntPtr(uid), m_out.constInt32(m_node->getPutInfo())));
}
+ void compileUnreachable()
+ {
+ // It's so tempting to assert that AI has proved that this is unreachable. But that's
+ // simply not a requirement of the Unreachable opcode at all. If you emit an opcode that
+ // *you* know will not return, then it's fine to end the basic block with Unreachable
+ // after that opcode. You don't have to also prove to AI that your opcode does not return.
+ // Hence, there is nothing to do here but emit code that will crash, so that we catch
+ // cases where you said Unreachable but you lied.
+
+ crash();
+ }
+
void compareEqObjectOrOtherToObject(Edge leftChild, Edge rightChild)
{
LValue rightCell = lowCell(rightChild);
