From a50657a013870707f308647f0d260a436fa785de Mon Sep 17 00:00:00 2001
From: Jon Coppeard <jcoppeard@mozilla.com>
Date: Thu, 10 Sep 2020 09:18:39 +0000
Subject: [PATCH] Bug 1663741 - Fix problems updating slots after swapping
 objects r=jandem

This does two things: avoid triggering an assertion about the number of dynamic slots while the object is in an intermidiate state, and preserve the number of dictionary mode slots when reallocating the dynamic slots.

Differential Revision: https://phabricator.services.mozilla.com/D89630
---
 js/src/jit-test/tests/basic/bug-1663741.js | 29 ++++++++++++++++++++++
 js/src/vm/JSObject.cpp                     | 17 +++++++++----
 2 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 js/src/jit-test/tests/basic/bug-1663741.js

diff --git a/js/src/jit-test/tests/basic/bug-1663741.js b/js/src/jit-test/tests/basic/bug-1663741.js
new file mode 100644
index 000000000000..866bd083a384
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug-1663741.js
@@ -0,0 +1,29 @@
+const thisGlobal = this;
+const otherGlobalSameCompartment = newGlobal({sameCompartmentAs: thisGlobal});
+const otherGlobalNewCompartment = newGlobal({newCompartment: true});
+
+const globals = [thisGlobal, otherGlobalSameCompartment, otherGlobalNewCompartment];
+
+function testProperties(global, count) {
+  let {object: source, transplant} = transplantableObject();
+
+  // Create a bunch properties on |source|, which force allocation of dynamic
+  // slots.
+  for (let i = 0; i < count; i++) {
+    source["foo" + i] = i;
+  }
+
+  // Calling |transplant| transplants the object and then returns undefined.
+  transplant(global);
+
+  // Check the properties were copied over to the swapped object.
+  for (let i = 0; i < count; i++) {
+    assertEq(source["foo" + i], i);
+  }
+}
+
+for (let global of globals) {
+  for (let count of [0, 10, 30]) {
+    testProperties(global, count);
+  }
+}
diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp
index d5bb9d7325ca..b7c6659c3080 100644
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -1487,25 +1487,32 @@ bool NativeObject::fillInAfterSwap(JSContext* cx, HandleNativeObject obj,
     MOZ_ASSERT(!priv);
   }
 
+  uint32_t oldDictionarySlotSpan =
+      obj->inDictionaryMode() ? obj->dictionaryModeSlotSpan() : 0;
+
   Zone* zone = obj->zone();
   if (obj->hasDynamicSlots()) {
     ObjectSlots* slotsHeader = obj->getSlotsHeader();
-    uint32_t oldDictionarySlotSpan = slotsHeader->dictionarySlotSpan();
     size_t size = ObjectSlots::allocSize(slotsHeader->capacity());
     zone->removeCellMemory(old, size, MemoryUse::ObjectSlots);
     js_free(slotsHeader);
-    obj->setEmptyDynamicSlots(oldDictionarySlotSpan);
+    obj->setEmptyDynamicSlots(0);
   }
 
   size_t ndynamic =
       calculateDynamicSlots(nfixed, values.length(), obj->getClass());
-  MOZ_ASSERT(ndynamic >= obj->numDynamicSlots());
-  if (ndynamic > obj->numDynamicSlots()) {
-    if (!obj->growSlots(cx, obj->numDynamicSlots(), ndynamic)) {
+  size_t currentSlots = obj->getSlotsHeader()->capacity();
+  MOZ_ASSERT(ndynamic >= currentSlots);
+  if (ndynamic > currentSlots) {
+    if (!obj->growSlots(cx, currentSlots, ndynamic)) {
       return false;
     }
   }
 
+  if (obj->inDictionaryMode()) {
+    obj->setDictionaryModeSlotSpan(oldDictionarySlotSpan);
+  }
+
   obj->initSlotRange(0, values.begin(), values.length());
 
   return true;