Bug 1662559 - Part 6: Stamp the private brand on instances. r=arai

This adds a hasPrivateBrand bit to the MemberInitializers struct in the
stencil. It is necessary to persist this bit across compilation units because
emitInitializeInstanceMembers can be called in direct eval code inside a
derived-class constructor.

Since .privateBrand is currently a `let` binding, this currently emits an
unnecessary CheckAliasedLexical instruction in the constructor for each class
that has nonstatic private methods. We'll eliminate these later on.

Depends on D108289

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

js/src/frontend/BytecodeEmitter.cpp

@@ -9250,7 +9250,9 @@ mozilla::Maybe<MemberInitializers> BytecodeEmitter::setupMemberInitializers(
   if (numFields + numPrivateMethods > MemberInitializers::MaxInitializers) {
     return Nothing();
   }
-  return Some(MemberInitializers(numFields + numPrivateMethods));
+  bool hasPrivateBrand = numPrivateMethods > 0;
+  return Some(
+      MemberInitializers(hasPrivateBrand, numFields + numPrivateMethods));
 }
 
 // Purpose of .fieldKeys:
@@ -9638,8 +9640,36 @@ bool BytecodeEmitter::emitInitializeInstanceMembers() {
   const MemberInitializers& memberInitializers =
       findMemberInitializersForCall();
   MOZ_ASSERT(memberInitializers.valid);
-  size_t numInitializers = memberInitializers.numMemberInitializers;
 
+  if (memberInitializers.hasPrivateBrand) {
+    // Stamp the class's private brand onto the instance.  We use a getter
+    // instead of a field to save a slot per object, but the getter is never
+    // called, so it doesn't matter what function we use.
+
+    // This is guaranteed to run after super(), so we don't need TDZ checks.
+    if (!emitGetName(TaggedParserAtomIndex::WellKnown::dotThis())) {
+      //            [stack] THIS
+      return false;
+    }
+    if (!emitGetName(TaggedParserAtomIndex::WellKnown::dotPrivateBrand())) {
+      //            [stack] THIS BRAND
+      return false;
+    }
+    if (!emitBuiltinObject(BuiltinObjectKind::FunctionPrototype)) {
+      //            [stack] THIS BRAND GETTER
+      return false;
+    }
+    if (!emit1(JSOp::InitHiddenElemGetter)) {
+      //            [stack] THIS
+      return false;
+    }
+    if (!emit1(JSOp::Pop)) {
+      //            [stack]
+      return false;
+    }
+  }
+
+  size_t numInitializers = memberInitializers.numMemberInitializers;
   if (numInitializers == 0) {
     return true;
   }

js/src/frontend/CompilationStencil.h

@@ -53,7 +53,7 @@ struct CompilationStencil;
 struct CompilationGCOutput;
 class ScriptStencilIterable;
 
-// ScopeContext hold information derivied from the scope and environment chains
+// ScopeContext holds information derived from the scope and environment chains
 // to try to avoid the parser needing to traverse VM structures directly.
 struct ScopeContext {
   // Class field initializer info if we are nested within a class constructor.

js/src/frontend/Parser.cpp

@@ -575,7 +575,8 @@ bool PerHandlerParser<ParseHandler>::noteDestructuredPositionalFormalParameter(
 
 template <class ParseHandler, typename Unit>
 bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
-    TaggedParserAtomIndex name, DeclarationKind kind, TokenPos pos) {
+    TaggedParserAtomIndex name, DeclarationKind kind, TokenPos pos,
+    ClosedOver isClosedOver) {
   // The asm.js validator does all its own symbol-table management so, as an
   // optimization, avoid doing any work here.
   if (pc_->useAsmOrInsideUseAsm()) {
@@ -609,7 +610,8 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
         return false;
       }
 
-      if (!pc_->varScope().addDeclaredName(pc_, p, name, kind, pos.begin)) {
+      if (!pc_->varScope().addDeclaredName(pc_, p, name, kind, pos.begin,
+                                           isClosedOver)) {
         return false;
       }
 
@@ -630,8 +632,8 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
         return false;
       }
 
-      if (!pc_->functionScope().addDeclaredName(pc_, p, name, kind,
-                                                pos.begin)) {
+      if (!pc_->functionScope().addDeclaredName(pc_, p, name, kind, pos.begin,
+                                                isClosedOver)) {
         return false;
       }
 
@@ -647,7 +649,8 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
         return false;
       }
 
-      if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin)) {
+      if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin,
+                                  isClosedOver)) {
         return false;
       }
 
@@ -672,7 +675,8 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
           return false;
         }
       } else {
-        if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin)) {
+        if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin,
+                                    isClosedOver)) {
           return false;
         }
       }
@@ -723,7 +727,8 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
         return false;
       }
 
-      if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin)) {
+      if (!scope->addDeclaredName(pc_, p, name, kind, pos.begin,
+                                  isClosedOver)) {
         return false;
       }
 
@@ -7749,9 +7754,10 @@ bool GeneralParser<ParseHandler, Unit>::finishClassConstructor(
 
   size_t numMemberInitializers = classInitializedMembers.privateMethods +
                                  classInitializedMembers.instanceFields;
-  if (numMemberInitializers) {
+  bool hasPrivateBrand = classInitializedMembers.hasPrivateBrand();
+  if (hasPrivateBrand || numMemberInitializers > 0) {
     // Now that we have full set of initializers, update the constructor.
-    MemberInitializers initializers(numMemberInitializers);
+    MemberInitializers initializers(hasPrivateBrand, numMemberInitializers);
     ctorbox->setMemberInitializers(initializers);
 
     // Field initialization need access to `this`.
@@ -7868,9 +7874,13 @@ GeneralParser<ParseHandler, Unit>::classDefinition(
       }
 
       if (classInitializedMembers.privateMethods > 0) {
+        // We declare `.privateBrand` as ClosedOver because the constructor
+        // always uses it, even a default constructor. We could equivalently
+        // `noteNameUsed` when parsing the constructor, except that at that
+        // time, we don't necessarily know if the class has a private brand.
         if (!noteDeclaredName(
                 TaggedParserAtomIndex::WellKnown::dotPrivateBrand(),
-                DeclarationKind::Let, namePos)) {
+                DeclarationKind::Let, namePos, ClosedOver::Yes)) {
           return null();
         }
       }

js/src/frontend/Parser.h

@@ -1265,6 +1265,8 @@ class MOZ_STACK_CLASS GeneralParser : public PerHandlerParser<ParseHandler> {
 
     // The number of instance class private methods.
     size_t privateMethods = 0;
+
+    bool hasPrivateBrand() const { return privateMethods > 0; }
   };
   [[nodiscard]] bool classMember(
       YieldHandling yieldHandling,
@@ -1437,7 +1439,7 @@ class MOZ_STACK_CLASS GeneralParser : public PerHandlerParser<ParseHandler> {
   bool matchOrInsertSemicolon(Modifier modifier = TokenStream::SlashIsRegExp);
 
   bool noteDeclaredName(TaggedParserAtomIndex name, DeclarationKind kind,
-                        TokenPos pos);
+                        TokenPos pos, ClosedOver isClosedOver = ClosedOver::No);
 
   bool noteDeclaredPrivateName(Node nameNode, TaggedParserAtomIndex name,
                                PropertyType propType, TokenPos pos);

js/src/frontend/Stencil.h

@@ -900,7 +900,7 @@ class ScriptStencilExtra {
 
   MemberInitializers memberInitializers() const {
     MOZ_ASSERT(useMemberInitializers());
-    return MemberInitializers(memberInitializers_);
+    return MemberInitializers::deserialize(memberInitializers_);
   }
 
 #if defined(DEBUG) || defined(JS_JITSPEW)

js/src/vm/JSScript.cpp

@@ -263,15 +263,14 @@ XDRResult BaseScript::XDRLazyScriptData(XDRState<mode>* xdr,
   RootedFunction func(cx);
 
   if (lazy->useMemberInitializers()) {
-    uint32_t numMemberInitializers;
+    uint32_t bits;
     if (mode == XDR_ENCODE) {
       MOZ_ASSERT(lazy->getMemberInitializers().valid);
-      numMemberInitializers =
-          lazy->getMemberInitializers().numMemberInitializers;
+      bits = lazy->getMemberInitializers().serialize();
     }
-    MOZ_TRY(xdr->codeUint32(&numMemberInitializers));
+    MOZ_TRY(xdr->codeUint32(&bits));
     if (mode == XDR_DECODE) {
-      lazy->setMemberInitializers(MemberInitializers(numMemberInitializers));
+      lazy->setMemberInitializers(MemberInitializers::deserialize(bits));
     }
   }
 
@@ -819,15 +818,14 @@ XDRResult js::PrivateScriptData::XDR(XDRState<mode>* xdr, HandleScript script,
 
   // Code the field initializer data.
   if (script->useMemberInitializers()) {
-    uint32_t numMemberInitializers;
+    uint32_t bits;
     if (mode == XDR_ENCODE) {
       MOZ_ASSERT(data->getMemberInitializers().valid);
-      numMemberInitializers =
-          data->getMemberInitializers().numMemberInitializers;
+      bits = data->getMemberInitializers().serialize();
     }
-    MOZ_TRY(xdr->codeUint32(&numMemberInitializers));
+    MOZ_TRY(xdr->codeUint32(&bits));
     if (mode == XDR_DECODE) {
-      data->setMemberInitializers(MemberInitializers(numMemberInitializers));
+      data->setMemberInitializers(MemberInitializers::deserialize(bits));
     }
   }
 

js/src/vm/SharedStencil.h

@@ -587,22 +587,29 @@ using SharedImmutableScriptDataTable =
                      SharedImmutableScriptData::Hasher, SystemAllocPolicy>;
 
 struct MemberInitializers {
-  static constexpr uint32_t MaxInitializers = INT32_MAX;
+  static constexpr size_t NumBits = 31;
+  static constexpr uint32_t MaxInitializers = BitMask(NumBits);
 
 #ifdef DEBUG
   bool valid = false;
 #endif
 
+  bool hasPrivateBrand : 1;
+
   // This struct will eventually have a vector of constant values for optimizing
   // field initializers.
-  uint32_t numMemberInitializers = 0;
+  uint32_t numMemberInitializers : NumBits;
 
-  explicit MemberInitializers(uint32_t numMemberInitializers)
+  MemberInitializers(bool hasPrivateBrand, uint32_t numMemberInitializers)
       :
 #ifdef DEBUG
         valid(true),
 #endif
+        hasPrivateBrand(hasPrivateBrand),
         numMemberInitializers(numMemberInitializers) {
+    MOZ_ASSERT(
+        this->numMemberInitializers == numMemberInitializers,
+        "numMemberInitializers should easily fit in the 31-bit bitfield");
   }
 
   static MemberInitializers Invalid() { return MemberInitializers(); }
@@ -611,14 +618,28 @@ struct MemberInitializers {
   // fields. This is used when we elide the trivial data but still need a valid
   // set to stop scope walking.
   static const MemberInitializers& Empty() {
-    static const MemberInitializers zeroInitializers(0);
+    static const MemberInitializers zeroInitializers(false, 0);
     return zeroInitializers;
   }
 
-  uint32_t serialize() const { return numMemberInitializers; }
+  uint32_t serialize() const {
+    return (hasPrivateBrand << NumBits) | numMemberInitializers;
+  }
+
+  static MemberInitializers deserialize(uint32_t bits) {
+    return MemberInitializers((bits & Bit(NumBits)) != 0,
+                              bits & BitMask(NumBits));
+  }
 
  private:
-  MemberInitializers() = default;
+  MemberInitializers()
+      :
+#ifdef DEBUG
+        valid(false),
+#endif
+        hasPrivateBrand(false),
+        numMemberInitializers(0) {
+  }
 };
 
 }  // namespace js