Title: [188067] trunk/Source/_javascript_Core
Revision
188067
Author
fpi...@apple.com
Date
2015-08-06 15:36:34 -0700 (Thu, 06 Aug 2015)

Log Message

Structures used for tryGetConstantProperty() should be registered first
https://bugs.webkit.org/show_bug.cgi?id=147750

Reviewed by Saam Barati and Michael Saboff.

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::tryGetConstantProperty): Add an assertion to that effect. This should catch the bug sooner.
* dfg/DFGGraph.h:
(JSC::DFG::Graph::addStructureSet): Register structures when we make a structure set. That ensures that we won't call tryGetConstantProperty() on a structure that hasn't been registered yet.
* dfg/DFGStructureRegistrationPhase.cpp:
(JSC::DFG::StructureRegistrationPhase::run): Don't register structure sets here anymore. Registering them before we get here means there is no chance of the code being DCE'd before the structures get registered. It also enables the tryGetConstantProperty() assertion, since that code runs before StructureRegisterationPhase.
(JSC::DFG::StructureRegistrationPhase::registerStructures):
(JSC::DFG::StructureRegistrationPhase::registerStructure):
(JSC::DFG::StructureRegistrationPhase::assertAreRegistered):
(JSC::DFG::StructureRegistrationPhase::assertIsRegistered):
(JSC::DFG::performStructureRegistration):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (188066 => 188067)


--- trunk/Source/_javascript_Core/ChangeLog	2015-08-06 22:32:06 UTC (rev 188066)
+++ trunk/Source/_javascript_Core/ChangeLog	2015-08-06 22:36:34 UTC (rev 188067)
@@ -1,3 +1,22 @@
+2015-08-06  Filip Pizlo  <fpi...@apple.com>
+
+        Structures used for tryGetConstantProperty() should be registered first
+        https://bugs.webkit.org/show_bug.cgi?id=147750
+
+        Reviewed by Saam Barati and Michael Saboff.
+
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::tryGetConstantProperty): Add an assertion to that effect. This should catch the bug sooner.
+        * dfg/DFGGraph.h:
+        (JSC::DFG::Graph::addStructureSet): Register structures when we make a structure set. That ensures that we won't call tryGetConstantProperty() on a structure that hasn't been registered yet.
+        * dfg/DFGStructureRegistrationPhase.cpp:
+        (JSC::DFG::StructureRegistrationPhase::run): Don't register structure sets here anymore. Registering them before we get here means there is no chance of the code being DCE'd before the structures get registered. It also enables the tryGetConstantProperty() assertion, since that code runs before StructureRegisterationPhase.
+        (JSC::DFG::StructureRegistrationPhase::registerStructures):
+        (JSC::DFG::StructureRegistrationPhase::registerStructure):
+        (JSC::DFG::StructureRegistrationPhase::assertAreRegistered):
+        (JSC::DFG::StructureRegistrationPhase::assertIsRegistered):
+        (JSC::DFG::performStructureRegistration):
+
 2015-08-06  Keith Miller  <keith_mil...@apple.com>
 
         Remove UnspecifiedBoolType from JSC

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (188066 => 188067)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2015-08-06 22:32:06 UTC (rev 188066)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp	2015-08-06 22:36:34 UTC (rev 188067)
@@ -1002,6 +1002,8 @@
     
     for (unsigned i = structureSet.size(); i--;) {
         Structure* structure = structureSet[i];
+        assertIsRegistered(structure);
+        
         WatchpointSet* set = structure->propertyReplacementWatchpointSet(offset);
         if (!set || !set->isStillValid())
             return JSValue();

Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.h (188066 => 188067)


--- trunk/Source/_javascript_Core/dfg/DFGGraph.h	2015-08-06 22:32:06 UTC (rev 188066)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.h	2015-08-06 22:36:34 UTC (rev 188067)
@@ -325,6 +325,8 @@
     
     StructureSet* addStructureSet(const StructureSet& structureSet)
     {
+        for (Structure* structure : structureSet)
+            registerStructure(structure);
         m_structureSet.append(structureSet);
         return &m_structureSet.last();
     }

Modified: trunk/Source/_javascript_Core/dfg/DFGStructureRegistrationPhase.cpp (188066 => 188067)


--- trunk/Source/_javascript_Core/dfg/DFGStructureRegistrationPhase.cpp	2015-08-06 22:32:06 UTC (rev 188066)
+++ trunk/Source/_javascript_Core/dfg/DFGStructureRegistrationPhase.cpp	2015-08-06 22:36:34 UTC (rev 188067)
@@ -57,7 +57,7 @@
         registerStructure(m_graph.m_vm.getterSetterStructure.get());
         
         for (FrozenValue* value : m_graph.m_frozenValues)
-            m_graph.assertIsRegistered(value->structure());
+            assertIsRegistered(value->structure());
         
         for (BlockIndex blockIndex = m_graph.numBlocks(); blockIndex--;) {
             BasicBlock* block = m_graph.block(blockIndex);
@@ -69,7 +69,7 @@
             
                 switch (node->op()) {
                 case CheckStructure:
-                    registerStructures(node->structureSet());
+                    assertAreRegistered(node->structureSet());
                     break;
                 
                 case NewObject:
@@ -145,8 +145,8 @@
 private:
     void registerStructures(const StructureSet& set)
     {
-        for (unsigned i = set.size(); i--;)
-            registerStructure(set[i]);
+        for (Structure* structure : set)
+            registerStructure(structure);
     }
     
     void registerStructure(Structure* structure)
@@ -154,6 +154,18 @@
         if (structure)
             m_graph.registerStructure(structure);
     }
+
+    void assertAreRegistered(const StructureSet& set)
+    {
+        for (Structure* structure : set)
+            assertIsRegistered(structure);
+    }
+
+    void assertIsRegistered(Structure* structure)
+    {
+        if (structure)
+            m_graph.assertIsRegistered(structure);
+    }
 };
 
 bool performStructureRegistration(Graph& graph)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to