The compiler should always register a structure when it adds its transitionWatchPointSet.

https://bugs.webkit.org/show_bug.cgi?id=178420
<rdar://problem/34814024>

Reviewed by Saam Barati and Filip Pizlo.

JSTests:

* stress/regress-178420.js: Added.
(new.Array.10000.map):

Source/JavaScriptCore:

Instead of invoking addLazily() to add a structure's transitionWatchpointSet, we
now invoke Graph::registerAndWatchStructureTransition() on the structure.
registerAndWatchStructureTransition() both registers the structure and add its
transitionWatchpointSet to the plan desired watchpoints.

Graph::registerAndWatchStructureTransition() is based on Graph::registerStructure()
except registerAndWatchStructureTransition() adds the structure's
transitionWatchpointSet unconditionally.

* dfg/DFGArgumentsEliminationPhase.cpp:
* dfg/DFGArrayMode.cpp:
(JSC::DFG::ArrayMode::refine const):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleIntrinsicCall):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):

* dfg/DFGGraph.cpp:
(JSC::DFG::Graph::registerAndWatchStructureTransition):
* dfg/DFGGraph.h:

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileGetByValOnString):
- The second set of addLazily()s is redundant.  This set is executed only when
  prototypeChainIsSane is true, and prototypeChainIsSane can only be true if and
  only if we've executed the if statement above it.  That preceding if statement
  already registerAndWatchStructureTransition() the same 2 structures.  Hence,
  this second set can be deleted.

* dfg/DFGWatchpointCollectionPhase.cpp:
(JSC::DFG::WatchpointCollectionPhase::addLazily):
- Deleted an unused function.

* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223614 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
index 96fbb34..7c4c39e 100644 (file)
--- a/JSTests/ChangeLog
+++ b/JSTests/ChangeLog
@@ -1,3 +1,14 @@
+2017-10-18  Mark Lam  <mark.lam@apple.com>
+
+        The compiler should always register a structure when it adds its transitionWatchPointSet.
+        https://bugs.webkit.org/show_bug.cgi?id=178420
+        <rdar://problem/34814024>
+
+        Reviewed by Saam Barati and Filip Pizlo.
+
+        * stress/regress-178420.js: Added.
+        (new.Array.10000.map):
+
 2017-10-18  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] __proto__ getter should be fast

diff --git a/JSTests/stress/regress-178420.js b/JSTests/stress/regress-178420.js
new file mode 100644 (file)
index 0000000..5d58544
--- /dev/null
+++ b/JSTests/stress/regress-178420.js
@@ -0,0 +1,17 @@
+// This test passes if it does not crash.
+
+var arr0 = [42];
+var arr4 = [,,,,,,,,,,,,,,,,,,,,,,,,];
+
+new Array(10000).map((function() {
+    arr4[-35] = 1.1;
+}), this);
+
+arr0[0] = [];
+gc();
+
+Array.prototype.__proto__ = {};
+gc();
+
+for(var i = 0; i < 65536; i++)
+    arr0['a'+i] = 1.1;

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 4f04cbb..4c06b12 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,47 @@
+2017-10-18  Mark Lam  <mark.lam@apple.com>
+
+        The compiler should always register a structure when it adds its transitionWatchPointSet.
+        https://bugs.webkit.org/show_bug.cgi?id=178420
+        <rdar://problem/34814024>
+
+        Reviewed by Saam Barati and Filip Pizlo.
+
+        Instead of invoking addLazily() to add a structure's transitionWatchpointSet, we
+        now invoke Graph::registerAndWatchStructureTransition() on the structure.
+        registerAndWatchStructureTransition() both registers the structure and add its
+        transitionWatchpointSet to the plan desired watchpoints.
+
+        Graph::registerAndWatchStructureTransition() is based on Graph::registerStructure()
+        except registerAndWatchStructureTransition() adds the structure's
+        transitionWatchpointSet unconditionally.
+
+        * dfg/DFGArgumentsEliminationPhase.cpp:
+        * dfg/DFGArrayMode.cpp:
+        (JSC::DFG::ArrayMode::refine const):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+
+        * dfg/DFGGraph.cpp:
+        (JSC::DFG::Graph::registerAndWatchStructureTransition):
+        * dfg/DFGGraph.h:
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnString):
+        - The second set of addLazily()s is redundant.  This set is executed only when
+          prototypeChainIsSane is true, and prototypeChainIsSane can only be true if and
+          only if we've executed the if statement above it.  That preceding if statement
+          already registerAndWatchStructureTransition() the same 2 structures.  Hence,
+          this second set can be deleted.
+
+        * dfg/DFGWatchpointCollectionPhase.cpp:
+        (JSC::DFG::WatchpointCollectionPhase::addLazily):
+        - Deleted an unused function.
+
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringCharAt):
+
 2017-10-18  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         [JSC] Remove unused private name structure

diff --git a/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp b/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
index a457f8f..50aca23 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -248,20 +248,20 @@ private:
                 // If we're out-of-bounds then we proceed only if the prototype chain
                 // for the allocation is sane (i.e. doesn't have indexed properties).
                 JSGlobalObject* globalObject = m_graph.globalObjectFor(edge->origin.semantic);
-                InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
+                Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
                 if (edge->op() == CreateRest) {
-                    InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
-                    if (arrayPrototypeTransition.isStillValid() 
-                        && objectPrototypeTransition.isStillValid() 
+                    Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+                    if (arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                        && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
                         && globalObject->arrayPrototypeChainIsSane()) {
-                        m_graph.watchpoints().addLazily(arrayPrototypeTransition);
-                        m_graph.watchpoints().addLazily(objectPrototypeTransition);
+                        m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+                        m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
                         break;
                     }
                 } else {
-                    if (objectPrototypeTransition.isStillValid() 
+                    if (objectPrototypeStructure->transitionWatchpointSetIsStillValid()
                         && globalObject->objectPrototypeIsSane()) {
-                        m_graph.watchpoints().addLazily(objectPrototypeTransition);
+                        m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
                         break;
                     }
                 }

diff --git a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
index 5908915..319f419 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
+++ b/Source/JavaScriptCore/dfg/DFGArrayMode.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -214,8 +214,8 @@ ArrayMode ArrayMode::refine(
             && arrayClass() == Array::OriginalArray
             && globalObject->arrayPrototypeChainIsSane()
             && !graph.hasExitSite(node->origin.semantic, OutOfBounds)) {
-            graph.watchpoints().addLazily(globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
-            graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+            graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
+            graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
             if (globalObject->arrayPrototypeChainIsSane())
                 return withSpeculation(Array::SaneChain);
         }

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index 7e921ce..9b97bf6 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -2229,21 +2229,21 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
         case Array::Contiguous: {
             JSGlobalObject* globalObject = m_graph.globalObjectFor(currentNodeOrigin().semantic);
 
-            InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
-            InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
+            Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+            Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
 
             // FIXME: We could easily relax the Array/Object.prototype transition as long as we OSR exitted if we saw a hole.
             // https://bugs.webkit.org/show_bug.cgi?id=173171
             if (globalObject->arraySpeciesWatchpoint().state() == IsWatched
                 && globalObject->havingABadTimeWatchpoint()->isStillValid()
-                && arrayPrototypeTransition.isStillValid()
-                && objectPrototypeTransition.isStillValid()
+                && arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
                 && globalObject->arrayPrototypeChainIsSane()) {
 
                 m_graph.watchpoints().addLazily(globalObject->arraySpeciesWatchpoint());
                 m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
-                m_graph.watchpoints().addLazily(arrayPrototypeTransition);
-                m_graph.watchpoints().addLazily(objectPrototypeTransition);
+                m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+                m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
 
                 insertChecks();
 
@@ -2318,19 +2318,19 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, int resultOperand, Intrin
         case Array::Contiguous: {
             JSGlobalObject* globalObject = m_graph.globalObjectFor(currentNodeOrigin().semantic);
 
-            InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
-            InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
+            Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+            Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
 
             // FIXME: We could easily relax the Array/Object.prototype transition as long as we OSR exitted if we saw a hole.
             // https://bugs.webkit.org/show_bug.cgi?id=173171
             if (globalObject->havingABadTimeWatchpoint()->isStillValid()
-                && arrayPrototypeTransition.isStillValid()
-                && objectPrototypeTransition.isStillValid()
+                && arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
                 && globalObject->arrayPrototypeChainIsSane()) {
 
                 m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
-                m_graph.watchpoints().addLazily(arrayPrototypeTransition);
-                m_graph.watchpoints().addLazily(objectPrototypeTransition);
+                m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
+                m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
 
                 insertChecks();
 

diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index 7d152ed..da10cf0 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -759,10 +759,8 @@ private:
                         }
                         
                         if (canDoSaneChain) {
-                            m_graph.watchpoints().addLazily(
-                                globalObject->arrayPrototype()->structure()->transitionWatchpointSet());
-                            m_graph.watchpoints().addLazily(
-                                globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+                            m_graph.registerAndWatchStructureTransition(globalObject->arrayPrototype()->structure());
+                            m_graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
                             if (globalObject->arrayPrototypeChainIsSane())
                                 node->setArrayMode(arrayMode.withSpeculation(Array::SaneChain));
                         }
@@ -1218,16 +1216,16 @@ private:
             // When we go down the fast path, we don't consult the prototype chain, so we must prove
             // that it doesn't contain any indexed properties, and that any holes will result in
             // jsUndefined().
-            InlineWatchpointSet& objectPrototypeTransition = globalObject->objectPrototype()->structure()->transitionWatchpointSet();
-            InlineWatchpointSet& arrayPrototypeTransition = globalObject->arrayPrototype()->structure()->transitionWatchpointSet();
-            if (node->child1()->shouldSpeculateArray() 
-                && arrayPrototypeTransition.isStillValid() 
-                && objectPrototypeTransition.isStillValid() 
+            Structure* arrayPrototypeStructure = globalObject->arrayPrototype()->structure();
+            Structure* objectPrototypeStructure = globalObject->objectPrototype()->structure();
+            if (node->child1()->shouldSpeculateArray()
+                && arrayPrototypeStructure->transitionWatchpointSetIsStillValid()
+                && objectPrototypeStructure->transitionWatchpointSetIsStillValid()
                 && globalObject->arrayPrototypeChainIsSane()
                 && m_graph.isWatchingArrayIteratorProtocolWatchpoint(node->child1().node())
                 && m_graph.isWatchingHavingABadTimeWatchpoint(node->child1().node())) {
-                m_graph.watchpoints().addLazily(objectPrototypeTransition);
-                m_graph.watchpoints().addLazily(arrayPrototypeTransition);
+                m_graph.registerAndWatchStructureTransition(objectPrototypeStructure);
+                m_graph.registerAndWatchStructureTransition(arrayPrototypeStructure);
                 fixEdge<ArrayUse>(node->child1());
             } else
                 fixEdge<CellUse>(node->child1());

diff --git a/Source/JavaScriptCore/dfg/DFGGraph.cpp b/Source/JavaScriptCore/dfg/DFGGraph.cpp
index 173eaed..4a07e65 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGGraph.cpp
+++ b/Source/JavaScriptCore/dfg/DFGGraph.cpp
@@ -1502,6 +1502,12 @@ RegisteredStructure Graph::registerStructure(Structure* structure, StructureRegi
     return RegisteredStructure::createPrivate(structure);
 }
 
+void Graph::registerAndWatchStructureTransition(Structure* structure)
+{
+    m_plan.weakReferences.addLazily(structure);
+    m_plan.watchpoints.addLazily(structure->transitionWatchpointSet());
+}
+
 void Graph::assertIsRegistered(Structure* structure)
 {
     // It's convenient to be able to call this with a maybe-null structure.

diff --git a/Source/JavaScriptCore/dfg/DFGGraph.h b/Source/JavaScriptCore/dfg/DFGGraph.h
index b1be68b..9402cd0 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGGraph.h
+++ b/Source/JavaScriptCore/dfg/DFGGraph.h
@@ -228,6 +228,7 @@ public:
         return registerStructure(structure, ignored);
     }
     RegisteredStructure registerStructure(Structure*, StructureRegistrationResult&);
+    void registerAndWatchStructureTransition(Structure*);
     void assertIsRegistered(Structure* structure);
     
     // CodeBlock is optional, but may allow additional information to be dumped (e.g. Identifier names).

diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
index f46864c..8913aba 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
@@ -2137,14 +2137,11 @@ void SpeculativeJIT::compileGetByValOnString(Node* node)
             // on a stringPrototypeChainIsSane() guaranteeing that the prototypes have no negative
             // indexed properties either.
             // https://bugs.webkit.org/show_bug.cgi?id=144668
-            m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
-            m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
+            m_jit.graph().registerAndWatchStructureTransition(globalObject->stringPrototype()->structure());
+            m_jit.graph().registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
             prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
         }
         if (prototypeChainIsSane) {
-            m_jit.graph().watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
-            m_jit.graph().watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
-            
 #if USE(JSVALUE64)
             addSlowPathGenerator(std::make_unique<SaneStringGetByValSlowPathGenerator>(
                 outOfBounds, this, JSValueRegs(scratchReg), baseReg, propertyReg));

diff --git a/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp b/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
index 39e296a..8bf6511 100644 (file)
--- a/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp
@@ -112,10 +112,6 @@ private:
     {
         m_graph.watchpoints().addLazily(set);
     }
-    void addLazily(InlineWatchpointSet& set)
-    {
-        m_graph.watchpoints().addLazily(set);
-    }
     
     JSGlobalObject* globalObject()
     {

diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
index 1732f88..f67fae1 100644 (file)
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
@@ -5759,9 +5759,9 @@ private:
                 // SaneChainOutOfBounds.
                 // https://bugs.webkit.org/show_bug.cgi?id=144668
                 
-                m_graph.watchpoints().addLazily(globalObject->stringPrototype()->structure()->transitionWatchpointSet());
-                m_graph.watchpoints().addLazily(globalObject->objectPrototype()->structure()->transitionWatchpointSet());
-                
+                m_graph.registerAndWatchStructureTransition(globalObject->stringPrototype()->structure());
+                m_graph.registerAndWatchStructureTransition(globalObject->objectPrototype()->structure());
+
                 prototypeChainIsSane = globalObject->stringPrototypeChainIsSane();
             }
             if (prototypeChainIsSane) {