Bug 1532376 - Fix places where we don't respect the shouldPretenure flag when creating an object r=jandem

marco-c · marco-c · commit 4ef4f3207a87 · 2019-10-04T02:41:52.000Z
This adds an overload of GetInitialHeap that takes an ObjectGroup* instead of a Class* and also takes into account whether the group's shouldPreTenure flag is set. I moved this to JSObject-inl.h too. I removed the heap parameter in a few places, in particular in NewDenseCopyOnWriteArray which required a bunch of changes elsewhere including the JITs. I left the heap parameter intact for environment objects where we may have reason prefer these objects to be allocated in the tenure heap. It's possible we should just remove all these parameters too and make allocation more uniform. Differential Revision: https://phabricator.services.mozilla.com/D22324 UltraBlame original commit: 1b4fd78107e2bcf7fe0f44038176ca745b07cd88
diff --git a/js/src/builtin/Array.cpp b/js/src/builtin/Array.cpp
@@ -4064,7 +4064,7 @@ static MOZ_ALWAYS_INLINE ArrayObject* NewArray(
   AutoSetNewObjectMetadata metadata(cx);
   RootedArrayObject arr(
       cx, ArrayObject::createArray(
-              cx, allocKind, GetInitialHeap(newKind, &ArrayObject::class_),
+              cx, allocKind, GetInitialHeap(newKind, group),
               shape, group, length, metadata));
   if (!arr) {
     return nullptr;
@@ -4153,7 +4153,7 @@ ArrayObject* js::NewDenseFullyAllocatedArrayWithTemplate(
   RootedObjectGroup group(cx, templateObject->group());
   RootedShape shape(cx, templateObject->as<ArrayObject>().lastProperty());
 
-  gc::InitialHeap heap = GetInitialHeap(GenericObject, &ArrayObject::class_);
+  gc::InitialHeap heap = GetInitialHeap(GenericObject, group);
   Rooted<ArrayObject*> arr(
       cx, ArrayObject::createArray(cx, allocKind, heap, shape, group, length,
                                    metadata));
@@ -4171,10 +4171,11 @@ ArrayObject* js::NewDenseFullyAllocatedArrayWithTemplate(
 }
 
 ArrayObject* js::NewDenseCopyOnWriteArray(JSContext* cx,
-                                          HandleArrayObject templateObject,
-                                          gc::InitialHeap heap) {
+                                          HandleArrayObject templateObject) {
   MOZ_ASSERT(!gc::IsInsideNursery(templateObject));
 
+  gc::InitialHeap heap = GetInitialHeap(GenericObject, templateObject->group());
+
   ArrayObject* arr =
       ArrayObject::createCopyOnWriteArray(cx, heap, templateObject);
   if (!arr) {
diff --git a/js/src/builtin/Array.h b/js/src/builtin/Array.h
@@ -79,8 +79,7 @@ extern ArrayObject* NewDenseFullyAllocatedArrayWithTemplate(
 
 
 extern ArrayObject* NewDenseCopyOnWriteArray(JSContext* cx,
-                                             HandleArrayObject templateObject,
-                                             gc::InitialHeap heap);
+                                             HandleArrayObject templateObject);
 
 extern ArrayObject* NewFullyAllocatedArrayTryUseGroup(
     JSContext* cx, HandleObjectGroup group, size_t length,
diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp
@@ -1553,7 +1553,7 @@ static MOZ_MUST_USE JSObject* ReadableStreamCreateReadResult(
   NativeObject* obj;
   JS_TRY_VAR_OR_RETURN_NULL(
       cx, obj,
-      NativeObject::createWithTemplate(cx, gc::DefaultHeap, templateObject));
+      NativeObject::createWithTemplate(cx, templateObject));
 
   
   obj->setSlot(Realm::IterResultObjectValueSlot, value);
diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp
@@ -2447,10 +2447,9 @@ bool BaselineCompilerCodeGen::emit_JSOP_NEWARRAY_COPYONWRITE() {
 
   prepareVMCall();
 
-  pushArg(Imm32(gc::DefaultHeap));
   pushArg(ImmGCPtr(obj));
 
-  using Fn = ArrayObject* (*)(JSContext*, HandleArrayObject, gc::InitialHeap);
+  using Fn = ArrayObject* (*)(JSContext*, HandleArrayObject);
   if (!callVM<Fn, js::NewDenseCopyOnWriteArray>()) {
     return false;
   }
diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp
@@ -6608,8 +6608,7 @@ void CodeGenerator::visitOutOfLineNewArray(OutOfLineNewArray* ool) {
   masm.jump(ool->rejoin());
 }
 
-typedef ArrayObject* (*NewArrayCopyOnWriteFn)(JSContext*, HandleArrayObject,
-                                              gc::InitialHeap);
+typedef ArrayObject* (*NewArrayCopyOnWriteFn)(JSContext*, HandleArrayObject);
 static const VMFunction NewArrayCopyOnWriteInfo =
     FunctionInfo<NewArrayCopyOnWriteFn>(js::NewDenseCopyOnWriteArray,
                                         "NewDenseCopyOnWriteArray");
@@ -6623,7 +6622,7 @@ void CodeGenerator::visitNewArrayCopyOnWrite(LNewArrayCopyOnWrite* lir) {
   
   OutOfLineCode* ool =
       oolCallVM(NewArrayCopyOnWriteInfo, lir,
-                ArgList(ImmGCPtr(templateObject), Imm32(initialHeap)),
+                ArgList(ImmGCPtr(templateObject)),
                 StoreRegisterTo(objReg));
 
   TemplateObject templateObj(templateObject);
diff --git a/js/src/jit/Recover.cpp b/js/src/jit/Recover.cpp
@@ -1254,12 +1254,10 @@ bool RNewArray::recover(JSContext* cx, SnapshotIterator& iter) const {
 bool MNewArrayCopyOnWrite::writeRecoverData(CompactBufferWriter& writer) const {
   MOZ_ASSERT(canRecoverOnBailout());
   writer.writeUnsigned(uint32_t(RInstruction::Recover_NewArrayCopyOnWrite));
-  writer.writeByte(initialHeap());
   return true;
 }
 
 RNewArrayCopyOnWrite::RNewArrayCopyOnWrite(CompactBufferReader& reader) {
-  initialHeap_ = gc::InitialHeap(reader.readByte());
 }
 
 bool RNewArrayCopyOnWrite::recover(JSContext* cx,
@@ -1269,7 +1267,7 @@ bool RNewArrayCopyOnWrite::recover(JSContext* cx,
   RootedValue result(cx);
 
   ArrayObject* resultObject =
-      NewDenseCopyOnWriteArray(cx, templateObject, initialHeap_);
+      NewDenseCopyOnWriteArray(cx, templateObject);
   if (!resultObject) {
     return false;
   }
diff --git a/js/src/jit/Recover.h b/js/src/jit/Recover.h
@@ -617,9 +617,6 @@ class RNewArray final : public RInstruction {
 };
 
 class RNewArrayCopyOnWrite final : public RInstruction {
- private:
-  gc::InitialHeap initialHeap_;
-
  public:
   RINSTRUCTION_HEADER_NUM_OP_(NewArrayCopyOnWrite, 1)
 
diff --git a/js/src/vm/EnvironmentObject.cpp b/js/src/vm/EnvironmentObject.cpp
@@ -79,9 +79,11 @@ CallObject* CallObject::create(JSContext* cx, HandleShape shape,
   MOZ_ASSERT(CanBeFinalizedInBackground(kind, &CallObject::class_));
   kind = gc::GetBackgroundAllocKind(kind);
 
+  gc::InitialHeap heap = GetInitialHeap(GenericObject, group);
+
   JSObject* obj;
   JS_TRY_VAR_OR_RETURN_NULL(
-      cx, obj, NativeObject::create(cx, kind, gc::DefaultHeap, shape, group));
+      cx, obj, NativeObject::create(cx, kind, heap, shape, group));
 
   return &obj->as<CallObject>();
 }
@@ -108,6 +110,10 @@ CallObject* CallObject::createTemplateObject(JSContext* cx, HandleScript script,
   MOZ_ASSERT(CanBeFinalizedInBackground(kind, &class_));
   kind = gc::GetBackgroundAllocKind(kind);
 
+  if (group->shouldPreTenureDontCheckGeneration()) {
+    heap = gc::TenuredHeap;
+  }
+
   JSObject* obj;
   JS_TRY_VAR_OR_RETURN_NULL(cx, obj,
                             NativeObject::create(cx, kind, heap, shape, group));
@@ -887,6 +893,10 @@ LexicalEnvironmentObject* LexicalEnvironmentObject::createTemplateObject(
     return nullptr;
   }
 
+  if (group->shouldPreTenureDontCheckGeneration()) {
+    heap = gc::TenuredHeap;
+  }
+
   gc::AllocKind allocKind = gc::GetGCObjectKind(shape->numFixedSlots());
   MOZ_ASSERT(
       CanBeFinalizedInBackground(allocKind, &LexicalEnvironmentObject::class_));
diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp
@@ -5288,7 +5288,7 @@ ArrayObject* js::NewArrayCopyOnWriteOperation(JSContext* cx,
     return nullptr;
   }
 
-  return NewDenseCopyOnWriteArray(cx, baseobj, gc::DefaultHeap);
+  return NewDenseCopyOnWriteArray(cx, baseobj);
 }
 
 void js::ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber,
diff --git a/js/src/vm/Iteration.cpp b/js/src/vm/Iteration.cpp
@@ -619,7 +619,7 @@ static PropertyIteratorObject* NewPropertyIteratorObject(JSContext* cx) {
   JS_TRY_VAR_OR_RETURN_NULL(
       cx, obj,
       NativeObject::create(cx, ITERATOR_FINALIZE_KIND,
-                           GetInitialHeap(GenericObject, clasp), shape, group));
+                           GetInitialHeap(GenericObject, group), shape, group));
 
   PropertyIteratorObject* res = &obj->as<PropertyIteratorObject>();
 
@@ -996,7 +996,7 @@ JSObject* js::CreateIterResultObject(JSContext* cx, HandleValue value,
   NativeObject* resultObj;
   JS_TRY_VAR_OR_RETURN_NULL(
       cx, resultObj,
-      NativeObject::createWithTemplate(cx, gc::DefaultHeap, templateObject));
+      NativeObject::createWithTemplate(cx, templateObject));
 
   
   resultObj->setSlot(Realm::IterResultObjectValueSlot, value);
diff --git a/js/src/vm/JSObject-inl.h b/js/src/vm/JSObject-inl.h
@@ -399,6 +399,32 @@ inline bool IsInternalFunctionObject(JSObject& funobj) {
   return fun.isInterpreted() && !fun.environment();
 }
 
+inline gc::InitialHeap GetInitialHeap(NewObjectKind newKind,
+                                      const Class* clasp) {
+  if (newKind == NurseryAllocatedProxy) {
+    MOZ_ASSERT(clasp->isProxy());
+    MOZ_ASSERT(clasp->hasFinalize());
+    MOZ_ASSERT(!CanNurseryAllocateFinalizedClass(clasp));
+    return gc::DefaultHeap;
+  }
+  if (newKind != GenericObject) {
+    return gc::TenuredHeap;
+  }
+  if (clasp->hasFinalize() && !CanNurseryAllocateFinalizedClass(clasp)) {
+    return gc::TenuredHeap;
+  }
+  return gc::DefaultHeap;
+}
+
+inline gc::InitialHeap GetInitialHeap(NewObjectKind newKind,
+                                      ObjectGroup* group) {
+  if (group->shouldPreTenureDontCheckGeneration()) {
+    return gc::TenuredHeap;
+  }
+
+  return GetInitialHeap(newKind, group->clasp());
+}
+
 
 
 
diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp
@@ -792,7 +792,7 @@ static inline JSObject* NewObject(JSContext* cx, HandleObjectGroup group,
     return nullptr;
   }
 
-  gc::InitialHeap heap = GetInitialHeap(newKind, clasp);
+  gc::InitialHeap heap = GetInitialHeap(newKind, group);
 
   JSObject* obj;
   if (clasp->isJSFunction()) {
@@ -975,7 +975,7 @@ JSObject* js::NewObjectWithGroupCommon(JSContext* cx, HandleObjectGroup group,
     NewObjectCache::EntryIndex entry = -1;
     if (cache.lookupGroup(group, allocKind, &entry)) {
       JSObject* obj = cache.newObjectFromHit(
-          cx, entry, GetInitialHeap(newKind, group->clasp()));
+          cx, entry, GetInitialHeap(newKind, group));
       if (obj) {
         return obj;
       }
diff --git a/js/src/vm/JSObject.h b/js/src/vm/JSObject.h
@@ -802,23 +802,6 @@ using ClassInitializerOp = JSObject* (*)(JSContext* cx,
 
 namespace js {
 
-inline gc::InitialHeap GetInitialHeap(NewObjectKind newKind,
-                                      const Class* clasp) {
-  if (newKind == NurseryAllocatedProxy) {
-    MOZ_ASSERT(clasp->isProxy());
-    MOZ_ASSERT(clasp->hasFinalize());
-    MOZ_ASSERT(!CanNurseryAllocateFinalizedClass(clasp));
-    return gc::DefaultHeap;
-  }
-  if (newKind != GenericObject) {
-    return gc::TenuredHeap;
-  }
-  if (clasp->hasFinalize() && !CanNurseryAllocateFinalizedClass(clasp)) {
-    return gc::TenuredHeap;
-  }
-  return gc::DefaultHeap;
-}
-
 bool NewObjectWithTaggedProtoIsCachable(JSContext* cx,
                                         Handle<TaggedProto> proto,
                                         NewObjectKind newKind,
diff --git a/js/src/vm/NativeObject-inl.h b/js/src/vm/NativeObject-inl.h
@@ -522,9 +522,11 @@ inline bool NativeObject::isInWholeCellBuffer() const {
 }
 
  inline JS::Result<NativeObject*, JS::OOM&>
-NativeObject::createWithTemplate(JSContext* cx, js::gc::InitialHeap heap,
-                                 HandleObject templateObject) {
+NativeObject::createWithTemplate(JSContext* cx, HandleObject templateObject) {
   RootedObjectGroup group(cx, templateObject->group());
+
+  gc::InitialHeap heap = GetInitialHeap(GenericObject, group);
+
   RootedShape shape(cx, templateObject->as<NativeObject>().lastProperty());
 
   gc::AllocKind kind = gc::GetGCObjectKind(shape->numFixedSlots());
diff --git a/js/src/vm/NativeObject.h b/js/src/vm/NativeObject.h
@@ -565,7 +565,7 @@ class NativeObject : public ShapedObject {
       js::HandleShape shape, js::HandleObjectGroup group);
 
   static inline JS::Result<NativeObject*, JS::OOM&> createWithTemplate(
-      JSContext* cx, js::gc::InitialHeap heap, HandleObject templateObject);
+      JSContext* cx, HandleObject templateObject);
 
 #ifdef DEBUG
   static void enableShapeConsistencyChecks();
diff --git a/js/src/vm/ObjectGroup-inl.h b/js/src/vm/ObjectGroup-inl.h
@@ -54,8 +54,13 @@ inline bool ObjectGroup::unknownProperties(const AutoSweepObjectGroup& sweep) {
 }
 
 inline bool ObjectGroup::shouldPreTenure(const AutoSweepObjectGroup& sweep) {
-  return hasAnyFlags(sweep, OBJECT_FLAG_PRE_TENURE) &&
-         !unknownProperties(sweep);
+  MOZ_ASSERT(sweep.group() == this);
+  return shouldPreTenureDontCheckGeneration();
+}
+
+inline bool ObjectGroup::shouldPreTenureDontCheckGeneration() {
+  return hasAnyFlagsDontCheckGeneration(OBJECT_FLAG_PRE_TENURE) &&
+         !unknownPropertiesDontCheckGeneration();
 }
 
 inline bool ObjectGroup::canPreTenure(const AutoSweepObjectGroup& sweep) {
diff --git a/js/src/vm/ObjectGroup.h b/js/src/vm/ObjectGroup.h
@@ -404,6 +404,10 @@ class ObjectGroup : public gc::TenuredCell {
   inline bool hasAllFlags(const AutoSweepObjectGroup& sweep,
                           ObjectGroupFlags flags);
 
+  bool hasAnyFlagsDontCheckGeneration(ObjectGroupFlags flags) {
+    MOZ_ASSERT((flags & OBJECT_FLAG_DYNAMIC_MASK) == flags);
+    return !!(this->flagsDontCheckGeneration() & flags);
+  }
   bool hasAllFlagsDontCheckGeneration(ObjectGroupFlags flags) {
     MOZ_ASSERT((flags & OBJECT_FLAG_DYNAMIC_MASK) == flags);
     return (this->flagsDontCheckGeneration() & flags) == flags;
@@ -418,6 +422,7 @@ class ObjectGroup : public gc::TenuredCell {
   }
 
   inline bool shouldPreTenure(const AutoSweepObjectGroup& sweep);
+  inline bool shouldPreTenureDontCheckGeneration();
 
   gc::InitialHeap initialHeap(CompilerConstraintList* constraints);
 
diff --git a/js/src/vm/ProxyObject.cpp b/js/src/vm/ProxyObject.cpp
@@ -184,7 +184,7 @@ void ProxyObject::nuke() {
     realm->newProxyCache.add(group, shape);
   }
 
-  gc::InitialHeap heap = GetInitialHeap(newKind, clasp);
+  gc::InitialHeap heap = GetInitialHeap(newKind, group);
   debugCheckNewObject(group, shape, allocKind, heap);
 
   JSObject* obj = js::Allocate<JSObject>(cx, allocKind,  0,
diff --git a/js/src/vm/UnboxedObject.cpp b/js/src/vm/UnboxedObject.cpp
@@ -875,7 +875,7 @@ UnboxedPlainObject* UnboxedPlainObject::create(JSContext* cx,
     AutoSweepObjectGroup sweep(group);
     allocKind = group->unboxedLayout(sweep).getAllocKind();
   }
-  gc::InitialHeap heap = GetInitialHeap(newKind, &class_);
+  gc::InitialHeap heap = GetInitialHeap(newKind, group);
 
   MOZ_ASSERT(newKind != SingletonObject);
 

Original file line number	Diff line number	Diff line change
`@@ -2447,10 +2447,9 @@ bool BaselineCompilerCodeGen::emit_JSOP_NEWARRAY_COPYONWRITE() {`
`2447`	`2447`
`2448`	`2448`	`prepareVMCall();`
`2449`	`2449`
`2450`		`- pushArg(Imm32(gc::DefaultHeap));`
`2451`	`2450`	`pushArg(ImmGCPtr(obj));`
`2452`	`2451`
`2453`		`- using Fn = ArrayObject* ()(JSContext, HandleArrayObject, gc::InitialHeap);`
	`2452`	`+ using Fn = ArrayObject* ()(JSContext, HandleArrayObject);`
`2454`	`2453`	`if (!callVM<Fn, js::NewDenseCopyOnWriteArray>()) {`
`2455`	`2454`	`return false;`
`2456`	`2455`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1254,12 +1254,10 @@ bool RNewArray::recover(JSContext* cx, SnapshotIterator& iter) const {`
`1254`	`1254`	`bool MNewArrayCopyOnWrite::writeRecoverData(CompactBufferWriter& writer) const {`
`1255`	`1255`	`MOZ_ASSERT(canRecoverOnBailout());`
`1256`	`1256`	`writer.writeUnsigned(uint32_t(RInstruction::Recover_NewArrayCopyOnWrite));`
`1257`		`- writer.writeByte(initialHeap());`
`1258`	`1257`	`return true;`
`1259`	`1258`	`}`
`1260`	`1259`
`1261`	`1260`	`RNewArrayCopyOnWrite::RNewArrayCopyOnWrite(CompactBufferReader& reader) {`
`1262`		`- initialHeap_ = gc::InitialHeap(reader.readByte());`
`1263`	`1261`	`}`
`1264`	`1262`
`1265`	`1263`	`bool RNewArrayCopyOnWrite::recover(JSContext* cx,`
`@@ -1269,7 +1267,7 @@ bool RNewArrayCopyOnWrite::recover(JSContext* cx,`
`1269`	`1267`	`RootedValue result(cx);`
`1270`	`1268`
`1271`	`1269`	`ArrayObject* resultObject =`
`1272`		`- NewDenseCopyOnWriteArray(cx, templateObject, initialHeap_);`
	`1270`	`+ NewDenseCopyOnWriteArray(cx, templateObject);`
`1273`	`1271`	`if (!resultObject) {`
`1274`	`1272`	`return false;`
`1275`	`1273`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5288,7 +5288,7 @@ ArrayObject* js::NewArrayCopyOnWriteOperation(JSContext* cx,`
`5288`	`5288`	`return nullptr;`
`5289`	`5289`	`}`
`5290`	`5290`
`5291`		`- return NewDenseCopyOnWriteArray(cx, baseobj, gc::DefaultHeap);`
	`5291`	`+ return NewDenseCopyOnWriteArray(cx, baseobj);`
`5292`	`5292`	`}`
`5293`	`5293`
`5294`	`5294`	`void js::ReportRuntimeLexicalError(JSContext* cx, unsigned errorNumber,`