Bug 1782495 - Replace watchtower testing callback with a log-based mechanism. r=iain

The ability to run arbitrary JS can cause various problems. This replaces the callback
with a different mechanism to avoid this.

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

js/src/builtin/TestingFunctions.cpp

@@ -3012,18 +3012,31 @@ static bool CheckObjectWithManyReservedSlots(JSContext* cx, unsigned argc,
   return true;
 }
 
-static bool SetWatchtowerCallback(JSContext* cx, unsigned argc, Value* vp) {
+static bool GetWatchtowerLog(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
-  JSFunction* fun = nullptr;
+  Rooted<GCVector<Value>> values(cx, GCVector<Value>(cx));
-  if (args.length() != 1 || !IsFunctionObject(args[0], &fun)) {
+
-    JS_ReportErrorASCII(cx, "Expected a single function argument.");
+  if (auto* log = cx->runtime()->watchtowerTestingLog.ref().get()) {
+    Rooted<JSObject*> elem(cx);
+    for (PlainObject* obj : *log) {
+      elem = obj;
+      if (!cx->compartment()->wrap(cx, &elem)) {
+        return false;
+      }
+      if (!values.append(ObjectValue(*elem))) {
+        return false;
+      }
+    }
+    log->clearAndFree();
+  }
+
+  ArrayObject* arr = NewDenseCopiedArray(cx, values.length(), values.begin());
+  if (!arr) {
     return false;
   }
 
-  cx->watchtowerTestingCallbackRef() = fun;
+  args.rval().setObject(*arr);
-
-  args.rval().setUndefined();
   return true;
 }
 
@@ -3035,8 +3048,16 @@ static bool AddWatchtowerTarget(JSContext* cx, unsigned argc, Value* vp) {
     return false;
   }
 
+  if (!cx->runtime()->watchtowerTestingLog.ref()) {
+    auto vec = cx->make_unique<JSRuntime::RootedPlainObjVec>(cx);
+    if (!vec) {
+      return false;
+    }
+    cx->runtime()->watchtowerTestingLog = std::move(vec);
+  }
+
   RootedObject obj(cx, &args[0].toObject());
-  if (!JSObject::setUseWatchtowerTestingCallback(cx, obj)) {
+  if (!JSObject::setUseWatchtowerTestingLog(cx, obj)) {
     return false;
   }
 
@@ -8176,10 +8197,11 @@ static const JSFunctionSpecWithHelp TestingFunctions[] = {
 "  Checks the reserved slots set by newObjectWithManyReservedSlots still hold the expected\n"
 "  values."),
 
-    JS_FN_HELP("setWatchtowerCallback", SetWatchtowerCallback, 1, 0,
+    JS_FN_HELP("getWatchtowerLog", GetWatchtowerLog, 0, 0,
-"setWatchtowerCallback(function)",
+"getWatchtowerLog()",
-"  Use the given function as callback for objects added to Watchtower by\n"
+"  Returns the Watchtower log recording object changes for objects for which\n"
-"  addWatchtowerTarget. The callback is called with the following arguments:\n"
+"  addWatchtowerTarget was called. The internal log is cleared. The return\n"
+"  value is an array of plain objects with the following properties:\n"
 "  - kind: a string describing the kind of mutation, for example \"add-prop\"\n"
 "  - object: the object being mutated\n"
 "  - extra: an extra value, for example the name of the property being added"),

js/src/jit-test/tests/watchtower/basic.js

@@ -1,17 +1,16 @@
-setWatchtowerCallback(function(kind, object, extra) {
+function getLogString(obj) {
-    assertEq(object, o);
+    let log = getWatchtowerLog();
-    if (typeof extra === "symbol") {
+    return log.map(item => {
-        extra = "<symbol>";
+        assertEq(item.object, obj);
+        if (typeof item.extra === "symbol") {
+            item.extra = "<symbol>";
+        }
+        return item.kind + (item.extra ? ": " + item.extra : "");
+    }).join("\n");
 }
-    log.push(kind + (extra ? ": " + extra : ""));
-});
-
-let log;
-let o;
 
 function testBasic() {
-    log = [];
+    let o = {};
-    o = {};
     addWatchtowerTarget(o);
 
     o.x = 1;
@@ -28,7 +27,8 @@ function testBasic() {
     Object.seal(o);
     Object.freeze(o);
 
-    assertEq(log.join("\n"),
+    let log = getLogString(o);
+    assertEq(log,
 `add-prop: x
 add-prop: y
 add-prop: <symbol>
@@ -49,38 +49,38 @@ for (var i = 0; i < 20; i++) {
 
 // Object.assign edge case.
 function testAssign() {
-    o = {};
+    let o = {};
     o.x = 1;
     delete o.x;
     let from = {x: 1, y: 2, z: 3, a: 4};
-    log = [];
     addWatchtowerTarget(o);
     addWatchtowerTarget(from);
     Object.assign(o, from);
-    assertEq(log.join("\n"), "add-prop: x\nadd-prop: y\nadd-prop: z\nadd-prop: a");
+    let log = getLogString(o);
+    assertEq(log, "add-prop: x\nadd-prop: y\nadd-prop: z\nadd-prop: a");
 }
 testAssign();
 
 function testJit() {
     for (var i = 0; i < 20; i++) {
-        o = {};
+        let o = {};
         addWatchtowerTarget(o);
-        log = [];
         o.x = 1;
         o.y = 2;
-        assertEq(log.join("\n"), "add-prop: x\nadd-prop: y");
+        let log = getLogString(o);
+        assertEq(log, "add-prop: x\nadd-prop: y");
     }
 }
 testJit();
 
 // array.length is a custom data property.
 function testCustomDataProp() {
-    o = [];
+    let o = [];
-    log = [];
     addWatchtowerTarget(o);
 
     Object.defineProperty(o, "length", {writable: false});
-    assertEq(log.join("\n"), "change-prop: length");
+    let log = getLogString(o);
+    assertEq(log, "change-prop: length");
 }
 for (var i = 0; i < 20; i++) {
     testCustomDataProp();

js/src/vm/JSContext.cpp

@@ -1027,7 +1027,6 @@ JSContext::JSContext(JSRuntime* runtime, const JS::ContextOptions& options)
 #endif
       generatingError(this, false),
       cycleDetectorVector_(this, this),
-      watchtowerTestingCallback_(this),
       data(nullptr),
       asyncStackForNewActivations_(this),
       asyncCauseForNewActivations(this, nullptr),

js/src/vm/JSContext.h

@@ -684,17 +684,6 @@ struct JS_PUBLIC_API JSContext : public JS::RootingContext,
     return cycleDetectorVector_.ref();
   }
 
- private:
-  js::ContextData<JS::PersistentRooted<JSFunction*>> watchtowerTestingCallback_;
-
- public:
-  JSFunction*& watchtowerTestingCallbackRef() {
-    if (!watchtowerTestingCallback_.ref().initialized()) {
-      watchtowerTestingCallback_.ref().init(this);
-    }
-    return watchtowerTestingCallback_.ref().get();
-  }
-
   /* Client opaque pointer. */
   js::UnprotectedData<void*> data;
 

js/src/vm/JSObject.h

@@ -186,12 +186,11 @@ class JSObject
     return setFlag(cx, obj, js::ObjectFlag::IsUsedAsPrototype);
   }
 
-  bool useWatchtowerTestingCallback() const {
+  bool useWatchtowerTestingLog() const {
-    return hasFlag(js::ObjectFlag::UseWatchtowerTestingCallback);
+    return hasFlag(js::ObjectFlag::UseWatchtowerTestingLog);
   }
-  static bool setUseWatchtowerTestingCallback(JSContext* cx,
+  static bool setUseWatchtowerTestingLog(JSContext* cx, JS::HandleObject obj) {
-                                              JS::HandleObject obj) {
+    return setFlag(cx, obj, js::ObjectFlag::UseWatchtowerTestingLog);
-    return setFlag(cx, obj, js::ObjectFlag::UseWatchtowerTestingCallback);
   }
 
   // A "qualified" varobj is the object on which "qualified" variable

js/src/vm/ObjectFlags.h

@@ -61,8 +61,8 @@ enum class ObjectFlag : uint16_t {
   // objects. See also the SMDOC comment in vm/GetterSetter.h.
   HadGetterSetterChange = 1 << 10,
 
-  // If set, invoke the watchtower testing callback for changes to this object.
+  // If set, use the watchtower testing mechanism to log changes to this object.
-  UseWatchtowerTestingCallback = 1 << 11,
+  UseWatchtowerTestingLog = 1 << 11,
 };
 
 using ObjectFlags = EnumFlags<ObjectFlag>;

js/src/vm/Runtime.cpp

@@ -121,6 +121,7 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
       defaultLocale(nullptr),
       profilingScripts(false),
       scriptAndCountsVector(nullptr),
+      watchtowerTestingLog(nullptr),
       lcovOutput_(),
       jitRuntime_(nullptr),
       gc(thisFromCtor()),
@@ -220,6 +221,8 @@ void JSRuntime::destroyRuntime() {
   sharedIntlData.ref().destroyInstance();
 #endif
 
+  watchtowerTestingLog.ref().reset();
+
   // Caches might hold on ScriptData which are saved in the ScriptDataTable.
   // Clear all stencils from caches to remove ScriptDataTable entries.
   caches().purgeStencils();

js/src/vm/Runtime.h

@@ -678,6 +678,10 @@ struct JSRuntime {
   js::MainThreadData<JS::PersistentRooted<js::ScriptAndCountsVector>*>
       scriptAndCountsVector;
 
+  using RootedPlainObjVec = JS::PersistentRooted<
+      JS::GCVector<js::PlainObject*, 0, js::SystemAllocPolicy>>;
+  js::MainThreadData<js::UniquePtr<RootedPlainObjVec>> watchtowerTestingLog;
+
  private:
   /* Code coverage output. */
   js::UnprotectedData<js::coverage::LCovRuntime> lcovOutput_;

js/src/vm/Watchtower.cpp

@@ -11,6 +11,7 @@
 #include "vm/JSContext.h"
 #include "vm/JSObject.h"
 #include "vm/NativeObject.h"
+#include "vm/PlainObject.h"
 #include "vm/Realm.h"
 
 #include "vm/Compartment-inl.h"
@@ -19,38 +20,38 @@
 
 using namespace js;
 
-static bool InvokeWatchtowerCallback(JSContext* cx, const char* kind,
+static bool AddToWatchtowerLog(JSContext* cx, const char* kind,
                                HandleObject obj, HandleValue extra) {
-  // Invoke the callback set by the setWatchtowerCallback testing function with
+  // Add an object storing {kind, object, extra} to the log for testing
-  // arguments (kind, obj, extra).
+  // purposes.
 
-  if (!cx->watchtowerTestingCallbackRef()) {
+  MOZ_ASSERT(obj->useWatchtowerTestingLog());
-    return true;
-  }
 
   RootedString kindString(cx, NewStringCopyZ<CanGC>(cx, kind));
   if (!kindString) {
     return false;
   }
 
-  constexpr size_t NumArgs = 3;
+  Rooted<PlainObject*> logObj(cx, NewPlainObjectWithProto(cx, nullptr));
-  JS::RootedValueArray<NumArgs> argv(cx);
+  if (!logObj) {
-  argv[0].setString(kindString);
-  argv[1].setObject(*obj);
-  argv[2].set(extra);
-
-  RootedValue funVal(cx, ObjectValue(*cx->watchtowerTestingCallbackRef()));
-  AutoRealm ar(cx, &funVal.toObject());
-
-  for (size_t i = 0; i < NumArgs; i++) {
-    if (!cx->compartment()->wrap(cx, argv[i])) {
     return false;
   }
+  if (!JS_DefineProperty(cx, logObj, "kind", kindString, JSPROP_ENUMERATE)) {
+    return false;
+  }
+  if (!JS_DefineProperty(cx, logObj, "object", obj, JSPROP_ENUMERATE)) {
+    return false;
+  }
+  if (!JS_DefineProperty(cx, logObj, "extra", extra, JSPROP_ENUMERATE)) {
+    return false;
   }
 
-  RootedValue rval(cx);
+  if (!cx->runtime()->watchtowerTestingLog->append(logObj)) {
-  return JS_CallFunctionValue(cx, nullptr, funVal, HandleValueArray(argv),
+    ReportOutOfMemory(cx);
-                              &rval);
+    return false;
+  }
+
+  return true;
 }
 
 static bool ReshapeForShadowedProp(JSContext* cx, Handle<NativeObject*> obj,
@@ -111,9 +112,9 @@ bool Watchtower::watchPropertyAddSlow(JSContext* cx, Handle<NativeObject*> obj,
     }
   }
 
-  if (MOZ_UNLIKELY(obj->useWatchtowerTestingCallback())) {
+  if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
     RootedValue val(cx, IdToValue(id));
-    if (!InvokeWatchtowerCallback(cx, "add-prop", obj, val)) {
+    if (!AddToWatchtowerLog(cx, "add-prop", obj, val)) {
       return false;
     }
   }
@@ -179,8 +180,8 @@ bool Watchtower::watchProtoChangeSlow(JSContext* cx, HandleObject obj) {
     }
   }
 
-  if (MOZ_UNLIKELY(obj->useWatchtowerTestingCallback())) {
+  if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
-    if (!InvokeWatchtowerCallback(cx, "proto-change", obj,
+    if (!AddToWatchtowerLog(cx, "proto-change", obj,
                             JS::UndefinedHandleValue)) {
       return false;
     }
@@ -199,9 +200,9 @@ bool Watchtower::watchPropertyRemoveSlow(JSContext* cx,
     InvalidateMegamorphicCache(cx, obj);
   }
 
-  if (MOZ_UNLIKELY(obj->useWatchtowerTestingCallback())) {
+  if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
     RootedValue val(cx, IdToValue(id));
-    if (!InvokeWatchtowerCallback(cx, "remove-prop", obj, val)) {
+    if (!AddToWatchtowerLog(cx, "remove-prop", obj, val)) {
       return false;
     }
   }
@@ -219,9 +220,9 @@ bool Watchtower::watchPropertyChangeSlow(JSContext* cx,
     InvalidateMegamorphicCache(cx, obj);
   }
 
-  if (MOZ_UNLIKELY(obj->useWatchtowerTestingCallback())) {
+  if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
     RootedValue val(cx, IdToValue(id));
-    if (!InvokeWatchtowerCallback(cx, "change-prop", obj, val)) {
+    if (!AddToWatchtowerLog(cx, "change-prop", obj, val)) {
       return false;
     }
   }
@@ -234,8 +235,8 @@ bool Watchtower::watchFreezeOrSealSlow(JSContext* cx,
                                        Handle<NativeObject*> obj) {
   MOZ_ASSERT(watchesFreezeOrSeal(obj));
 
-  if (MOZ_UNLIKELY(obj->useWatchtowerTestingCallback())) {
+  if (MOZ_UNLIKELY(obj->useWatchtowerTestingLog())) {
-    if (!InvokeWatchtowerCallback(cx, "freeze-or-seal", obj,
+    if (!AddToWatchtowerLog(cx, "freeze-or-seal", obj,
                             JS::UndefinedHandleValue)) {
       return false;
     }

js/src/vm/Watchtower.h

@@ -46,28 +46,28 @@ class Watchtower {
 
  public:
   static bool watchesPropertyAdd(NativeObject* obj) {
-    return obj->hasAnyFlag({ObjectFlag::IsUsedAsPrototype,
+    return obj->hasAnyFlag(
-                            ObjectFlag::UseWatchtowerTestingCallback});
+        {ObjectFlag::IsUsedAsPrototype, ObjectFlag::UseWatchtowerTestingLog});
   }
   static bool watchesPropertyRemove(NativeObject* obj) {
-    return obj->hasAnyFlag({ObjectFlag::IsUsedAsPrototype,
+    return obj->hasAnyFlag(
-                            ObjectFlag::UseWatchtowerTestingCallback});
+        {ObjectFlag::IsUsedAsPrototype, ObjectFlag::UseWatchtowerTestingLog});
   }
   static bool watchesPropertyChange(NativeObject* obj) {
-    return obj->hasAnyFlag({ObjectFlag::IsUsedAsPrototype,
+    return obj->hasAnyFlag(
-                            ObjectFlag::UseWatchtowerTestingCallback});
+        {ObjectFlag::IsUsedAsPrototype, ObjectFlag::UseWatchtowerTestingLog});
   }
   static bool watchesFreezeOrSeal(NativeObject* obj) {
-    return obj->hasAnyFlag({ObjectFlag::UseWatchtowerTestingCallback});
+    return obj->hasAnyFlag({ObjectFlag::UseWatchtowerTestingLog});
   }
   static bool watchesProtoChange(JSObject* obj) {
-    return obj->hasAnyFlag({ObjectFlag::IsUsedAsPrototype,
+    return obj->hasAnyFlag(
-                            ObjectFlag::UseWatchtowerTestingCallback});
+        {ObjectFlag::IsUsedAsPrototype, ObjectFlag::UseWatchtowerTestingLog});
   }
   static bool watchesObjectSwap(JSObject* a, JSObject* b) {
     auto watches = [](JSObject* obj) {
-      return obj->hasAnyFlag({ObjectFlag::IsUsedAsPrototype,
+      return obj->hasAnyFlag(
-                              ObjectFlag::UseWatchtowerTestingCallback});
+          {ObjectFlag::IsUsedAsPrototype, ObjectFlag::UseWatchtowerTestingLog});
     };
     return watches(a) || watches(b);
   }