Bug 1568029 - Make TypeNewScript::rollbackPartiallyInitializedObjects detect when it's finished traversing the new script initializer list correctly r=tcampbell a=RyanVM
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 08 Aug 2019 17:18:10 +0000
changeset 541940 325b59be2036995d06469cda0a87a17f65b3ea0f
parent 541939 7ad4069f1ffcea355944a57153a8f293ba175ef7
child 541941 39cc6e14e6a0ad804a5e62960371e939999435d8
push id11786
push userarchaeopteryx@coole-files.de
push dateThu, 15 Aug 2019 09:17:18 +0000
treeherdermozilla-beta@39cc6e14e6a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell, RyanVM
bugs1568029
milestone69.0
Bug 1568029 - Make TypeNewScript::rollbackPartiallyInitializedObjects detect when it's finished traversing the new script initializer list correctly r=tcampbell a=RyanVM My previous change to the format of the new script initializer list broke this because it relies on finding the sentinel value at the end of the list to know that it didn't break out of the loop early. This means we always call NativeObject::rollbackProperties after this loop when we shouldn't. The symptom of this is that the workspace property of a Blockly.BlockSvg object becomes undefined causing an infinite loop in code that tries to remove it from that workspace. I verified that the patch fixes this problem. Differential Revision: https://phabricator.services.mozilla.com/D41249
js/src/vm/TypeInference.cpp
js/src/vm/TypeInference.h
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -4009,66 +4009,64 @@ bool TypeNewScript::rollbackPartiallyIni
     if (!thisv.isObject() || thisv.toObject().hasLazyGroup() ||
         thisv.toObject().group() != group) {
       continue;
     }
 
     // Found a matching frame.
     RootedPlainObject obj(cx, &thisv.toObject().as<PlainObject>());
 
-    // Whether all identified 'new' properties have been initialized.
-    bool finished = false;
-
     // If not finished, number of properties that have been added.
     uint32_t numProperties = 0;
 
     // Whether the current SETPROP is within an inner frame which has
     // finished entirely.
     bool pastProperty = false;
 
     // Index in pcOffsets of the outermost frame.
     int callDepth = pcOffsets.length() - 1;
 
     // Index in pcOffsets of the frame currently being checked for a SETPROP.
     int setpropDepth = callDepth;
 
-    for (size_t i = 0; i < initializerList->length; i++) {
+    size_t i;
+    for (i = 0; i < initializerList->length; i++) {
       const TypeNewScriptInitializer init = initializerList->entries[i];
       if (init.kind == TypeNewScriptInitializer::SETPROP) {
         if (!pastProperty && pcOffsets[setpropDepth] < init.offset) {
           // Have not yet reached this setprop.
           break;
         }
         // This setprop has executed, reset state for the next one.
         numProperties++;
         pastProperty = false;
         setpropDepth = callDepth;
-      } else if (init.kind == TypeNewScriptInitializer::SETPROP_FRAME) {
+      } else {
+        MOZ_ASSERT(init.kind == TypeNewScriptInitializer::SETPROP_FRAME);
         if (!pastProperty) {
           if (pcOffsets[setpropDepth] < init.offset) {
             // Have not yet reached this inner call.
             break;
           } else if (pcOffsets[setpropDepth] > init.offset) {
             // Have advanced past this inner call.
             pastProperty = true;
           } else if (setpropDepth == 0) {
             // Have reached this call but not yet in it.
             break;
           } else {
             // Somewhere inside this inner call.
             setpropDepth--;
           }
         }
-      } else {
-        MOZ_ASSERT(init.kind == TypeNewScriptInitializer::DONE);
-        finished = true;
-        break;
       }
     }
 
+    // Whether all identified 'new' properties have been initialized.
+    bool finished = i == initializerList->length;
+
     if (!finished) {
       (void)NativeObject::rollbackProperties(cx, obj, numProperties);
       found = true;
     }
   }
 
   return found;
 }
--- a/js/src/vm/TypeInference.h
+++ b/js/src/vm/TypeInference.h
@@ -160,17 +160,17 @@ class PreliminaryObjectArrayWithTemplate
 };
 
 /**
  * A type representing the initializer of a property within a script being
  * 'new'd.
  */
 class TypeNewScriptInitializer {
  public:
-  enum Kind { SETPROP, SETPROP_FRAME, DONE } kind;
+  enum Kind { SETPROP, SETPROP_FRAME } kind;
   uint32_t offset;
 
   TypeNewScriptInitializer(Kind kind, uint32_t offset)
       : kind(kind), offset(offset) {}
 };
 
 /* Is this a reasonable PC to be doing inlining on? */
 inline bool isInlinableCall(jsbytecode* pc);