Bug 1877406 - Set native object shape to a safe value if allocating d…

…ynamic slots fails r=jandem

The assertion is failing beacuse the number of slots allocated (zero) doesn't
match the number we expect given the object's shape. Since allocation is
failing and the object will be unreachable, the patch sets the shape to that of
a plain object with zero slots. This should be a safe default.

We have to also ensure that this shape always exists so we can allocate it when
the global is initialized.

This means we can also remove the workaround fix for bug 1828396.

The patch also fixes DumpHeap so it doesn't try and flcose a null file handle,
which crashes on Windows.

Differential Revision: https://phabricator.services.mozilla.com/D200675

js/src/builtin/TestingFunctions.cpp

@@ -4406,7 +4406,7 @@ static bool DumpHeap(JSContext* cx, unsigned argc, Value* vp) {
 
   FILE* dumpFile = stdout;
   auto closeFile = mozilla::MakeScopeExit([&dumpFile] {
-    if (dumpFile != stdout) {
+    if (dumpFile && dumpFile != stdout) {
       fclose(dumpFile);
     }
   });

js/src/gc/GC.cpp

@@ -2345,25 +2345,6 @@ bool CompartmentCheckTracer::edgeIsInCrossCompartmentMap(JS::GCCellPtr dst) {
          InCrossCompartmentMap(runtime(), static_cast<JSObject*>(src), dst);
 }
 
-static bool IsPartiallyInitializedObject(Cell* cell) {
-  if (!cell->is<JSObject>()) {
-    return false;
-  }
-
-  JSObject* obj = cell->as<JSObject>();
-  if (!obj->is<NativeObject>()) {
-    return false;
-  }
-
-  NativeObject* nobj = &obj->as<NativeObject>();
-
-  // Check for failed allocation of dynamic slots in
-  // NativeObject::allocateInitialSlots.
-  size_t nDynamicSlots = NativeObject::calculateDynamicSlots(
-      nobj->numFixedSlots(), nobj->slotSpan(), nobj->getClass());
-  return nDynamicSlots != 0 && !nobj->hasDynamicSlots();
-}
-
 void GCRuntime::checkForCompartmentMismatches() {
   JSContext* cx = rt->mainContextFromOwnThread();
   if (cx->disableStrictProxyCheckingCount) {
@@ -2377,12 +2358,6 @@ void GCRuntime::checkForCompartmentMismatches() {
     for (auto thingKind : AllAllocKinds()) {
       for (auto i = zone->cellIterUnsafe<TenuredCell>(thingKind, empty);
            !i.done(); i.next()) {
-        // We may encounter partially initialized objects. These are unreachable
-        // and it's safe to ignore them.
-        if (IsPartiallyInitializedObject(i.getCell())) {
-          continue;
-        }
-
         trc.src = i.getCell();
         trc.srcKind = MapAllocToTraceKind(thingKind);
         trc.compartment = MapGCThingTyped(

js/src/jit-test/tests/gc/bug-1877406.js

@@ -0,0 +1,7 @@
+// |jit-test| skip-if: !('oomTest' in this); --fuzzing-safe
+
+oomTest(Debugger);
+oomTest(Debugger);
+async function* f() {}
+f().return();
+dumpHeap(f);

js/src/vm/GlobalObject.cpp

@@ -666,6 +666,13 @@ GlobalObject* GlobalObject::new_(JSContext* cx, const JSClass* clasp,
       return nullptr;
     }
 
+    // Create a shape for plain objects with zero slots. This is required to be
+    // present in case allocating dynamic slots for objects fails, so we can
+    // leave a valid object in the heap.
+    if (!createPlainObjectShapeWithDefaultProto(cx, gc::AllocKind::OBJECT0)) {
+      return nullptr;
+    }
+
     realm->clearInitializingGlobal();
     if (hookOption == JS::FireOnNewGlobalHook) {
       JS_FireOnNewGlobalObject(cx, global);

js/src/vm/GlobalObject.h

@@ -1086,6 +1086,14 @@ class GlobalObject : public NativeObject {
   static SharedShape* createPlainObjectShapeWithDefaultProto(
       JSContext* cx, gc::AllocKind kind);
 
+  static SharedShape* getEmptyPlainObjectShape(JSContext* cx) {
+    const PlainObjectSlotsKind kind = PlainObjectSlotsKind::Slots0;
+    SharedShape* shape =
+        cx->global()->data().plainObjectShapesWithDefaultProto[kind];
+    MOZ_ASSERT(shape);  // This is created on initialization.
+    return shape;
+  }
+
   static SharedShape* getFunctionShapeWithDefaultProto(JSContext* cx,
                                                        bool extended) {
     GlobalObjectData& data = cx->global()->data();

js/src/vm/NativeObject.cpp

@@ -391,10 +391,11 @@ bool NativeObject::growSlotsForNewSlot(JSContext* cx, uint32_t numFixed,
 bool NativeObject::allocateInitialSlots(JSContext* cx, uint32_t capacity) {
   uint32_t count = ObjectSlots::allocCount(capacity);
   HeapSlot* allocation = AllocateCellBuffer<HeapSlot>(cx, this, count);
-  if (!allocation) {
-    // The new object will be unreachable, but we still have to make it safe
-    // for finalization. Also we must check for it during GC compartment
-    // checks (see IsPartiallyInitializedObject).
+  if (MOZ_UNLIKELY(!allocation)) {
+    // The new object will be unreachable, but we have to make it safe for
+    // finalization. It can also be observed with dumpHeap().
+    // Give it a dummy shape that has no dynamic slots.
+    setShape(GlobalObject::getEmptyPlainObjectShape(cx));
     initEmptyDynamicSlots();
     return false;
   }