Bug 1644160 - Initial Proxy Support for Private Fields r=jorendorff

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

js/public/Proxy.h

@@ -317,6 +317,22 @@ class JS_FRIEND_API BaseProxyHandler {
                    HandleValue v, HandleValue receiver,
                    ObjectOpResult& result) const;
 
+  // Private fields don't use [[Has]], [[Get]] and [[Set]] semantics, as they
+  // technically use a weak-map semantics, however we end up on these paths with
+  // our implementation.
+  virtual bool hasPrivate(JSContext* cx, HandleObject proxy, HandleId id,
+                          bool* bp) const;
+  virtual bool getPrivate(JSContext* cx, HandleObject proxy,
+                          HandleValue receiver, HandleId id,
+                          MutableHandleValue vp) const;
+  virtual bool setPrivate(JSContext* cx, HandleObject proxy, HandleId id,
+                          HandleValue v, HandleValue receiver,
+                          ObjectOpResult& result) const;
+
+  virtual bool definePrivateField(JSContext* cx, HandleObject proxy,
+                                  HandleId id, Handle<PropertyDescriptor> desc,
+                                  ObjectOpResult& result) const;
+
   /*
    * [[Call]] and [[Construct]] are standard internal methods but according
    * to the spec, they are not present on every object.
@@ -386,6 +402,8 @@ namespace detail {
 //
 // Every proxy has a ProxyValueArray that contains the following Values:
 //
+// - The expando slot. This is used to hold private fields should they be
+//   stamped into a non-forwarding proxy type.
 // - The private slot.
 // - The reserved slots. The number of slots is determined by the proxy's Class.
 //
@@ -419,10 +437,12 @@ struct ProxyReservedSlots {
 };
 
 struct ProxyValueArray {
+  Value expandoSlot;
   Value privateSlot;
   ProxyReservedSlots reservedSlots;
 
   void init(size_t nreserved) {
+    expandoSlot = JS::ObjectOrNullValue(nullptr);
     privateSlot = JS::UndefinedValue();
     reservedSlots.init(nreserved);
   }
@@ -505,6 +525,10 @@ inline const Value& GetProxyPrivate(const JSObject* obj) {
   return detail::GetProxyDataLayout(obj)->values()->privateSlot;
 }
 
+inline const Value& GetProxyExpando(const JSObject* obj) {
+  return detail::GetProxyDataLayout(obj)->values()->expandoSlot;
+}
+
 inline JSObject* GetProxyTargetObject(JSObject* obj) {
   return GetProxyPrivate(obj).toObjectOrNull();
 }

js/src/jit-test/tests/fields/private-proxy-oom.js

@@ -0,0 +1,48 @@
+// |jit-test| skip-if: !('oomTest' in this); --enable-private-fields
+// Check for proxy expando OOM issues.
+
+function assertThrowsTypeError(f) {
+  assertThrowsInstanceOf(f, TypeError);
+}
+
+
+function testing() {
+  var target = {};
+  var p1 = new Proxy(target, {});
+  var p2 = new Proxy(target, {});
+
+  class A extends class {
+    constructor(o) {
+      return o;
+    }
+  }
+  {
+    #field = 10;
+    static gf(o) {
+      return o.#field;
+    }
+    static sf(o) {
+      o.#field = 15;
+    }
+  }
+
+  // Verify field handling on the proxy we install it on.
+  new A(p1);
+  assertEq(A.gf(p1), 10);
+  A.sf(p1)
+  assertEq(A.gf(p1), 15);
+
+  // Should't be on the target
+  assertThrowsTypeError(() => A.gf(target));
+
+  // Can't set the field, doesn't exist
+  assertThrowsTypeError(() => A.sf(p2));
+
+  // Definitely can't get the field, doesn't exist.
+  assertThrowsTypeError(() => A.gf(p2));
+
+  // Still should't be on the target.
+  assertThrowsTypeError(() => A.gf(target));
+}
+
+oomTest(testing);

js/src/proxy/BaseProxyHandler.cpp

@@ -22,6 +22,22 @@ bool BaseProxyHandler::enter(JSContext* cx, HandleObject wrapper, HandleId id,
   return true;
 }
 
+bool BaseProxyHandler::hasPrivate(JSContext* cx, HandleObject proxy,
+                                  HandleId id, bool* bp) const {
+  assertEnteredPolicy(cx, proxy, id, GET);
+
+  // For BaseProxyHandler, private names are stored in the expando object.
+  RootedObject expando(cx, proxy->as<ProxyObject>().expando().toObjectOrNull());
+
+  // If there is no expando object, then there is no private field.
+  if (!expando) {
+    *bp = false;
+    return true;
+  }
+
+  return HasOwnProperty(cx, expando, id, bp);
+}
+
 bool BaseProxyHandler::has(JSContext* cx, HandleObject proxy, HandleId id,
                            bool* bp) const {
   assertEnteredPolicy(cx, proxy, id, GET);
@@ -69,6 +85,35 @@ bool BaseProxyHandler::hasOwn(JSContext* cx, HandleObject proxy, HandleId id,
   return true;
 }
 
+bool BaseProxyHandler::getPrivate(JSContext* cx, HandleObject proxy,
+                                  HandleValue receiver, HandleId id,
+                                  MutableHandleValue vp) const {
+  assertEnteredPolicy(cx, proxy, id, GET);
+
+  // For BaseProxyHandler, private names are stored in the expando object.
+  RootedObject expando(cx, proxy->as<ProxyObject>().expando().toObjectOrNull());
+
+  // We must have the expando, or GetPrivateElemOperation didn't call
+  // hasPrivate first.
+  MOZ_ASSERT(expando);
+
+  // Because we controlled the creation of the expando, we know it's not a
+  // proxy, and so can safely call internal methods on it without worrying about
+  // exposing information about private names.
+  Rooted<PropertyDescriptor> desc(cx);
+  if (!GetOwnPropertyDescriptor(cx, expando, id, &desc)) {
+    return false;
+  }
+
+  // We must have the object, same reasoning as the expando.
+  MOZ_ASSERT(desc.object());
+  MOZ_ASSERT(desc.hasValue());
+  MOZ_ASSERT(desc.isDataDescriptor());
+
+  vp.set(desc.value());
+  return true;
+}
+
 bool BaseProxyHandler::get(JSContext* cx, HandleObject proxy,
                            HandleValue receiver, HandleId id,
                            MutableHandleValue vp) const {
@@ -125,6 +170,33 @@ bool BaseProxyHandler::get(JSContext* cx, HandleObject proxy,
   return CallGetter(cx, receiver, getterFunc, vp);
 }
 
+bool BaseProxyHandler::setPrivate(JSContext* cx, HandleObject proxy,
+                                  HandleId id, HandleValue v,
+                                  HandleValue receiver,
+                                  ObjectOpResult& result) const {
+  assertEnteredPolicy(cx, proxy, id, SET);
+  MOZ_ASSERT(JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName());
+
+  // For BaseProxyHandler, private names are stored in the expando object.
+  RootedObject expando(cx, proxy->as<ProxyObject>().expando().toObjectOrNull());
+
+  // SetPrivateElementOperation checks for hasPrivate first, which ensures the
+  // expando exsists.
+  MOZ_ASSERT(expando);
+
+  Rooted<PropertyDescriptor> ownDesc(cx);
+  if (!GetOwnPropertyDescriptor(cx, expando, id, &ownDesc)) {
+    return false;
+  }
+  ownDesc.assertCompleteIfFound();
+
+  MOZ_ASSERT(ownDesc.object());
+
+  RootedValue expandoValue(cx, proxy->as<ProxyObject>().expando());
+  return SetPropertyIgnoringNamedGetter(cx, expando, id, v, expandoValue,
+                                        ownDesc, result);
+}
+
 bool BaseProxyHandler::set(JSContext* cx, HandleObject proxy, HandleId id,
                            HandleValue v, HandleValue receiver,
                            ObjectOpResult& result) const {
@@ -230,6 +302,41 @@ bool js::SetPropertyIgnoringNamedGetter(JSContext* cx, HandleObject obj,
   return result.succeed();
 }
 
+bool BaseProxyHandler::definePrivateField(JSContext* cx, HandleObject proxy,
+                                          HandleId id,
+                                          Handle<PropertyDescriptor> desc,
+                                          ObjectOpResult& result) const {
+  MOZ_ASSERT(JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName());
+
+  // For BaseProxyHandler, private names are stored in the expando object.
+  RootedObject expando(cx, proxy->as<ProxyObject>().expando().toObjectOrNull());
+
+  if (!expando) {
+    expando = NewObjectWithGivenProto<PlainObject>(cx, nullptr);
+    if (!expando) {
+      return false;
+    }
+
+    proxy->as<ProxyObject>().setExpando(expando);
+  }
+
+  // Get a new property descriptor for the expando.
+  Rooted<PropertyDescriptor> ownDesc(cx);
+  if (!GetOwnPropertyDescriptor(cx, expando, id, &ownDesc)) {
+    return false;
+  }
+  ownDesc.assertCompleteIfFound();
+  // Copy the incoming value into the expando descriptor.
+  ownDesc.setValue(desc.value());
+
+  // PrivateFieldAdd: Step 4: We must throw a type error if obj already has
+  // this property. This should have been checked by InitPrivateElemOperation
+  // calling hasPrivate already.
+  MOZ_ASSERT(!ownDesc.object());
+
+  return DefineProperty(cx, expando, id, ownDesc, result);
+}
+
 bool BaseProxyHandler::getOwnEnumerablePropertyKeys(
     JSContext* cx, HandleObject proxy, MutableHandleIdVector props) const {
   assertEnteredPolicy(cx, proxy, JSID_VOID, ENUMERATE);

js/src/proxy/Proxy.cpp

@@ -86,6 +86,9 @@ bool Proxy::getOwnPropertyDescriptor(JSContext* cx, HandleObject proxy,
   if (!policy.allowed()) {
     return policy.returnValue();
   }
+
+  // Private names shouldn't take this path
+  MOZ_ASSERT_IF(JSID_IS_SYMBOL(id), !JSID_TO_SYMBOL(id)->isPrivateName());
   return handler->getOwnPropertyDescriptor(cx, proxy, id, desc);
 }
 
@@ -103,6 +106,17 @@ bool Proxy::defineProperty(JSContext* cx, HandleObject proxy, HandleId id,
     }
     return result.succeed();
   }
+
+  // Private field accesses have different semantics depending on the kind
+  // of proxy involved, and so take a different path compared to regular
+  // [[Get]] operations. For example, scripted handlers don't fire traps
+  // when accessing private fields (because of the WeakMap semantics)
+  bool isPrivate = JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName();
+  if (isPrivate) {
+    return proxy->as<ProxyObject>().handler()->definePrivateField(cx, proxy, id,
+                                                                  desc, result);
+  }
+
   return proxy->as<ProxyObject>().handler()->defineProperty(cx, proxy, id, desc,
                                                             result);
 }
@@ -135,6 +149,11 @@ bool Proxy::delete_(JSContext* cx, HandleObject proxy, HandleId id,
     }
     return ok;
   }
+
+  // Private names shouldn't take this path, as deleting a private name
+  // should be a syntax error.
+  MOZ_ASSERT_IF(JSID_IS_SYMBOL(id), !JSID_TO_SYMBOL(id)->isPrivateName());
+
   return proxy->as<ProxyObject>().handler()->delete_(cx, proxy, id, result);
 }
 
@@ -233,6 +252,9 @@ bool Proxy::has(JSContext* cx, HandleObject proxy, HandleId id, bool* bp) {
     return policy.returnValue();
   }
 
+  // Private names shouldn't take this path, but only hasOwn;
+  MOZ_ASSERT_IF(JSID_IS_SYMBOL(id), !JSID_TO_SYMBOL(id)->isPrivateName());
+
   if (handler->hasPrototype()) {
     if (!handler->hasOwn(cx, proxy, id, bp)) {
       return false;
@@ -275,6 +297,16 @@ bool Proxy::hasOwn(JSContext* cx, HandleObject proxy, HandleId id, bool* bp) {
   if (!policy.allowed()) {
     return policy.returnValue();
   }
+
+  // Private field accesses have different semantics depending on the kind
+  // of proxy involved, and so take a different path compared to regular
+  // [[Get]] operations. For example, scripted handlers don't fire traps
+  // when accessing private fields (because of the WeakMap semantics)
+  bool isPrivate = JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName();
+  if (isPrivate) {
+    return handler->hasPrivate(cx, proxy, id, bp);
+  }
+
   return handler->hasOwn(cx, proxy, id, bp);
 }
 
@@ -311,6 +343,15 @@ MOZ_ALWAYS_INLINE bool Proxy::getInternal(JSContext* cx, HandleObject proxy,
     return policy.returnValue();
   }
 
+  // Private field accesses have different semantics depending on the kind
+  // of proxy involved, and so take a different path compared to regular
+  // [[Get]] operations. For example, scripted handlers don't fire traps
+  // when accessing private fields (because of the WeakMap semantics)
+  bool isPrivate = JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName();
+  if (isPrivate) {
+    return handler->getPrivate(cx, proxy, receiver, id, vp);
+  }
+
   if (handler->hasPrototype()) {
     bool own;
     if (!handler->hasOwn(cx, proxy, id, &own)) {
@@ -365,6 +406,7 @@ MOZ_ALWAYS_INLINE bool Proxy::setInternal(JSContext* cx, HandleObject proxy,
   if (!CheckRecursionLimit(cx)) {
     return false;
   }
+
   const BaseProxyHandler* handler = proxy->as<ProxyObject>().handler();
   AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true);
   if (!policy.allowed()) {
@@ -374,6 +416,17 @@ MOZ_ALWAYS_INLINE bool Proxy::setInternal(JSContext* cx, HandleObject proxy,
     return result.succeed();
   }
 
+  // Private field accesses have different semantics depending on the kind
+  // of proxy involved, and so take a different path compared to regular
+  // [[Set]] operations.
+  //
+  // This doesn't interact with hasPrototype, as PrivateFields are always
+  // own propertiers, and so we never deal with prototype traversals.
+  bool isPrivate = JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id)->isPrivateName();
+  if (isPrivate) {
+    return handler->setPrivate(cx, proxy, id, v, receiver, result);
+  }
+
   // Special case. See the comment on BaseProxyHandler::mHasPrototype.
   if (handler->hasPrototype()) {
     return handler->BaseProxyHandler::set(cx, proxy, id, v, receiver, result);
@@ -664,6 +717,7 @@ void ProxyObject::trace(JSTracer* trc, JSObject* obj) {
   ProxyObject* proxy = &obj->as<ProxyObject>();
 
   TraceEdge(trc, proxy->shapePtr(), "ProxyObject_shape");
+  TraceNullableEdge(trc, proxy->slotOfExpando(), "expando");
 
 #ifdef DEBUG
   if (TlsContext.get()->isStrictProxyCheckingEnabled() &&

js/src/tests/non262/PrivateName/proxy-1.js

@@ -0,0 +1,21 @@
+// |reftest| shell-option(--enable-private-fields)
+
+class A {
+  #x = 10;
+  g() {
+    return this.#x;
+  }
+};
+
+var p = new Proxy(new A, {});
+var completed = false;
+try {
+  p.g();
+  completed = true;
+} catch (e) {
+  assertEq(e instanceof TypeError, true);
+}
+assertEq(completed, false);
+
+
+if (typeof reportCompare === 'function') reportCompare(0, 0);

js/src/tests/non262/PrivateName/proxy-init-set.js

@@ -0,0 +1,75 @@
+// |reftest| shell-option(--enable-private-fields)
+
+// Ensure that the distinction between Proxy Init and Proxy Set holds
+
+function assertThrowsTypeError(f) {
+  var type;
+  try {
+    f();
+  } catch (ex) {
+    type = ex.name;
+  }
+  assertEq(type, 'TypeError');
+}
+
+
+
+var target = {};
+var p1 = new Proxy(target, {});
+var p2 = new Proxy(target, {});
+
+class Base {
+  constructor(o) {
+    return o;
+  }
+}
+
+class A extends Base {
+  #field = 10;
+  static gf(o) {
+    return o.#field;
+  }
+  static sf(o) {
+    o.#field = 15;
+  }
+}
+
+class B extends Base {
+  #field = 25;
+  static gf(o) {
+    return o.#field;
+  }
+  static sf(o) {
+    o.#field = 20;
+  }
+}
+
+// Verify field handling on the proxy we install it on.
+new A(p1);
+assertEq(A.gf(p1), 10);
+A.sf(p1)
+assertEq(A.gf(p1), 15);
+
+// Despite P1 being stamped with A's field, it shouldn't
+// be sufficient to set B's field.
+assertThrowsTypeError(() => B.sf(p1));
+assertThrowsTypeError(() => B.gf(p1));
+assertThrowsTypeError(() => B.sf(p1));
+new B(p1);
+assertEq(B.gf(p1), 25);
+B.sf(p1);
+assertEq(B.gf(p1), 20);
+
+// A's field should't be on the target
+assertThrowsTypeError(() => A.gf(target));
+
+// Can't set the field, doesn't exist
+assertThrowsTypeError(() => A.sf(p2));
+
+// Definitely can't get the field, doesn't exist.
+assertThrowsTypeError(() => A.gf(p2));
+
+// Still should't be on the target.
+assertThrowsTypeError(() => A.gf(target));
+
+if (typeof reportCompare === 'function') reportCompare(0, 0);

js/src/vm/ProxyObject.cpp

@@ -61,6 +61,10 @@ void ProxyObject::init(const BaseProxyHandler* handler, HandleValue priv,
   } else {
     setSameCompartmentPrivate(priv);
   }
+
+  // The expando slot is nullptr until required by the installation of
+  // a private field.
+  setExpando(nullptr);
 }
 
 /* static */
@@ -267,6 +271,18 @@ inline void ProxyObject::setPrivate(const Value& priv) {
   *slotOfPrivate() = priv;
 }
 
+void ProxyObject::setExpando(JSObject* expando) {
+  // Ensure that we don't accidentally end up pointing to a
+  // grey object, which would violate GC invariants.
+  MOZ_ASSERT_IF(IsMarkedBlack(this) && expando,
+                !JS::GCThingIsMarkedGray(JS::GCCellPtr(expando)));
+
+  // Ensure we're in the same compartment as the proxy object: Don't want the
+  // expando to end up as a CCW.
+  MOZ_ASSERT_IF(expando, expando->compartment() == compartment());
+  *slotOfExpando() = ObjectOrNullValue(expando);
+}
+
 void ProxyObject::nuke() {
   // Notify the zone that a delegate is no longer a delegate. Be careful not to
   // expose this pointer, because it has already been removed from the wrapper

js/src/vm/ProxyObject.h

@@ -68,6 +68,9 @@ class ProxyObject : public JSObject {
                                                     HandleValueVector values);
 
   const Value& private_() const { return GetProxyPrivate(this); }
+  const Value& expando() const { return GetProxyExpando(this); }
+
+  void setExpando(JSObject* expando);
 
   void setCrossCompartmentPrivate(const Value& priv);
   void setSameCompartmentPrivate(const Value& priv);
@@ -109,6 +112,11 @@ class ProxyObject : public JSObject {
         &detail::GetProxyDataLayout(this)->values()->privateSlot);
   }
 
+  GCPtrValue* slotOfExpando() {
+    return reinterpret_cast<GCPtrValue*>(
+        &detail::GetProxyDataLayout(this)->values()->expandoSlot);
+  }
+
   void setPrivate(const Value& priv);
 
   static bool isValidProxyClass(const JSClass* clasp) {