Use the correct group for JIT constraints.

This fixes a rare crash/CTD in JS.
This adds information about the constraints to a new RAII class so we
can finish all constraints at the end.
Based on changes in BZ 1568397

4 files changed, 164 insertions, 18 deletions

diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp
index 5fc624fb1d..ace6cd81e4 100644
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -4005,7 +4005,7 @@ jit::ConvertLinearInequality(TempAllocator& alloc, MBasicBlock* block, const Lin
 }
 
 static bool
-AnalyzePoppedThis(JSContext* cx, ObjectGroup* group,
+AnalyzePoppedThis(JSContext* cx, DPAConstraintInfo& constraintInfo, ObjectGroup* group,
                   MDefinition* thisValue, MInstruction* ins, bool definitelyExecuted,
                   HandlePlainObject baseobj,
                   Vector<TypeNewScript::Initializer>* initializerList,
@@ -4046,7 +4046,12 @@ AnalyzePoppedThis(JSContext* cx, ObjectGroup* group,
             return true;
 
         RootedId id(cx, NameToId(setprop->name()));
-        if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, group, id)) {
+        bool added = false;
+        if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, constraintInfo,
+                                                           group, id, &added)) {
+            return false;
+        }
+        if (!added) {
             // The prototype chain already contains a getter/setter for this
             // property, or type information is too imprecise.
             return true;
@@ -4106,7 +4111,12 @@ AnalyzePoppedThis(JSContext* cx, ObjectGroup* group,
         if (!baseobj->lookup(cx, id) && !accessedProperties->append(get->name()))
             return false;
 
-        if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, group, id)) {
+        bool added = false;
+        if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, constraintInfo,
+                                                           group, id, &added)) {
+            return false;
+        }
+        if (!added) {
             // The |this| value can escape if any property reads it does go
             // through a getter.
             return true;
@@ -4132,8 +4142,11 @@ CmpInstructions(const void* a, const void* b)
 }
 
 bool
-jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun,
-                                        ObjectGroup* group, HandlePlainObject baseobj,
+jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx,
+                                        DPAConstraintInfo& constraintInfo,
+                                        HandleFunction fun,
+                                        ObjectGroup* group,
+                                        HandlePlainObject baseobj,
                                         Vector<TypeNewScript::Initializer>* initializerList)
 {
     MOZ_ASSERT(cx->zone()->types.activeAnalysis);
@@ -4293,7 +4306,7 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun,
 
         bool handled = false;
         size_t slotSpan = baseobj->slotSpan();
-        if (!AnalyzePoppedThis(cx, group, thisValue, ins, definitelyExecuted,
+        if (!AnalyzePoppedThis(cx, constraintInfo, group, thisValue, ins, definitelyExecuted,
                                baseobj, initializerList, &accessedProperties, &handled))
         {
             return false;
@@ -4312,7 +4325,6 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun,
         // contingent on the correct frames being inlined. Add constraints to
         // invalidate the definite properties if additional functions could be
         // called at the inline frame sites.
-        Vector<MBasicBlock*> exitBlocks(cx);
         for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++) {
             // Inlining decisions made after the last new property was added to
             // the object don't need to be frozen.
@@ -4320,9 +4332,11 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun,
                 break;
             if (MResumePoint* rp = block->callerResumePoint()) {
                 if (block->numPredecessors() == 1 && block->getPredecessor(0) == rp->block()) {
-                    JSScript* script = rp->block()->info().script();
-                    if (!AddClearDefiniteFunctionUsesInScript(cx, group, script, block->info().script()))
+                    JSScript* caller = rp->block()->info().script();
+                    JSScript* callee = block->info().script();
+                    if (!constraintInfo.addInliningConstraint(caller, callee)) {
                         return false;
+                    }
                 }
             }
         }
diff --git a/js/src/jit/IonAnalysis.h b/js/src/jit/IonAnalysis.h
index efd31415b4..49bc0b5919 100644
--- a/js/src/jit/IonAnalysis.h
+++ b/js/src/jit/IonAnalysis.h
@@ -196,8 +196,11 @@ MCompare*
 ConvertLinearInequality(TempAllocator& alloc, MBasicBlock* block, const LinearSum& sum);
 
 MOZ_MUST_USE bool
-AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun,
-                                   ObjectGroup* group, HandlePlainObject baseobj,
+AnalyzeNewScriptDefiniteProperties(JSContext* cx,
+                                   DPAConstraintInfo& constraintInfo,
+                                   HandleFunction fun,
+                                   ObjectGroup* group,
+                                   HandlePlainObject baseobj,
                                    Vector<TypeNewScript::Initializer>* initializerList);
 
 MOZ_MUST_USE bool
diff --git a/js/src/vm/TypeInference.cpp b/js/src/vm/TypeInference.cpp
index 7c2c0194e6..88327b47e2 100644
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -3084,29 +3084,39 @@ class TypeConstraintClearDefiniteGetterSetter : public TypeConstraint
 };
 
 bool
-js::AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, ObjectGroup* group, HandleId id)
+js::AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx,
+                                                  DPAConstraintInfo& constraintInfo,
+                                                  ObjectGroup* group,
+                                                  HandleId id,
+                                                  bool* added)
 {
     /*
      * Ensure that if the properties named here could have a getter, setter or
      * a permanent property in any transitive prototype, the definite
      * properties get cleared from the group.
      */
+     
+    *added = false;
+    
     RootedObject proto(cx, group->proto().toObjectOrNull());
     while (proto) {
         ObjectGroup* protoGroup = JSObject::getGroup(cx, proto);
         if (!protoGroup) {
-            cx->recoverFromOutOfMemory();
             return false;
         }
         if (protoGroup->unknownProperties())
-            return false;
+            return true;
         HeapTypeSet* protoTypes = protoGroup->getProperty(cx, proto, id);
-        if (!protoTypes || protoTypes->nonDataProperty() || protoTypes->nonWritableProperty())
+        if (!protoTypes)
             return false;
-        if (!protoTypes->addConstraint(cx, cx->typeLifoAlloc().new_<TypeConstraintClearDefiniteGetterSetter>(group)))
+        if (protoTypes->nonDataProperty() || protoTypes->nonWritableProperty())
+            return true;
+        if (!constraintInfo.addProtoConstraint(proto, id))
             return false;
         proto = proto->staticPrototype();
     }
+    
+    *added = true;
     return true;
 }
 
@@ -3612,6 +3622,43 @@ struct DestroyTypeNewScript
 
 } // namespace
 
+bool DPAConstraintInfo::finishConstraints(JSContext* cx, ObjectGroup* group) {

+  for (const ProtoConstraint& constraint : protoConstraints_) {

+    ObjectGroup* protoGroup = constraint.proto->group();

+

+    // Note: we rely on the group's type information being unchanged since

+    // AddClearDefiniteGetterSetterForPrototypeChain.

+

+    bool unknownProperties = protoGroup->unknownProperties();

+    MOZ_RELEASE_ASSERT(!unknownProperties);

+

+    HeapTypeSet* protoTypes =

+        protoGroup->getProperty(cx, constraint.proto, constraint.id);

+    MOZ_RELEASE_ASSERT(protoTypes);

+

+    MOZ_ASSERT(!protoTypes->nonDataProperty());

+    MOZ_ASSERT(!protoTypes->nonWritableProperty());

+

+    if (!protoTypes->addConstraint(

+            cx,

+            cx->typeLifoAlloc().new_<TypeConstraintClearDefiniteGetterSetter>(

+                group))) {

+      ReportOutOfMemory(cx);

+      return false;

+    }

+  }

+

+  for (const InliningConstraint& constraint : inliningConstraints_) {

+    if (!AddClearDefiniteFunctionUsesInScript(cx, group, constraint.caller,

+                                              constraint.callee)) {

+      ReportOutOfMemory(cx);

+      return false;

+    }

+  }

+

+  return true;

+}

+
 bool
 TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, bool force)
 {
@@ -3715,10 +3762,17 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate,
         return false;
 
     Vector<Initializer> initializerVector(cx);
+    
+    DPAConstraintInfo constraintInfo(cx);
 
     RootedPlainObject templateRoot(cx, templateObject());
     RootedFunction fun(cx, function());
-    if (!jit::AnalyzeNewScriptDefiniteProperties(cx, fun, group, templateRoot, &initializerVector))
+    if (!jit::AnalyzeNewScriptDefiniteProperties(cx,
+                                                 constraintInfo,
+                                                 fun,
+                                                 group,
+                                                 templateRoot,
+                                                 &initializerVector))
         return false;
 
     if (!group->newScript())
@@ -3772,6 +3826,14 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate,
         // The definite properties analysis found exactly the properties that
         // are held in common by the preliminary objects. No further analysis
         // is needed.
+

+        if (!constraintInfo.finishConstraints(cx, group)) {

+            return false;

+        }

+        if (!group->newScript()) {

+            return true;

+        }

+
         group->addDefiniteProperties(cx, templateObject()->lastProperty());
 
         destroyNewScript.group = nullptr;
@@ -3792,6 +3854,16 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate,
                                                                   initialFlags);
     if (!initialGroup)
         return false;
+

+    // Add the constraints. Use the initialGroup as group referenced by the

+    // constraints because that's the group that will have the TypeNewScript

+    // associated with it. See the detachNewScript and setNewScript calls below.

+    if (!constraintInfo.finishConstraints(cx, initialGroup)) {

+        return false;

+    }

+    if (!group->newScript()) {

+        return true;

+    }

 
     initialGroup->addDefiniteProperties(cx, templateObject()->lastProperty());
     group->addDefiniteProperties(cx, prefixShape);
diff --git a/js/src/vm/TypeInference.h b/js/src/vm/TypeInference.h
index 94ce7e8719..fd021fc96c 100644
--- a/js/src/vm/TypeInference.h
+++ b/js/src/vm/TypeInference.h
@@ -789,8 +789,65 @@ class TemporaryTypeSet : public TypeSet
                                  TypedArraySharedness* sharedness);
 };
 
+// Stack class to record information about constraints that need to be added
+// after finishing the Definite Properties Analysis. When the analysis succeeds,
+// the |finishConstraints| method must be called to add the constraints to the
+// TypeSets.
+//
+// There are two constraint types managed here:
+//
+//   1. Proto constraints for HeapTypeSets, to guard against things like getters
+//      and setters on the proto chain.
+//
+//   2. Inlining constraints for StackTypeSets, to invalidate when additional
+//      functions could be called at call sites where we inlined a function.
+//
+// This class uses bare GC-thing pointers because GC is suppressed when the
+// analysis runs.
+class MOZ_RAII DPAConstraintInfo {
+  struct ProtoConstraint {
+    JSObject* proto;
+    jsid id;
+    ProtoConstraint(JSObject* proto, jsid id) : proto(proto), id(id) {}
+  };
+  struct InliningConstraint {
+    JSScript* caller;
+    JSScript* callee;
+    InliningConstraint(JSScript* caller, JSScript* callee)
+        : caller(caller), callee(callee) {}
+  };
+
+  JS::AutoCheckCannotGC nogc_;
+  Vector<ProtoConstraint, 8> protoConstraints_;
+  Vector<InliningConstraint, 4> inliningConstraints_;
+
+public:
+  explicit DPAConstraintInfo(JSContext* cx)
+      : nogc_(cx)
+      , protoConstraints_(cx)
+      , inliningConstraints_(cx)
+  {
+  }
+
+  DPAConstraintInfo(const DPAConstraintInfo&) = delete;
+  void operator=(const DPAConstraintInfo&) = delete;
+
+  MOZ_MUST_USE bool addProtoConstraint(JSObject* proto, jsid id) {
+    return protoConstraints_.emplaceBack(proto, id);
+  }
+  MOZ_MUST_USE bool addInliningConstraint(JSScript* caller, JSScript* callee) {
+    return inliningConstraints_.emplaceBack(caller, callee);
+  }
+
+  MOZ_MUST_USE bool finishConstraints(JSContext* cx, ObjectGroup* group);
+};
+
 bool
-AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, ObjectGroup* group, HandleId id);
+AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx,
+                                              DPAConstraintInfo& constraintInfo,
+                                              ObjectGroup* group,
+                                              HandleId id,
+                                              bool* added);
 
 bool
 AddClearDefiniteFunctionUsesInScript(JSContext* cx, ObjectGroup* group,