Bug 1616516 - Always use non-null clasp argument for ObjectGroup::defaultNewGroup and table lookup. r=iain
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 20 Feb 2020 15:26:54 +0000
changeset 514957 acc0c8327c266ac0b0c61eecdba6d625957ecf81
parent 514956 4dfe33cbf167780e0778cdf90aa3bab82fc9a7a2
child 514958 3e45abc48904a1c98faecd1c36ffd7de6669c794
child 515082 793e3b87db82871f182046db439391d86d47cb67
push id107919
push userjdemooij@mozilla.com
push dateFri, 21 Feb 2020 06:44:29 +0000
treeherderautoland@acc0c8327c26 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersiain
bugs1616516
milestone75.0a1
first release with
nightly linux32
acc0c8327c26 / 75.0a1 / 20200221095110 / files
nightly linux64
acc0c8327c26 / 75.0a1 / 20200221095110 / files
nightly mac
acc0c8327c26 / 75.0a1 / 20200221095110 / files
nightly win32
acc0c8327c26 / 75.0a1 / 20200221095110 / files
nightly win64
acc0c8327c26 / 75.0a1 / 20200221095110 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1616516 - Always use non-null clasp argument for ObjectGroup::defaultNewGroup and table lookup. r=iain The nullptr class mechanism was added for unboxed objects: because a group with a PlainObject class could be mutated into a group with an UnboxedPlainObject class, if we did a lookup with the PlainObject class we would no longer find the group in the table. With unboxed objects gone, a group's class can no longer change so we can (again) always use a non-nullptr class for the table lookup. Differential Revision: https://phabricator.services.mozilla.com/D63446
js/src/jit/CacheIR.cpp
js/src/vm/JSObject.cpp
js/src/vm/ObjectGroup.cpp
js/src/vm/TypeInference.cpp
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -5129,18 +5129,18 @@ bool CallIRGenerator::getTemplateObjectF
     trackAttached(IRGenerator::NotAttached);
     *skipAttach = true;
     return true;
   }
 
   if (protov.isObject()) {
     AutoRealm ar(cx_, calleeFunc);
     TaggedProto proto(&protov.toObject());
-    ObjectGroup* group =
-        ObjectGroup::defaultNewGroup(cx_, nullptr, proto, newTarget);
+    ObjectGroup* group = ObjectGroup::defaultNewGroup(cx_, &PlainObject::class_,
+                                                      proto, newTarget);
     if (!group) {
       return false;
     }
 
     AutoSweepObjectGroup sweep(group);
     if (group->newScript(sweep) && !group->newScript(sweep)->analyzed()) {
       // Function newScript has not been analyzed
       trackAttached(IRGenerator::NotAttached);
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -1116,34 +1116,34 @@ JSObject* js::CreateThisForFunctionWithP
   mozilla::Maybe<AutoRealm> ar;
   if (cx->realm() != callee->realm()) {
     MOZ_ASSERT(cx->compartment() == callee->compartment());
     ar.emplace(cx, callee);
   }
 
   if (proto) {
     RootedObjectGroup group(
-        cx, ObjectGroup::defaultNewGroup(cx, nullptr, TaggedProto(proto),
-                                         newTarget));
+        cx, ObjectGroup::defaultNewGroup(cx, &PlainObject::class_,
+                                         TaggedProto(proto), newTarget));
     if (!group) {
       return nullptr;
     }
 
     {
       AutoSweepObjectGroup sweep(group);
       if (group->newScript(sweep) && !group->newScript(sweep)->analyzed()) {
         bool regenerate;
         if (!group->newScript(sweep)->maybeAnalyze(cx, group, &regenerate)) {
           return nullptr;
         }
         if (regenerate) {
           // The script was analyzed successfully and may have changed
           // the new type table, so refetch the group.
-          group = ObjectGroup::defaultNewGroup(cx, nullptr, TaggedProto(proto),
-                                               newTarget);
+          group = ObjectGroup::defaultNewGroup(cx, &PlainObject::class_,
+                                               TaggedProto(proto), newTarget);
           AutoSweepObjectGroup sweepNewGroup(group);
           MOZ_ASSERT(group && group->newScript(sweepNewGroup));
         }
       }
     }
 
     res = CreateThisForFunctionWithGroup(cx, group, newKind);
   } else {
--- a/js/src/vm/ObjectGroup.cpp
+++ b/js/src/vm/ObjectGroup.cpp
@@ -391,26 +391,27 @@ struct ObjectGroupRealm::NewEntry {
 
   struct Lookup {
     const JSClass* clasp;
     TaggedProto proto;
     JSObject* associated;
 
     Lookup(const JSClass* clasp, TaggedProto proto, JSObject* associated)
         : clasp(clasp), proto(proto), associated(associated) {
-      MOZ_ASSERT((associated && associated->is<JSFunction>()) == !clasp);
+      MOZ_ASSERT(clasp);
+      MOZ_ASSERT_IF(associated && associated->is<JSFunction>(),
+                    clasp == &PlainObject::class_);
     }
 
     explicit Lookup(const NewEntry& entry)
         : clasp(entry.group.unbarrieredGet()->clasp()),
           proto(entry.group.unbarrieredGet()->proto()),
           associated(entry.associated) {
-      if (associated && associated->is<JSFunction>()) {
-        clasp = nullptr;
-      }
+      MOZ_ASSERT_IF(associated && associated->is<JSFunction>(),
+                    clasp == &PlainObject::class_);
     }
   };
 
   bool needsSweep() {
     return IsAboutToBeFinalized(&group) ||
            (associated && IsAboutToBeFinalizedUnbarriered(&associated));
   }
 
@@ -439,17 +440,17 @@ struct MovableCellHasher<ObjectGroupReal
     HashNumber hash = MovableCellHasher<TaggedProto>::hash(lookup.proto);
     hash = mozilla::AddToHash(
         hash, MovableCellHasher<JSObject*>::hash(lookup.associated));
     return mozilla::AddToHash(hash, mozilla::HashGeneric(lookup.clasp));
   }
 
   static inline bool match(const ObjectGroupRealm::NewEntry& key,
                            const Lookup& lookup) {
-    if (lookup.clasp && key.group.unbarrieredGet()->clasp() != lookup.clasp) {
+    if (key.group.unbarrieredGet()->clasp() != lookup.clasp) {
       return false;
     }
 
     TaggedProto proto = key.group.unbarrieredGet()->proto();
     if (!MovableCellHasher<TaggedProto>::match(proto, lookup.proto)) {
       return false;
     }
 
@@ -476,68 +477,58 @@ class ObjectGroupRealm::NewTable
 
 /* static*/ ObjectGroupRealm& ObjectGroupRealm::getForNewObject(JSContext* cx) {
   return cx->realm()->objectGroups_;
 }
 
 MOZ_ALWAYS_INLINE ObjectGroup* ObjectGroupRealm::DefaultNewGroupCache::lookup(
     const JSClass* clasp, TaggedProto proto, JSObject* associated) {
   if (group_ && associated_ == associated && group_->proto() == proto &&
-      (!clasp || group_->clasp() == clasp)) {
+      group_->clasp() == clasp) {
     return group_;
   }
   return nullptr;
 }
 
 /* static */
 ObjectGroup* ObjectGroup::defaultNewGroup(JSContext* cx, const JSClass* clasp,
                                           TaggedProto proto,
                                           JSObject* associated) {
+  MOZ_ASSERT(clasp);
   MOZ_ASSERT_IF(associated, proto.isObject());
   MOZ_ASSERT_IF(proto.isObject(),
                 cx->isInsideCurrentCompartment(proto.toObject()));
 
-  // A null lookup clasp is used for 'new' groups with an associated
-  // function. The group starts out as a plain object but might mutate into an
-  // unboxed plain object.
-  MOZ_ASSERT_IF(!clasp, !!associated);
-
   if (associated && !associated->is<TypeDescr>() && !IsTypeInferenceEnabled()) {
-    clasp = &PlainObject::class_;
     associated = nullptr;
   }
 
   if (associated) {
-    MOZ_ASSERT_IF(!associated->is<TypeDescr>(), !clasp);
     if (associated->is<JSFunction>()) {
       // Canonicalize new functions to use the original one associated with its
       // script.
       associated = associated->as<JSFunction>().maybeCanonicalFunction();
 
       // If we have previously cleared the 'new' script information for this
       // function, don't try to construct another one. Also, for simplicity,
       // don't bother optimizing cross-realm constructors.
       if (associated && (associated->as<JSFunction>().wasNewScriptCleared() ||
                          associated->as<JSFunction>().realm() != cx->realm())) {
         associated = nullptr;
       }
     } else if (associated->is<TypeDescr>()) {
-      if (!clasp) {
+      if (!IsTypedObjectClass(clasp)) {
         // This can happen when we call Reflect.construct with a TypeDescr as
-        // newTarget argument. We're creating a plain object in this case, so
+        // newTarget argument. We're not creating a TypedObject in this case, so
         // don't set the TypeDescr on the group.
         associated = nullptr;
       }
     } else {
       associated = nullptr;
     }
-
-    if (!associated) {
-      clasp = &PlainObject::class_;
-    }
   }
 
   ObjectGroupRealm& groups = ObjectGroupRealm::getForNewObject(cx);
 
   if (ObjectGroup* group =
           groups.defaultNewGroupCache.lookup(clasp, proto, associated)) {
     return group;
   }
@@ -578,33 +569,31 @@ ObjectGroup* ObjectGroup::defaultNewGrou
       }
     }
   }
 
   ObjectGroupRealm::NewTable::AddPtr p = table->lookupForAdd(
       ObjectGroupRealm::NewEntry::Lookup(clasp, proto, associated));
   if (p) {
     ObjectGroup* group = p->group;
-    MOZ_ASSERT_IF(clasp, group->clasp() == clasp);
-    MOZ_ASSERT_IF(!clasp, group->clasp() == &PlainObject::class_);
+    MOZ_ASSERT(group->clasp() == clasp);
     MOZ_ASSERT(group->proto() == proto);
     groups.defaultNewGroupCache.put(group, associated);
     return group;
   }
 
   ObjectGroupFlags initialFlags = 0;
   if (proto.isDynamic() ||
       (proto.isObject() && proto.toObject()->isNewGroupUnknown())) {
     initialFlags = OBJECT_FLAG_DYNAMIC_MASK;
   }
 
   Rooted<TaggedProto> protoRoot(cx, proto);
-  ObjectGroup* group = ObjectGroupRealm::makeGroup(
-      cx, cx->realm(), clasp ? clasp : &PlainObject::class_, protoRoot,
-      initialFlags);
+  ObjectGroup* group = ObjectGroupRealm::makeGroup(cx, cx->realm(), clasp,
+                                                   protoRoot, initialFlags);
   if (!group) {
     return nullptr;
   }
 
   if (!table->add(p, ObjectGroupRealm::NewEntry(group, associated))) {
     ReportOutOfMemory(cx);
     return nullptr;
   }
--- a/js/src/vm/TypeInference.cpp
+++ b/js/src/vm/TypeInference.cpp
@@ -3054,29 +3054,31 @@ void ObjectGroup::detachNewScript(bool i
   // produced by calling 'new' on the associated function anymore.
   // The TypeNewScript is not actually destroyed.
   AutoSweepObjectGroup sweep(this);
   TypeNewScript* newScript = this->newScript(sweep);
   MOZ_ASSERT(newScript);
 
   if (newScript->analyzed()) {
     ObjectGroupRealm& objectGroups = ObjectGroupRealm::get(this);
+    const JSClass* clasp = this->clasp();
+    MOZ_ASSERT(clasp == &PlainObject::class_);
     TaggedProto proto = this->proto();
     if (proto.isObject() && IsForwarded(proto.toObject())) {
       proto = TaggedProto(Forwarded(proto.toObject()));
     }
     JSObject* associated = MaybeForwarded(newScript->function());
     if (replacement) {
       AutoSweepObjectGroup sweepReplacement(replacement);
       MOZ_ASSERT(replacement->newScript(sweepReplacement)->function() ==
                  newScript->function());
-      objectGroups.replaceDefaultNewGroup(nullptr, proto, associated,
+      objectGroups.replaceDefaultNewGroup(clasp, proto, associated,
                                           replacement);
     } else {
-      objectGroups.removeDefaultNewGroup(nullptr, proto, associated);
+      objectGroups.removeDefaultNewGroup(clasp, proto, associated);
     }
   } else {
     MOZ_ASSERT(!replacement);
   }
 
   setAddendum(Addendum_None, nullptr, isSweeping);
 }
 
@@ -4026,17 +4028,17 @@ bool TypeNewScript::maybeAnalyze(JSConte
   if (!group->newScript(sweep)) {
     return true;
   }
 
   initialGroup->addDefiniteProperties(cx, templateObject()->lastProperty());
   group->addDefiniteProperties(cx, prefixShape);
 
   ObjectGroupRealm& realm = ObjectGroupRealm::get(group);
-  realm.replaceDefaultNewGroup(nullptr, group->proto(), function(),
+  realm.replaceDefaultNewGroup(group->clasp(), group->proto(), function(),
                                initialGroup);
 
   templateObject()->setGroup(initialGroup);
 
   // Transfer this TypeNewScript from the fully initialized group to the
   // partially initialized group.
   group->detachNewScript();
   initialGroup->setNewScript(this);