- Revision
- 145931
- Author
- [email protected]
- Date
- 2013-03-15 12:33:25 -0700 (Fri, 15 Mar 2013)
Log Message
Add runtime check for improper register allocations in DFG
https://bugs.webkit.org/show_bug.cgi?id=112380
Reviewed by Geoffrey Garen.
Source/_javascript_Core:
Added framework to check for register allocation within a branch source - target range. All register allocations
are saved using the offset in the code stream where the allocation occurred. Later when a jump is linked, the
currently saved register allocations are checked to make sure that they didn't occur in the range of code that was
jumped over. This protects against the case where an allocation could have spilled register contents to free up
a register and that spill only occurs on one path of a many through the code. A subsequent fill of the spilled
register may load garbage. See https://bugs.webkit.org/show_bug.cgi?id=111777 for one such bug.
This code is protected by the compile time check of #if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION).
The check is only done during the processing of SpeculativeJIT::compile(Node* node) and its callees.
* assembler/AbstractMacroAssembler.h:
(JSC::AbstractMacroAssembler::Jump::link): Invoke register allocation checks using source and target of link.
(JSC::AbstractMacroAssembler::Jump::linkTo): Invoke register allocation checks using source and target of link.
(AbstractMacroAssembler):
(RegisterAllocationOffset): New helper class to store the instruction stream offset and compare against a
jump range.
(JSC::AbstractMacroAssembler::RegisterAllocationOffset::RegisterAllocationOffset):
(JSC::AbstractMacroAssembler::RegisterAllocationOffset::check):
(JSC::AbstractMacroAssembler::addRegisterAllocationAtOffset):
(JSC::AbstractMacroAssembler::clearRegisterAllocationOffsets):
(JSC::AbstractMacroAssembler::checkRegisterAllocationAgainstBranchRange):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::allocate):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
Source/WTF:
* wtf/Platform.h: Added new ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION compilation flag to
enable generation of register allocation checking. This is on for debug builds.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (145930 => 145931)
--- trunk/Source/_javascript_Core/ChangeLog 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-03-15 19:33:25 UTC (rev 145931)
@@ -1,3 +1,37 @@
+2013-03-15 Michael Saboff <[email protected]>
+
+ Add runtime check for improper register allocations in DFG
+ https://bugs.webkit.org/show_bug.cgi?id=112380
+
+ Reviewed by Geoffrey Garen.
+
+ Added framework to check for register allocation within a branch source - target range. All register allocations
+ are saved using the offset in the code stream where the allocation occurred. Later when a jump is linked, the
+ currently saved register allocations are checked to make sure that they didn't occur in the range of code that was
+ jumped over. This protects against the case where an allocation could have spilled register contents to free up
+ a register and that spill only occurs on one path of a many through the code. A subsequent fill of the spilled
+ register may load garbage. See https://bugs.webkit.org/show_bug.cgi?id=111777 for one such bug.
+ This code is protected by the compile time check of #if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION).
+ The check is only done during the processing of SpeculativeJIT::compile(Node* node) and its callees.
+
+ * assembler/AbstractMacroAssembler.h:
+ (JSC::AbstractMacroAssembler::Jump::link): Invoke register allocation checks using source and target of link.
+ (JSC::AbstractMacroAssembler::Jump::linkTo): Invoke register allocation checks using source and target of link.
+ (AbstractMacroAssembler):
+ (RegisterAllocationOffset): New helper class to store the instruction stream offset and compare against a
+ jump range.
+ (JSC::AbstractMacroAssembler::RegisterAllocationOffset::RegisterAllocationOffset):
+ (JSC::AbstractMacroAssembler::RegisterAllocationOffset::check):
+ (JSC::AbstractMacroAssembler::addRegisterAllocationAtOffset):
+ (JSC::AbstractMacroAssembler::clearRegisterAllocationOffsets):
+ (JSC::AbstractMacroAssembler::checkRegisterAllocationAgainstBranchRange):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::allocate):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+
2013-03-14 Oliver Hunt <[email protected]>
REGRESSION(r145000): Crash loading arstechnica.com when Safari Web Inspector is open
Modified: trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h (145930 => 145931)
--- trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/_javascript_Core/assembler/AbstractMacroAssembler.h 2013-03-15 19:33:25 UTC (rev 145931)
@@ -538,6 +538,10 @@
void link(AbstractMacroAssembler<AssemblerType>* masm) const
{
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ masm->checkRegisterAllocationAgainstBranchRange(m_label.m_offset, masm->debugOffset());
+#endif
+
#if CPU(ARM_THUMB2)
masm->m_assembler.linkJump(m_label, masm->m_assembler.label(), m_type, m_condition);
#elif CPU(SH4)
@@ -549,6 +553,10 @@
void linkTo(Label label, AbstractMacroAssembler<AssemblerType>* masm) const
{
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ masm->checkRegisterAllocationAgainstBranchRange(label.m_label.m_offset, m_label.m_offset);
+#endif
+
#if CPU(ARM_THUMB2)
masm->m_assembler.linkJump(m_label, label.m_label, m_type, m_condition);
#else
@@ -683,6 +691,44 @@
return Label(this);
}
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ class RegisterAllocationOffset {
+ public:
+ RegisterAllocationOffset(unsigned offset)
+ : m_offset(offset)
+ {
+ }
+
+ void check(unsigned low, unsigned high)
+ {
+ RELEASE_ASSERT_WITH_MESSAGE(!(low <= m_offset && m_offset <= high), "Unsafe branch over register allocation at instruction offset %u in jump offset range %u..%u", m_offset, low, high);
+ }
+
+ private:
+ unsigned m_offset;
+ };
+
+ void addRegisterAllocationAtOffset(unsigned offset)
+ {
+ m_registerAllocationForOffsets.append(RegisterAllocationOffset(offset));
+ }
+
+ void clearRegisterAllocationOffsets()
+ {
+ m_registerAllocationForOffsets.clear();
+ }
+
+ void checkRegisterAllocationAgainstBranchRange(unsigned offset1, unsigned offset2)
+ {
+ if (offset1 > offset2)
+ std::swap(offset1, offset2);
+
+ size_t size = m_registerAllocationForOffsets.size();
+ for (size_t i = 0; i < size; ++i)
+ m_registerAllocationForOffsets[i].check(offset1, offset2);
+ }
+#endif
+
template<typename T, typename U>
static ptrdiff_t differenceBetween(T from, U to)
{
@@ -715,6 +761,10 @@
WeakRandom m_randomSource;
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ Vector<RegisterAllocationOffset, 10> m_registerAllocationForOffsets;
+#endif
+
#if ENABLE(JIT_CONSTANT_BLINDING)
static bool scratchRegisterForBlinding() { return false; }
static bool shouldBlindForSpecificArch(uint32_t) { return true; }
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h (145930 => 145931)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.h 2013-03-15 19:33:25 UTC (rev 145931)
@@ -173,6 +173,9 @@
// Allocate a gpr/fpr.
GPRReg allocate()
{
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.addRegisterAllocationAtOffset(m_jit.debugOffset());
+#endif
VirtualRegister spillMe;
GPRReg gpr = m_gprs.allocate(spillMe);
if (spillMe != InvalidVirtualRegister) {
@@ -188,6 +191,9 @@
}
GPRReg allocate(GPRReg specific)
{
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.addRegisterAllocationAtOffset(m_jit.debugOffset());
+#endif
VirtualRegister spillMe = m_gprs.allocateSpecific(specific);
if (spillMe != InvalidVirtualRegister) {
#if USE(JSVALUE32_64)
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (145930 => 145931)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2013-03-15 19:33:25 UTC (rev 145931)
@@ -1923,6 +1923,10 @@
{
NodeType op = node->op();
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.clearRegisterAllocationOffsets();
+#endif
+
switch (op) {
case JSConstant:
initConstantInfo(node);
@@ -4923,7 +4927,11 @@
RELEASE_ASSERT_NOT_REACHED();
break;
}
-
+
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.clearRegisterAllocationOffsets();
+#endif
+
if (!m_compileOkay)
return;
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (145930 => 145931)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2013-03-15 19:33:25 UTC (rev 145931)
@@ -1888,6 +1888,10 @@
{
NodeType op = node->op();
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.clearRegisterAllocationOffsets();
+#endif
+
switch (op) {
case JSConstant:
initConstantInfo(node);
@@ -4773,6 +4777,10 @@
break;
}
+#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
+ m_jit.clearRegisterAllocationOffsets();
+#endif
+
if (!m_compileOkay)
return;
Modified: trunk/Source/WTF/ChangeLog (145930 => 145931)
--- trunk/Source/WTF/ChangeLog 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/WTF/ChangeLog 2013-03-15 19:33:25 UTC (rev 145931)
@@ -1,3 +1,13 @@
+2013-03-15 Michael Saboff <[email protected]>
+
+ Add runtime check for improper register allocations in DFG
+ https://bugs.webkit.org/show_bug.cgi?id=112380
+
+ Reviewed by Geoffrey Garen.
+
+ * wtf/Platform.h: Added new ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION compilation flag to
+ enable generation of register allocation checking. This is on for debug builds.
+
2013-03-13 Jessie Berlin <[email protected]>
Remove svn:executable from a file that isn't supposed to be executable.
Modified: trunk/Source/WTF/wtf/Platform.h (145930 => 145931)
--- trunk/Source/WTF/wtf/Platform.h 2013-03-15 19:03:39 UTC (rev 145930)
+++ trunk/Source/WTF/wtf/Platform.h 2013-03-15 19:33:25 UTC (rev 145931)
@@ -850,6 +850,16 @@
#define ENABLE_WRITE_BARRIER_PROFILING 0
#endif
+/* Enable verification that that register allocations are not made within generated control flow.
+ Turned on for debug builds. */
+#if !defined(ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION) && ENABLE(DFG_JIT)
+#if !defined(NDEBUG)
+#define ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION 1
+#else
+#define ENABLE_DFG_REGISTER_ALLOCATION_VALIDATION 0
+#endif
+#endif
+
/* Configure the JIT */
#if CPU(X86) && COMPILER(MSVC)
#define JSC_HOST_CALL __fastcall
