Changeset 171662 in webkit for trunk/Source/JavaScriptCore/dfg

Timestamp:

Jul 27, 2014, 4:35:32 PM (11 years ago)

Author:

[email protected]

Message:

[REGRESSION][ftlopt merge][32-bit] stress/prune-multi-put-by-offset-replace-or-transition-variant.js.dfg-eager hits an assertion in SpeculativeJIT::silentSavePlanForGPR
​https://bugs.webkit.org/show_bug.cgi?id=135323

Reviewed by Oliver Hunt.

SpeculativeJIT::silentSavePlanForGPR likes to believe that if a node is a constant,
then it's a constant that can be represented using that node's current DataFormat.
This doesn't work if the constant had been filled as a JSValue, and then one of the
fillSpeculateBlah() methods had speculated that it's of some type that the constant
isn't. Unless fillSpeculateBlah() specifically defends against this case, we'll have
a constant that claims to have a contradictory data format.

This patch fixes such a bug in the 32-bit fillSpeculateCell(). The 64-bit
fillSpeculateCell() appears to not have this bug, but I added a similar defense
mechanism anyway just in case, since this is one of those mistakes that keeps
reappearing.

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::silentSavePlanForGPR):

• dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::fillSpeculateCell):

• dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::fillSpeculateCell):

Location:

trunk/Source/JavaScriptCore/dfg

Files:

• DFGSpeculativeJIT.cpp (modified) (1 diff)
• DFGSpeculativeJIT32_64.cpp (modified) (2 diffs)
• DFGSpeculativeJIT64.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -344 +344 @@
         ASSERT(info.gpr() == source);
         if (node->hasConstant()) {
+            DFG_ASSERT(m_jit.graph(), m_currentNode, node->isCellConstant());
             node->asCell(); // To get the assertion.
             fillAction = SetCellConstant;

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp

@@ -868 +868 @@
     VirtualRegister virtualRegister = edge->virtualRegister();
     GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
+    
+    if (edge->hasConstant() && !edge->isCellConstant()) {
+        // Protect the silent spill/fill logic by failing early. If we "speculate" on
+        // the constant then the silent filler may think that we have a cell and a
+        // constant, so it will try to fill this as an cell constant. Bad things will
+        // happen.
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+        return allocate();
+    }
 
     switch (info.registerFormat()) {
@@ -879 +888 @@
             JSValue jsValue = edge->asJSValue();
             GPRReg gpr = allocate();
-            if (jsValue.isCell()) {
-                m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
-                m_jit.move(MacroAssembler::TrustedImmPtr(jsValue.asCell()), gpr);
-                info.fillCell(*m_stream, gpr);
-                return gpr;
-            }
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+            m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
+            m_jit.move(MacroAssembler::TrustedImmPtr(jsValue.asCell()), gpr);
+            info.fillCell(*m_stream, gpr);
             return gpr;
         }

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp

@@ -996 +996 @@
     GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
+    if (edge->hasConstant() && !edge->isCellConstant()) {
+        // Better to fail early on constants.
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+        return allocate();
+    }
+
     switch (info.registerFormat()) {
     case DataFormatNone: {
@@ -1002 +1008 @@
         if (edge->hasConstant()) {
             JSValue jsValue = edge->asJSValue();
-            if (jsValue.isCell()) {
-                m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
-                m_jit.move(MacroAssembler::TrustedImm64(JSValue::encode(jsValue)), gpr);
-                info.fillJSValue(*m_stream, gpr, DataFormatJSCell);
-                return gpr;
-            }
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+            m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
+            m_jit.move(MacroAssembler::TrustedImm64(JSValue::encode(jsValue)), gpr);
+            info.fillJSValue(*m_stream, gpr, DataFormatJSCell);
             return gpr;
         }