Bug 1545770 - Invalidate shape teleporting when transplanting prototy…

…pe objects. r=iain

This unifies the code for proto mutations and swap.

Without the patch, the tests fail an assertion in debug builds or hit a
correctness bug in release builds.

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

js/src/jit-test/tests/basic/shape-teleporting-transplant-1.js

@@ -0,0 +1,36 @@
+// Test for invalidating shape teleporting when transplanting objects on proto
+// chains.
+
+function checkGetProp(obj, expected) {
+    for (var i = 0; i < 50; i++) {
+        assertEq(obj.prop, expected);
+    }
+}
+
+Object.prototype.prop = 1234;
+
+// Construct the following proto chain:
+//
+//   receiver => protoA (FakeDOMObject) => protoB {prop: 567} => null
+const protoB = Object.create(null);
+protoB.prop = 567;
+const protoA = new FakeDOMObject();
+Object.setPrototypeOf(protoA, protoB);
+const receiver = Object.create(protoA);
+
+// Ensure all objects we allocated are tenured. This way we don't need to trigger
+// a GC later in TransplantableObject, which makes the test more reliable.
+gc();
+
+// Attach an IC for `receiver.prop`.
+checkGetProp(receiver, 567);
+
+// Swap protoA with another object. This must invalidate shape teleporting,
+// because the proto chain of `receiver` now looks like this:
+//
+//   receiver => protoA (new FakeDOMObject) => FakeDOMObject.prototype => Object.prototype => null
+const {transplant} = transplantableObject({object: protoA});
+transplant(this);
+
+// `receiver.prop` now gets `prop` from Object.prototype.
+checkGetProp(receiver, 1234);

js/src/jit-test/tests/basic/shape-teleporting-transplant-2.js

@@ -0,0 +1,16 @@
+// Put DOM object on prototype chain
+let x = new FakeDOMObject;
+let y = Object.create(x);
+
+// Transplant the DOM object while it is still a prototype
+let g = newGlobal({newCompartment: true});
+let { transplant } = transplantableObject({ object: x });
+
+// JIT an IC to access Object.prototype.toString
+function f(o) { return o.toString; }
+for (var i = 0; i < 20; ++i) { f(y) }
+
+// Transplanting should not interfere with teleporting
+transplant(g);
+x.toString = "override";
+assertEq(f(y), "override");

js/src/vm/JSObject.cpp

@@ -1234,7 +1234,9 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b,
   MOZ_RELEASE_ASSERT(js::ObjectMayBeSwapped(a));
   MOZ_RELEASE_ASSERT(js::ObjectMayBeSwapped(b));
 
-  Watchtower::watchObjectSwap(cx, a, b);
+  if (!Watchtower::watchObjectSwap(cx, a, b)) {
+    oomUnsafe.crash("watchObjectSwap");
+  }
 
   // Ensure we update any embedded nursery pointers in either object.
   gc::StoreBuffer& storeBuffer = cx->runtime()->gc.storeBuffer();
@@ -1252,15 +1254,6 @@ void JSObject::swap(JSContext* cx, HandleObject a, HandleObject b,
 
   unsigned r = NotifyGCPreSwap(a, b);
 
-  // Don't swap objects that may currently be participating in shape
-  // teleporting optimizations.
-  //
-  // See: ReshapeForProtoMutation, ReshapeForShadowedProp
-  MOZ_ASSERT_IF(a->is<NativeObject>() && a->isUsedAsPrototype(),
-                a->taggedProto() == TaggedProto());
-  MOZ_ASSERT_IF(b->is<NativeObject>() && b->isUsedAsPrototype(),
-                b->taggedProto() == TaggedProto());
-
   bool aIsProxyWithInlineValues =
       a->is<ProxyObject>() && a->as<ProxyObject>().usingInlineValueArray();
   bool bIsProxyWithInlineValues =

js/src/vm/Watchtower.cpp

@@ -169,17 +169,25 @@ static bool ReshapeForProtoMutation(JSContext* cx, HandleObject obj) {
   return true;
 }
 
+static bool WatchProtoChangeImpl(JSContext* cx, HandleObject obj) {
+  if (!obj->isUsedAsPrototype()) {
+    return true;
+  }
+  if (!ReshapeForProtoMutation(cx, obj)) {
+    return false;
+  }
+  if (obj->is<NativeObject>()) {
+    InvalidateMegamorphicCache(cx, obj.as<NativeObject>());
+  }
+  return true;
+}
+
 // static
 bool Watchtower::watchProtoChangeSlow(JSContext* cx, HandleObject obj) {
   MOZ_ASSERT(watchesProtoChange(obj));
 
-  if (obj->isUsedAsPrototype()) {
-    if (!ReshapeForProtoMutation(cx, obj)) {
-      return false;
-    }
-    if (obj->is<NativeObject>()) {
-      InvalidateMegamorphicCache(cx, obj.as<NativeObject>());
-    }
+  if (!WatchProtoChangeImpl(cx, obj)) {
+    return false;
   }
 
   if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
@@ -267,17 +275,22 @@ bool Watchtower::watchFreezeOrSealSlow(JSContext* cx,
 }
 
 // static
-void Watchtower::watchObjectSwapSlow(JSContext* cx, HandleObject a,
+bool Watchtower::watchObjectSwapSlow(JSContext* cx, HandleObject a,
                                      HandleObject b) {
   MOZ_ASSERT(watchesObjectSwap(a, b));
 
-  if (a->isUsedAsPrototype() && a->is<NativeObject>()) {
-    InvalidateMegamorphicCache(cx, a.as<NativeObject>());
+  // If we're swapping an object that's used as prototype, we're mutating the
+  // proto chains of other objects. Treat this as a proto change to ensure we
+  // invalidate shape teleporting and megamorphic caches.
+  if (!WatchProtoChangeImpl(cx, a)) {
+    return false;
   }
-  if (b->isUsedAsPrototype() && b->is<NativeObject>()) {
-    InvalidateMegamorphicCache(cx, b.as<NativeObject>());
+  if (!WatchProtoChangeImpl(cx, b)) {
+    return false;
   }
 
   // Note: we don't invoke the testing callback for swap because the objects may
   // not be safe to expose to JS at this point. See bug 1754699.
+
+  return true;
 }

js/src/vm/Watchtower.h

@@ -41,7 +41,7 @@ class Watchtower {
                                       HandleId id, PropertyFlags flags);
   static bool watchFreezeOrSealSlow(JSContext* cx, Handle<NativeObject*> obj);
   static bool watchProtoChangeSlow(JSContext* cx, HandleObject obj);
-  static void watchObjectSwapSlow(JSContext* cx, HandleObject a,
+  static bool watchObjectSwapSlow(JSContext* cx, HandleObject a,
                                   HandleObject b);
 
  public:
@@ -107,11 +107,11 @@ class Watchtower {
     }
     return watchProtoChangeSlow(cx, obj);
   }
-  static void watchObjectSwap(JSContext* cx, HandleObject a, HandleObject b) {
+  static bool watchObjectSwap(JSContext* cx, HandleObject a, HandleObject b) {
     if (MOZ_LIKELY(!watchesObjectSwap(a, b))) {
-      return;
+      return true;
     }
-    watchObjectSwapSlow(cx, a, b);
+    return watchObjectSwapSlow(cx, a, b);
   }
 };