[webkit-changes] [182827] trunk/Source/JavaScriptCore

Tue, 14 Apr 2015 17:49:26 -0700

Title: [182827] trunk/Source/_javascript_Core
Revision
182827
Author
msab...@apple.com
Date
2015-04-14 17:49:03 -0700 (Tue, 14 Apr 2015)

Log Message

DFG register fillSpeculate*() functions should validate incoming spill format is compatible with requested fill format
https://bugs.webkit.org/show_bug.cgi?id=143727

Reviewed by Geoffrey Garen.

Used the result of AbstractInterpreter<>::filter() to check that the current spill format is compatible
with the requested fill format.  If filter() reports a contradiction, then we force an OSR exit.
Removed individual checks made redundant by the new check.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateInt52):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (182826 => 182827)


--- trunk/Source/_javascript_Core/ChangeLog	2015-04-15 00:47:25 UTC (rev 182826)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-04-15 00:49:03 UTC (rev 182827)
@@ -1,3 +1,24 @@
+2015-04-14  Michael Saboff  <msab...@apple.com>
+
+        DFG register fillSpeculate*() functions should validate incoming spill format is compatible with requested fill format
+        https://bugs.webkit.org/show_bug.cgi?id=143727
+
+        Reviewed by Geoffrey Garen.
+
+        Used the result of AbstractInterpreter<>::filter() to check that the current spill format is compatible
+        with the requested fill format.  If filter() reports a contradiction, then we force an OSR exit.
+        Removed individual checks made redundant by the new check.
+
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
+        (JSC::DFG::SpeculativeJIT::fillSpeculateCell):
+        (JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
+        (JSC::DFG::SpeculativeJIT::fillSpeculateInt52):
+        (JSC::DFG::SpeculativeJIT::fillSpeculateCell):
+        (JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
+
 2015-04-14  Joseph Pecoraro  <pecor...@apple.com>
 
         Replace _javascript_CoreOutputConsoleMessagesToSystemConsole default with an SPI

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (182826 => 182827)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2015-04-15 00:47:25 UTC (rev 182826)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp	2015-04-15 00:49:03 UTC (rev 182827)
@@ -845,16 +845,16 @@
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
     ASSERT(edge.useKind() != KnownInt32Use || !(value.m_type & ~SpecInt32));
-    m_interpreter.filter(value, SpecInt32);
-    VirtualRegister virtualRegister = edge->virtualRegister();
-    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
-    if (edge->hasConstant() && !edge->isInt32Constant()) {
+    if (m_interpreter.filter(value, SpecInt32) == Contradiction) {
         terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
         returnFormat = DataFormatInt32;
         return allocate();
     }
-    
+
+    VirtualRegister virtualRegister = edge->virtualRegister();
+    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
+
     switch (info.registerFormat()) {
     case DataFormatNone: {
         if (edge->hasConstant()) {
@@ -869,12 +869,6 @@
 
         DataFormat spillFormat = info.spillFormat();
 
-        if (spillFormat == DataFormatCell) {
-            terminateSpeculativeExecution(BadType, JSValueRegs(), edge);
-            returnFormat = DataFormatInt32;
-            return allocate();
-        }
-
         ASSERT_UNUSED(spillFormat, (spillFormat & DataFormatJS) || spillFormat == DataFormatInt32);
 
         // If we know this was spilled as an integer we can fill without checking.
@@ -920,10 +914,6 @@
     case DataFormatJSDouble:
     case DataFormatJSCell:
     case DataFormatJSBoolean:
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        returnFormat = DataFormatInt32;
-        return allocate();
-
     case DataFormatDouble:
     case DataFormatStorage:
     default:
@@ -982,26 +972,17 @@
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
     ASSERT((edge.useKind() != KnownCellUse && edge.useKind() != KnownStringUse) || !(value.m_type & ~SpecCell));
-    m_interpreter.filter(value, SpecCell);
-    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.
+
+    if (m_interpreter.filter(value, SpecCell) == Contradiction) {
         terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
         return allocate();
     }
 
+    VirtualRegister virtualRegister = edge->virtualRegister();
+    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
+
     switch (info.registerFormat()) {
     case DataFormatNone: {
-        if (info.spillFormat() == DataFormatInt32) {
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-            return allocate();
-        }
-
         if (edge->hasConstant()) {
             JSValue jsValue = edge->asJSValue();
             GPRReg gpr = allocate();
@@ -1059,9 +1040,6 @@
     case DataFormatJSDouble:
     case DataFormatJSBoolean:
     case DataFormatBoolean:
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        return allocate();
-
     case DataFormatDouble:
     case DataFormatStorage:
         RELEASE_ASSERT_NOT_REACHED();
@@ -1076,27 +1054,23 @@
 {
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
-    m_interpreter.filter(value, SpecBoolean);
+
+    if (m_interpreter.filter(value, SpecBoolean) == Contradiction) {
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+        return allocate();
+    }
+
     VirtualRegister virtualRegister = edge->virtualRegister();
     GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
     switch (info.registerFormat()) {
     case DataFormatNone: {
-        if (info.spillFormat() == DataFormatInt32) {
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-            return allocate();
-        }
-        
         if (edge->hasConstant()) {
             JSValue jsValue = edge->asJSValue();
             GPRReg gpr = allocate();
-            if (jsValue.isBoolean()) {
-                m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
-                m_jit.move(MacroAssembler::TrustedImm32(jsValue.asBoolean()), gpr);
-                info.fillBoolean(*m_stream, gpr);
-                return gpr;
-            }
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+            m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
+            m_jit.move(MacroAssembler::TrustedImm32(jsValue.asBoolean()), gpr);
+            info.fillBoolean(*m_stream, gpr);
             return gpr;
         }
 
@@ -1140,9 +1114,6 @@
     case DataFormatJSDouble:
     case DataFormatJSCell:
     case DataFormatCell:
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        return allocate();
-
     case DataFormatDouble:
     case DataFormatStorage:
         RELEASE_ASSERT_NOT_REACHED();

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (182826 => 182827)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2015-04-15 00:47:25 UTC (rev 182826)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp	2015-04-15 00:49:03 UTC (rev 182827)
@@ -817,20 +817,16 @@
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
     ASSERT(edge.useKind() != KnownInt32Use || !(value.m_type & ~SpecInt32));
-    m_interpreter.filter(value, SpecInt32);
-    VirtualRegister virtualRegister = edge->virtualRegister();
-    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
-    if (edge->hasConstant() && !edge->isInt32Constant()) {
-        // Protect the silent spill/fill logic by failing early. If we "speculate" on
-        // the constant then the silent filler may think that we have an int32 and a
-        // constant, so it will try to fill this as an int32 constant. Bad things will
-        // happen.
+    if (m_interpreter.filter(value, SpecInt32) == Contradiction) {
         terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
         returnFormat = DataFormatInt32;
         return allocate();
     }
-    
+
+    VirtualRegister virtualRegister = edge->virtualRegister();
+    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
+
     switch (info.registerFormat()) {
     case DataFormatNone: {
         GPRReg gpr = allocate();
@@ -929,12 +925,7 @@
     case DataFormatCell:
     case DataFormatBoolean:
     case DataFormatJSCell:
-    case DataFormatJSBoolean: {
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        returnFormat = DataFormatInt32;
-        return allocate();
-    }
-
+    case DataFormatJSBoolean:
     case DataFormatDouble:
     case DataFormatStorage:
     case DataFormatInt52:
@@ -967,17 +958,17 @@
 {
     ASSERT(desiredFormat == DataFormatInt52 || desiredFormat == DataFormatStrictInt52);
     AbstractValue& value = m_state.forNode(edge);
-    m_interpreter.filter(value, SpecMachineInt);
+
+    if (m_interpreter.filter(value, SpecMachineInt) == Contradiction) {
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+        return allocate();
+    }
+
     VirtualRegister virtualRegister = edge->virtualRegister();
     GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
     switch (info.registerFormat()) {
     case DataFormatNone: {
-        if (edge->hasConstant() && !edge->isMachineIntConstant()) {
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-            return allocate();
-        }
-        
         GPRReg gpr = allocate();
 
         if (edge->hasConstant()) {
@@ -1096,16 +1087,15 @@
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
     ASSERT((edge.useKind() != KnownCellUse && edge.useKind() != KnownStringUse) || !(value.m_type & ~SpecCell));
-    m_interpreter.filter(value, SpecCell);
-    VirtualRegister virtualRegister = edge->virtualRegister();
-    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
-    if (edge->hasConstant() && !edge->isCellConstant()) {
-        // Better to fail early on constants.
+    if (m_interpreter.filter(value, SpecCell) == Contradiction) {
         terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
         return allocate();
     }
 
+    VirtualRegister virtualRegister = edge->virtualRegister();
+    GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
+
     switch (info.registerFormat()) {
     case DataFormatNone: {
         GPRReg gpr = allocate();
@@ -1117,12 +1107,7 @@
             info.fillJSValue(*m_stream, gpr, DataFormatJSCell);
             return gpr;
         }
-        
-        if (!(info.spillFormat() & DataFormatJS)) {
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-            return gpr;
-        }
-        
+
         m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
         m_jit.load64(JITCompiler::addressFor(virtualRegister), gpr);
 
@@ -1158,11 +1143,7 @@
     case DataFormatInt32:
     case DataFormatJSDouble:
     case DataFormatJSBoolean:
-    case DataFormatBoolean: {
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        return allocate();
-    }
-
+    case DataFormatBoolean:
     case DataFormatDouble:
     case DataFormatStorage:
     case DataFormatInt52:
@@ -1179,28 +1160,24 @@
 {
     AbstractValue& value = m_state.forNode(edge);
     SpeculatedType type = value.m_type;
-    m_interpreter.filter(value, SpecBoolean);
+
+    if (m_interpreter.filter(value, SpecBoolean) == Contradiction) {
+        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
+        return allocate();
+    }
+
     VirtualRegister virtualRegister = edge->virtualRegister();
     GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);
 
     switch (info.registerFormat()) {
     case DataFormatNone: {
-        if (info.spillFormat() == DataFormatInt32) {
-            terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-            return allocate();
-        }
-        
         GPRReg gpr = allocate();
 
         if (edge->hasConstant()) {
             JSValue jsValue = edge->asJSValue();
-            if (jsValue.isBoolean()) {
-                m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
-                m_jit.move(MacroAssembler::TrustedImm64(JSValue::encode(jsValue)), gpr);
-                info.fillJSValue(*m_stream, gpr, DataFormatJSBoolean);
-                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, DataFormatJSBoolean);
             return gpr;
         }
         DFG_ASSERT(m_jit.graph(), m_currentNode, info.spillFormat() & DataFormatJS);
@@ -1241,9 +1218,6 @@
     case DataFormatJSDouble:
     case DataFormatJSCell:
     case DataFormatCell:
-        terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
-        return allocate();
-        
     case DataFormatDouble:
     case DataFormatStorage:
     case DataFormatInt52:

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to