[class] exclude brand symbols in JSReceiver::GetPrivateEntries

This patch excludes brand symbols from the result of
JSReceiver::GetPrivateEntries so that the brands do not show up
when the instances are inspected from the DevTools (e.g. via
`Runtime.getProperties()`).

To implement this, we use a bit in the Symbols to denote whether
it's a brand symbol. A brand symbol is also a private name
symbol so that we can just reuse the IC for accessing private
names and do not need to jump through extra ORs.

Design doc: https://docs.google.com/document/d/1N91LObhQexnB0eE7EvGe57HsvNMFX16CaWu-XCTnnmY/edit

Bug: v8:8671, v8:9839, v8:8330
Change-Id: I24346aeedce3602395289052d1e1350ae9390354
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1909757Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Cr-Commit-Position: refs/heads/master@{#64899}

src/diagnostics/objects-debug.cc

@@ -278,6 +278,7 @@ void Symbol::SymbolVerify(Isolate* isolate) {
   CHECK_GT(Hash(), 0);
   CHECK(description().IsUndefined(isolate) || description().IsString());
   CHECK_IMPLIES(IsPrivateName(), IsPrivate());
+  CHECK_IMPLIES(IsPrivateBrand(), IsPrivateName());
 }
 
 USE_TORQUE_VERIFIER(ByteArray)

src/interpreter/bytecode-generator.cc

@@ -2059,7 +2059,7 @@ void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr, Register name) {
     builder()
         ->LoadLiteral(class_name)
         .StoreAccumulatorInRegister(brand)
-        .CallRuntime(Runtime::kCreatePrivateNameSymbol, brand);
+        .CallRuntime(Runtime::kCreatePrivateBrandSymbol, brand);
     BuildVariableAssignment(expr->scope()->brand(), Token::INIT,
                             HoleCheckMode::kElided);
   }

src/objects/name-inl.h

@@ -28,6 +28,18 @@ BIT_FIELD_ACCESSORS(Symbol, flags, is_in_public_symbol_table,
 BIT_FIELD_ACCESSORS(Symbol, flags, is_interesting_symbol,
                     Symbol::IsInterestingSymbolBit)
 
+bool Symbol::is_private_brand() const {
+  bool value = Symbol::IsPrivateBrandBit::decode(flags());
+  DCHECK_IMPLIES(value, is_private());
+  return value;
+}
+
+void Symbol::set_is_private_brand() {
+  set_flags(Symbol::IsPrivateBit::update(flags(), true));
+  set_flags(Symbol::IsPrivateNameBit::update(flags(), true));
+  set_flags(Symbol::IsPrivateBrandBit::update(flags(), true));
+}
+
 bool Symbol::is_private_name() const {
   bool value = Symbol::IsPrivateNameBit::decode(flags());
   DCHECK_IMPLIES(value, is_private());
@@ -97,6 +109,13 @@ DEF_GETTER(Name, IsPrivateName, bool) {
   return is_private_name;
 }
 
+DEF_GETTER(Name, IsPrivateBrand, bool) {
+  bool is_private_brand =
+      this->IsSymbol(isolate) && Symbol::cast(*this).is_private_brand();
+  DCHECK_IMPLIES(is_private_brand, IsPrivateName());
+  return is_private_brand;
+}
+
 bool Name::AsArrayIndex(uint32_t* index) {
   return IsString() && String::cast(*this).AsArrayIndex(index);
 }

src/objects/name.h

@@ -50,6 +50,11 @@ class Name : public TorqueGeneratedName<Name, PrimitiveHeapObject> {
   inline bool IsPrivateName() const;
   inline bool IsPrivateName(Isolate* isolate) const;
 
+  // If the name is a private brand, it should behave like a private name
+  // symbol but is filtered out when generating list of private fields.
+  inline bool IsPrivateBrand() const;
+  inline bool IsPrivateBrand(Isolate* isolate) const;
+
   inline bool IsUniqueName() const;
   inline bool IsUniqueName(Isolate* isolate) const;
 
@@ -168,6 +173,14 @@ class Symbol : public TorqueGeneratedSymbol<Symbol, Name> {
   inline bool is_private_name() const;
   inline void set_is_private_name();
 
+  // [is_private_name]: Whether this is a brand symbol.  Brand symbols are
+  // private name symbols that are used for validating access to
+  // private methods and storing information about the private methods.
+  //
+  // This also sets the is_private bit.
+  inline bool is_private_brand() const;
+  inline void set_is_private_brand();
+
   // Dispatched behavior.
   DECL_PRINTER(Symbol)
   DECL_VERIFIER(Symbol)
@@ -178,7 +191,8 @@ class Symbol : public TorqueGeneratedSymbol<Symbol, Name> {
   V(IsWellKnownSymbolBit, bool, 1, _)     \
   V(IsInPublicSymbolTableBit, bool, 1, _) \
   V(IsInterestingSymbolBit, bool, 1, _)   \
-  V(IsPrivateNameBit, bool, 1, _)
+  V(IsPrivateNameBit, bool, 1, _)         \
+  V(IsPrivateBrandBit, bool, 1, _)
 
   DEFINE_BIT_FIELDS(FLAGS_BIT_FIELDS)
 #undef FLAGS_BIT_FIELDS

src/objects/objects.cc

@@ -8046,14 +8046,30 @@ MaybeHandle<FixedArray> JSReceiver::GetPrivateEntries(
                               GetKeysConversion::kConvertToString),
       MaybeHandle<FixedArray>());
 
+  // Calculate number of private entries to return in the FixedArray.
+  // TODO(v8:9839): take the number of private methods/accessors into account.
+  int private_brand_count = 0;
+  for (int i = 0; i < keys->length(); ++i) {
+    // Exclude the private brand symbols.
+    if (Symbol::cast(keys->get(i)).is_private_brand()) {
+      private_brand_count++;
+    }
+  }
+  int private_entries_count = keys->length() - private_brand_count;
+
   Handle<FixedArray> entries =
-      isolate->factory()->NewFixedArray(keys->length() * 2);
+      isolate->factory()->NewFixedArray(private_entries_count * 2);
   int length = 0;
 
   for (int i = 0; i < keys->length(); ++i) {
     Handle<Object> obj_key = handle(keys->get(i), isolate);
     Handle<Symbol> key(Symbol::cast(*obj_key), isolate);
     CHECK(key->is_private_name());
+    if (key->is_private_brand()) {
+      // TODO(v8:9839): get the private methods/accessors of the instance
+      // using the brand and add them to the entries.
+      continue;
+    }
     Handle<Object> value;
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(
         isolate, value, Object::GetProperty(isolate, receiver, key),

src/runtime/runtime-symbol.cc

@@ -26,6 +26,15 @@ RUNTIME_FUNCTION(Runtime_CreatePrivateSymbol) {
   return *symbol;
 }
 
+RUNTIME_FUNCTION(Runtime_CreatePrivateBrandSymbol) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
+  Handle<Symbol> symbol = isolate->factory()->NewPrivateNameSymbol(name);
+  symbol->set_is_private_brand();
+  return *symbol;
+}
+
 RUNTIME_FUNCTION(Runtime_CreatePrivateNameSymbol) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

src/runtime/runtime.cc

@@ -111,6 +111,7 @@ bool Runtime::NeedsExactContext(FunctionId id) {
     case Runtime::kCopyDataProperties:
     case Runtime::kCreateDataProperty:
     case Runtime::kCreatePrivateNameSymbol:
+    case Runtime::kCreatePrivateBrandSymbol:
     case Runtime::kLoadPrivateGetter:
     case Runtime::kLoadPrivateSetter:
     case Runtime::kReThrow:

src/runtime/runtime.h

@@ -435,6 +435,7 @@ namespace internal {
 
 #define FOR_EACH_INTRINSIC_SYMBOL(F, I)    \
   F(CreatePrivateNameSymbol, 1, 1)         \
+  F(CreatePrivateBrandSymbol, 1, 1)        \
   F(CreatePrivateSymbol, -1 /* <= 1 */, 1) \
   F(SymbolDescriptiveString, 1, 1)         \
   F(SymbolIsPrivate, 1, 1)

test/cctest/interpreter/bytecode_expectations/PrivateAccessorDeclaration.golden

@@ -24,7 +24,7 @@ bytecodes: [
                 B(PushContext), R(1),
                 B(LdaConstant), U8(2),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(3), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(6),
@@ -74,7 +74,7 @@ bytecodes: [
                 B(PushContext), R(1),
                 B(LdaConstant), U8(2),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(3), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(6),
@@ -123,7 +123,7 @@ bytecodes: [
                 B(PushContext), R(1),
                 B(LdaConstant), U8(2),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(3), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(6),
@@ -178,7 +178,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(2),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(7),
@@ -201,7 +201,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(8),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
   /*  118 E> */ B(CreateClosure), U8(9), U8(3), U8(2),
                 B(Star), R(3),
@@ -273,7 +273,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(6),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
   /*   77 E> */ B(CreateClosure), U8(7), U8(2), U8(2),
                 B(Star), R(3),
@@ -347,7 +347,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(6),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
   /*   80 E> */ B(CreateClosure), U8(7), U8(2), U8(2),
                 B(Star), R(3),

test/cctest/interpreter/bytecode_expectations/PrivateMethodDeclaration.golden

@@ -23,7 +23,7 @@ bytecodes: [
                 B(PushContext), R(1),
                 B(LdaConstant), U8(2),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(3), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(6),
@@ -71,7 +71,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(2),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
                 B(LdaTheHole),
                 B(Star), R(7),
@@ -90,7 +90,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(7),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
   /*   93 E> */ B(CreateClosure), U8(8), U8(2), U8(2),
                 B(Star), R(3),
@@ -155,7 +155,7 @@ bytecodes: [
                 B(PushContext), R(2),
                 B(LdaConstant), U8(6),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(4), U8(1),
                 B(StaCurrentContextSlot), U8(3),
   /*   77 E> */ B(CreateClosure), U8(7), U8(2), U8(2),
                 B(Star), R(3),

test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodDeclaration.golden

@@ -198,7 +198,7 @@ bytecodes: [
                 B(PushContext), R(1),
                 B(LdaConstant), U8(2),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kCreatePrivateNameSymbol), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kCreatePrivateBrandSymbol), R(3), U8(1),
                 B(StaCurrentContextSlot), U8(4),
                 B(LdaTheHole),
                 B(Star), R(6),

test/inspector/debugger/class-private-methods-expected.txt

+Test private class methods
+
+Running test: testScopesPaused
+[
+    [0] : {
+        name : #field
+        value : {
+            description : 2
+            type : number
+            value : 2
+        }
+    }
+]
+{
+    result : {
+        description : 3
+        type : number
+        value : 3
+    }
+}

test/inspector/debugger/class-private-methods.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-private-methods
+// TODO(v8:9839): return private methods and accessors
+
+let { session, contextGroup, Protocol } = InspectorTest.start(
+  "Test private class methods"
+);
+
+contextGroup.addScript(`
+function run() {
+  class A {
+    #field = 2;
+    #inc() { this.#field++; return this.#field; }
+    get #getter() { return this.#field; }
+    set #setter(val) { this.#field = val; }
+    fn () {
+      debugger;
+    }
+  };
+  const a = new A();
+  a.fn();
+}`);
+
+InspectorTest.runAsyncTestSuite([
+  async function testScopesPaused() {
+    Protocol.Debugger.enable();
+    Protocol.Runtime.evaluate({ expression: "run()" });
+
+    let {
+      params: { callFrames }
+    } = await Protocol.Debugger.oncePaused(); // inside fn()
+    const frame = callFrames[0];
+    let { result  } = await Protocol.Runtime.getProperties({
+      objectId: frame.this.objectId
+    });
+    InspectorTest.logMessage(result.privateProperties);
+    let { result: result2 } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#inc();',
+      callFrameId: callFrames[0].callFrameId
+    });
+    InspectorTest.logObject(result2);
+    Protocol.Debugger.resume();
+    Protocol.Debugger.disable();
+  }
+]);