From 233233ba3c1adb31eb7b16693dc75752015748b4 Mon Sep 17 00:00:00 2001
From: Jan de Mooij <jdemooij@mozilla.com>
Date: Mon, 10 May 2021 14:54:53 +0000
Subject: [PATCH] Bug 1709537 - Remove property from shape table after (instead
 of before) removing it from the shape list. r=jonco

In canSkipMarkingShapeCache we're now a bit more strict and assert that every shape
in the shape lineage is also found in the ShapeTable. However when removing a dictionary
property we first removed the entry from the ShapeTable and we can then assert as part
of the pre-barrier (verifier) when removing the shape from the shape list.

Differential Revision: https://phabricator.services.mozilla.com/D114581
---
 js/src/jit-test/tests/gc/bug1709537.js |  9 +++++++++
 js/src/vm/Shape.cpp                    | 14 ++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)
 create mode 100644 js/src/jit-test/tests/gc/bug1709537.js

diff --git a/js/src/jit-test/tests/gc/bug1709537.js b/js/src/jit-test/tests/gc/bug1709537.js
new file mode 100644
index 000000000000..77f3fd1d5cd8
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug1709537.js
@@ -0,0 +1,9 @@
+function f() {
+    var obj = [];
+    for (var count = 20000; count > 15900; count--) {
+        obj[count] = 2;
+    }
+    assertEq(Object.getOwnPropertyNames(obj).length, 4101);
+}
+gczeal(4);
+f();
diff --git a/js/src/vm/Shape.cpp b/js/src/vm/Shape.cpp
index 6b6cc3ba6ee7..b1615225c2f7 100644
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -974,6 +974,10 @@ void NativeObject::removeDictionaryPropertyWithoutReshape(ShapeTable* table,
                                                           Shape* shape) {
   // Removes a property from a dictionary object. The caller is responsible for
   // generating a new shape for the object.
+  //
+  // A dictionary-mode object owns mutable, unique shapes on a non-circular
+  // doubly linked list, hashed by lastProperty()->table. So we can edit the
+  // list and table in place.
 
   AutoCheckCannotGC nogc;
   MOZ_ASSERT(inDictionaryMode());
@@ -985,14 +989,10 @@ void NativeObject::removeDictionaryPropertyWithoutReshape(ShapeTable* table,
     freeDictionarySlot(table, shape->slot());
   }
 
-  // A dictionary-mode object owns mutable, unique shapes on a non-circular
-  // doubly linked list, hashed by lastProperty()->table. So we can edit the
-  // list and table in place.
-  table->remove(ptr);
-
-  // Remove shape from its non-circular doubly linked list.
+  // Remove shape from its non-circular doubly linked list and from the table.
   bool removingLastProperty = (shape == lastProperty());
   shape->removeFromDictionary(this);
+  table->remove(ptr);
 
   // If we just removed the object's last property, move its ShapeTable,
   // BaseShape and object flags to the new last property. Information in
@@ -1123,6 +1123,8 @@ bool NativeObject::densifySparseElements(JSContext* cx,
       obj->removeDictionaryPropertyWithoutReshape(table, ptr, shape);
       shape = previous;
     }
+
+    MOZ_ASSERT(obj->lastProperty()->maybeTable(nogc) == table);
   }
 
   // Generate a new shape for the object, infallibly.