Bug 1536880 - Add accessors to ReceiverGuard, and assert invariants r=iain

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

--HG--
extra : moz-landing-system : lando

js/src/jit/BaselineInspector.cpp

@@ -81,8 +81,8 @@ static bool GetCacheIRReceiverForNativeReadSlot(ICCacheIR_Monitored* stub,
   }
 
   if (reader.matchOp(CacheOp::GuardShape, objId)) {
-    receiver->shape =
-        stub->stubInfo()->getStubField<Shape*>(stub, reader.stubOffset());
+    receiver->setShape(
+        stub->stubInfo()->getStubField<Shape*>(stub, reader.stubOffset()));
     return reader.matchOpEither(CacheOp::LoadFixedSlotResult,
                                 CacheOp::LoadDynamicSlotResult);
   }
@@ -935,7 +935,8 @@ static bool MatchCacheIRReceiverGuard(CacheIRReader& reader, ICStub* stub,
 
   if (reader.matchOp(CacheOp::GuardShape, objId)) {
     // The first case.
-    receiver->shape = stubInfo->getStubField<Shape*>(stub, reader.stubOffset());
+    receiver->setShape(
+        stubInfo->getStubField<Shape*>(stub, reader.stubOffset()));
     return true;
   }
   return false;
@@ -1010,7 +1011,7 @@ static bool AddCacheIRGlobalGetter(ICCacheIR_Monitored* stub, bool innerized,
                             ->as<JSFunction>();
 
   ReceiverGuard receiver;
-  receiver.shape = globalLexicalShape;
+  receiver.setShape(globalLexicalShape);
   if (!AddReceiver(receiver, receivers)) {
     return false;
   }
@@ -1150,22 +1151,22 @@ static bool AddCacheIRGetPropFunction(
   if (reader.matchOp(CacheOp::CallScriptedGetterResult, objId) ||
       reader.matchOp(CacheOp::CallNativeGetterResult, objId)) {
     // This is an own property getter, the first case.
-    MOZ_ASSERT(receiver.shape);
-    MOZ_ASSERT(!receiver.group);
+    MOZ_ASSERT(receiver.getShape());
+    MOZ_ASSERT(!receiver.getGroup());
 
     size_t offset = reader.stubOffset();
     JSFunction* getter = &stub->stubInfo()
                               ->getStubField<JSObject*>(stub, offset)
                               ->as<JSFunction>();
 
-    if (*commonGetter &&
-        (!*isOwnProperty || *globalShape || *holderShape != receiver.shape)) {
+    if (*commonGetter && (!*isOwnProperty || *globalShape ||
+                          *holderShape != receiver.getShape())) {
       return false;
     }
 
     MOZ_ASSERT_IF(*commonGetter, *commonGetter == getter);
     *holder = nullptr;
-    *holderShape = receiver.shape;
+    *holderShape = receiver.getShape();
     *commonGetter = getter;
     *isOwnProperty = true;
     return true;
@@ -1196,9 +1197,9 @@ static bool AddCacheIRGetPropFunction(
                             ->as<JSFunction>();
 
   Shape* thisGlobalShape = nullptr;
-  if (getter->isNative() && receiver.shape &&
-      (receiver.shape->getObjectClass()->flags & JSCLASS_IS_GLOBAL)) {
-    thisGlobalShape = receiver.shape;
+  if (getter->isNative() && receiver.getShape() &&
+      (receiver.getShape()->getObjectClass()->flags & JSCLASS_IS_GLOBAL)) {
+    thisGlobalShape = receiver.getShape();
   }
 
   if (*commonGetter && (*isOwnProperty || *globalShape != thisGlobalShape ||
@@ -1403,21 +1404,22 @@ static bool AddCacheIRSetPropFunction(
   if (reader.matchOp(CacheOp::CallScriptedSetter, objId) ||
       reader.matchOp(CacheOp::CallNativeSetter, objId)) {
     // This is an own property setter, the first case.
-    MOZ_ASSERT(receiver.shape);
-    MOZ_ASSERT(!receiver.group);
+    MOZ_ASSERT(receiver.getShape());
+    MOZ_ASSERT(!receiver.getGroup());
 
     size_t offset = reader.stubOffset();
     JSFunction* setter = &stub->stubInfo()
                               ->getStubField<JSObject*>(stub, offset)
                               ->as<JSFunction>();
 
-    if (*commonSetter && (!*isOwnProperty || *holderShape != receiver.shape)) {
+    if (*commonSetter &&
+        (!*isOwnProperty || *holderShape != receiver.getShape())) {
       return false;
     }
 
     MOZ_ASSERT_IF(*commonSetter, *commonSetter == setter);
     *holder = nullptr;
-    *holderShape = receiver.shape;
+    *holderShape = receiver.getShape();
     *commonSetter = setter;
     *isOwnProperty = true;
     return true;

js/src/jit/CodeGenerator.cpp

@@ -3940,12 +3940,12 @@ void CodeGenerator::visitStoreSlotV(LStoreSlotV* lir) {
 
 static void GuardReceiver(MacroAssembler& masm, const ReceiverGuard& guard,
                           Register obj, Register scratch, Label* miss) {
-  if (guard.group) {
-    masm.branchTestObjGroup(Assembler::NotEqual, obj, guard.group, scratch, obj,
-                            miss);
+  if (guard.getGroup()) {
+    masm.branchTestObjGroup(Assembler::NotEqual, obj, guard.getGroup(), scratch,
+                            obj, miss);
   } else {
-    masm.branchTestObjShape(Assembler::NotEqual, obj, guard.shape, scratch, obj,
-                            miss);
+    masm.branchTestObjShape(Assembler::NotEqual, obj, guard.getShape(), scratch,
+                            obj, miss);
   }
 }
 
@@ -3963,7 +3963,7 @@ void CodeGenerator::emitGetPropertyPolymorphic(
     masm.comment("GuardReceiver");
     GuardReceiver(masm, receiver, obj, scratch, &next);
 
-    if (receiver.shape) {
+    if (receiver.getShape()) {
       masm.comment("loadTypedOrValue");
       Register target = obj;
 
@@ -4031,7 +4031,7 @@ void CodeGenerator::emitSetPropertyPolymorphic(
     Label next;
     GuardReceiver(masm, receiver, obj, scratch, &next);
 
-    if (receiver.shape) {
+    if (receiver.getShape()) {
       Register target = obj;
 
       Shape* shape = mir->shape(i);

js/src/jit/IonBuilder.cpp

@@ -11656,7 +11656,7 @@ bool IonBuilder::canInlinePropertyOpShapes(
     // dictionary mode. We cannot be sure that the shape is still a
     // lastProperty, and calling Shape::search() on dictionary mode
     // shapes that aren't lastProperty is invalid.
-    if (receivers[i].shape && receivers[i].shape->inDictionary()) {
+    if (receivers[i].getShape() && receivers[i].getShape()->inDictionary()) {
       trackOptimizationOutcome(TrackedOutcome::InDictionaryMode);
       return false;
     }
@@ -11669,11 +11669,11 @@ static Shape* PropertyShapesHaveSameSlot(
     const BaselineInspector::ReceiverVector& receivers, jsid id) {
   Shape* firstShape = nullptr;
   for (size_t i = 0; i < receivers.length(); i++) {
-    if (receivers[i].group) {
+    if (receivers[i].getGroup()) {
       return nullptr;
     }
 
-    Shape* shape = receivers[i].shape->searchLinear(id);
+    Shape* shape = receivers[i].getShape()->searchLinear(id);
     MOZ_ASSERT(shape);
 
     if (i == 0) {
@@ -11709,13 +11709,13 @@ AbortReasonOr<Ok> IonBuilder::getPropTryInlineAccess(bool* emitted,
   }
 
   if (receivers.length() == 1) {
-    if (!receivers[0].group) {
+    if (!receivers[0].getGroup()) {
       // Monomorphic load from a native object.
       spew("Inlining monomorphic native GETPROP");
 
-      obj = addShapeGuard(obj, receivers[0].shape, Bailout_ShapeGuard);
+      obj = addShapeGuard(obj, receivers[0].getShape(), Bailout_ShapeGuard);
 
-      Shape* shape = receivers[0].shape->searchLinear(NameToId(name));
+      Shape* shape = receivers[0].getShape()->searchLinear(NameToId(name));
       MOZ_ASSERT(shape);
 
       MOZ_TRY(loadSlot(obj, shape, rvalType, barrier, types));
@@ -11752,8 +11752,8 @@ AbortReasonOr<Ok> IonBuilder::getPropTryInlineAccess(bool* emitted,
 
   for (size_t i = 0; i < receivers.length(); i++) {
     Shape* propShape = nullptr;
-    if (receivers[i].shape) {
-      propShape = receivers[i].shape->searchLinear(NameToId(name));
+    if (receivers[i].getShape()) {
+      propShape = receivers[i].getShape()->searchLinear(NameToId(name));
       MOZ_ASSERT(propShape);
     }
     if (!load->addReceiver(receivers[i], propShape)) {
@@ -12407,13 +12407,13 @@ AbortReasonOr<Ok> IonBuilder::setPropTryInlineAccess(
   }
 
   if (receivers.length() == 1) {
-    if (!receivers[0].group) {
+    if (!receivers[0].getGroup()) {
       // Monomorphic store to a native object.
       spew("Inlining monomorphic native SETPROP");
 
-      obj = addShapeGuard(obj, receivers[0].shape, Bailout_ShapeGuard);
+      obj = addShapeGuard(obj, receivers[0].getShape(), Bailout_ShapeGuard);
 
-      Shape* shape = receivers[0].shape->searchLinear(NameToId(name));
+      Shape* shape = receivers[0].getShape()->searchLinear(NameToId(name));
       MOZ_ASSERT(shape);
 
       if (needsPostBarrier(value)) {
@@ -12464,8 +12464,8 @@ AbortReasonOr<Ok> IonBuilder::setPropTryInlineAccess(
 
   for (size_t i = 0; i < receivers.length(); i++) {
     Shape* propShape = nullptr;
-    if (receivers[i].shape) {
-      propShape = receivers[i].shape->searchLinear(NameToId(name));
+    if (receivers[i].getShape()) {
+      propShape = receivers[i].getShape()->searchLinear(NameToId(name));
       MOZ_ASSERT(propShape);
     }
     if (!ins->addReceiver(receivers[i], propShape)) {
@@ -13515,9 +13515,9 @@ MInstruction* IonBuilder::addGroupGuard(MDefinition* obj, ObjectGroup* group,
 MInstruction* IonBuilder::addGuardReceiverPolymorphic(
     MDefinition* obj, const BaselineInspector::ReceiverVector& receivers) {
   if (receivers.length() == 1) {
-    if (!receivers[0].group) {
+    if (!receivers[0].getGroup()) {
       // Monomorphic guard on a native object.
-      return addShapeGuard(obj, receivers[0].shape, Bailout_ShapeGuard);
+      return addShapeGuard(obj, receivers[0].getShape(), Bailout_ShapeGuard);
     }
   }
 

js/src/jit/MIR.h

@@ -1002,7 +1002,7 @@ class MRootList : public TempObject {
     return append(static_cast<T>(ptr));
   }
   MOZ_MUST_USE bool append(const ReceiverGuard& guard) {
-    return append(guard.group) && append(guard.shape);
+    return append(guard.getGroup()) && append(guard.getShape());
   }
 };
 

js/src/vm/ReceiverGuard-inl.h

@@ -15,27 +15,28 @@
 namespace js {
 
 MOZ_ALWAYS_INLINE
-ReceiverGuard::ReceiverGuard(JSObject* obj) : group(nullptr), shape(nullptr) {
-  if (!obj->isNative()) {
-    if (obj->is<TypedObject>()) {
-      group = obj->group();
-      return;
-    }
+ReceiverGuard::ReceiverGuard(JSObject* obj) : group_(nullptr), shape_(nullptr) {
+  if (obj->isNative() || IsProxy(obj)) {
+    shape_ = obj->shape();
+    return;
   }
-  shape = obj->as<JSObject>().shape();
+  MOZ_ASSERT(obj->is<TypedObject>());
+  group_ = obj->group();
 }
 
 MOZ_ALWAYS_INLINE
 ReceiverGuard::ReceiverGuard(ObjectGroup* group, Shape* shape)
-    : group(group), shape(shape) {
-  if (group) {
-    const Class* clasp = group->clasp();
+    : group_(group), shape_(shape) {
+  if (group_) {
+    const Class* clasp = group_->clasp();
     if (IsTypedObjectClass(clasp)) {
-      this->shape = nullptr;
+      this->shape_ = nullptr;
     } else {
-      this->group = nullptr;
+      this->group_ = nullptr;
     }
   }
+  // Only one of group_ or shape_ may be active at a time.
+  MOZ_ASSERT_IF(group_ || shape_, !!group_ != !!shape_);
 }
 
 }  // namespace js

js/src/vm/ReceiverGuard.h

@@ -11,6 +11,8 @@
 
 namespace js {
 
+// [SMDOC] Receiver Guard
+//
 // A ReceiverGuard encapsulates the information about an object that needs to
 // be tested to determine if it has the same 'structure' as another object.
 // The guard includes the shape and/or group of the object, and which of these
@@ -29,19 +31,21 @@ namespace js {
 //   All typed objects with the same group have the same class, prototype, and
 //   own properties.
 //
-// UnboxedPlainObject: The structure of an unboxed plain object is determined
-//   by its group and its expando object's shape, if there is one. All unboxed
-//   plain objects with the same group and expando shape have the same
-//   properties except those stored in the expando's dense elements.
-
+// In all cases, a ReceiverGuard has *either* a shape or a group active, and
+// never both.
 class HeapReceiverGuard;
 
 class ReceiverGuard {
- public:
-  ObjectGroup* group;
-  Shape* shape;
+  ObjectGroup* group_;
+  Shape* shape_;
 
-  ReceiverGuard() : group(nullptr), shape(nullptr) {}
+  void MOZ_ALWAYS_INLINE assertInvariants() {
+    // Only one of group_ or shape_ may be active at a time.
+    MOZ_ASSERT_IF(group_ || shape_, !!group_ != !!shape_);
+  }
+
+ public:
+  ReceiverGuard() : group_(nullptr), shape_(nullptr) {}
 
   inline MOZ_IMPLICIT ReceiverGuard(const HeapReceiverGuard& guard);
 
@@ -49,7 +53,7 @@ class ReceiverGuard {
   MOZ_ALWAYS_INLINE ReceiverGuard(ObjectGroup* group, Shape* shape);
 
   bool operator==(const ReceiverGuard& other) const {
-    return group == other.group && shape == other.shape;
+    return group_ == other.group_ && shape_ == other.shape_;
   }
 
   bool operator!=(const ReceiverGuard& other) const {
@@ -57,8 +61,21 @@ class ReceiverGuard {
   }
 
   uintptr_t hash() const {
-    return (uintptr_t(group) >> 3) ^ (uintptr_t(shape) >> 3);
+    return (uintptr_t(group_) >> 3) ^ (uintptr_t(shape_) >> 3);
   }
+
+  void setShape(Shape* shape) {
+    shape_ = shape;
+    assertInvariants();
+  }
+
+  void setGroup(ObjectGroup* group) {
+    group_ = group;
+    assertInvariants();
+  }
+
+  Shape* getShape() const { return shape_; }
+  ObjectGroup* getGroup() const { return group_; }
 };
 
 // Heap storage for ReceiverGuards.
@@ -74,13 +91,15 @@ class HeapReceiverGuard {
 
  public:
   explicit HeapReceiverGuard(const ReceiverGuard& guard)
-      : group_(guard.group), shape_(guard.shape) {}
+      : group_(guard.getGroup()), shape_(guard.getShape()) {}
 
   void trace(JSTracer* trc);
 };
 
 inline ReceiverGuard::ReceiverGuard(const HeapReceiverGuard& guard)
-    : group(guard.group_), shape(guard.shape_) {}
+    : group_(guard.group_), shape_(guard.shape_) {
+  assertInvariants();
+}
 
 }  // namespace js