cppgc: Add naming infrastructure

Adds NameProvider to allow specifying names of objects. The
corresponding internal NameTrait is registered with the GCInfo object.

Use name infrastructure to provide a hint on encountering an unmarked
object in the marking verifier.

Bug: chromium:1056170
Change-Id: I95bb290660f5905500f861bd5cc85148a1b47184
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2454087
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70400}

BUILD.gn

@@ -4353,6 +4353,7 @@ v8_source_set("cppgc_base") {
     "include/cppgc/internal/compiler-specific.h",
     "include/cppgc/internal/finalizer-trait.h",
     "include/cppgc/internal/gc-info.h",
+    "include/cppgc/internal/name-trait.h",
     "include/cppgc/internal/persistent-node.h",
     "include/cppgc/internal/pointer-policies.h",
     "include/cppgc/internal/prefinalizer-handler.h",
@@ -4361,6 +4362,7 @@ v8_source_set("cppgc_base") {
     "include/cppgc/liveness-broker.h",
     "include/cppgc/macros.h",
     "include/cppgc/member.h",
+    "include/cppgc/name-provider.h",
     "include/cppgc/persistent.h",
     "include/cppgc/platform.h",
     "include/cppgc/prefinalizer.h",
@@ -4410,6 +4412,7 @@ v8_source_set("cppgc_base") {
     "src/heap/cppgc/marking-visitor.h",
     "src/heap/cppgc/marking-worklists.cc",
     "src/heap/cppgc/marking-worklists.h",
+    "src/heap/cppgc/name-trait.cc",
     "src/heap/cppgc/object-allocator.cc",
     "src/heap/cppgc/object-allocator.h",
     "src/heap/cppgc/object-start-bitmap.h",

include/cppgc/internal/gc-info.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include "cppgc/internal/finalizer-trait.h"
+#include "cppgc/internal/name-trait.h"
 #include "cppgc/trace-trait.h"
 #include "v8config.h"  // NOLINT(build/include_directory)
 
@@ -19,7 +20,8 @@ using GCInfoIndex = uint16_t;
 class V8_EXPORT RegisteredGCInfoIndex final {
  public:
   RegisteredGCInfoIndex(FinalizationCallback finalization_callback,
-                        TraceCallback trace_callback, bool has_v_table);
+                        TraceCallback trace_callback,
+                        NameCallback name_callback, bool has_v_table);
   GCInfoIndex GetIndex() const { return index_; }
 
  private:
@@ -34,7 +36,7 @@ struct GCInfoTrait {
     static_assert(sizeof(T), "T must be fully defined");
     static const RegisteredGCInfoIndex registered_index(
         FinalizerTrait<T>::kCallback, TraceTrait<T>::Trace,
-        std::is_polymorphic<T>::value);
+        NameTrait<T>::GetName, std::is_polymorphic<T>::value);
     return registered_index.GetIndex();
   }
 };

include/cppgc/internal/name-trait.h

@@ -0,0 +1,66 @@
+// Copyright 2020 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.
+
+#ifndef INCLUDE_CPPGC_INTERNAL_NAME_TRAIT_H_
+#define INCLUDE_CPPGC_INTERNAL_NAME_TRAIT_H_
+
+#include "cppgc/name-provider.h"
+#include "v8config.h"  // NOLINT(build/include_directory)
+
+namespace cppgc {
+namespace internal {
+
+struct HeapObjectName {
+  const char* value;
+  bool name_was_hidden;
+};
+
+class V8_EXPORT NameTraitBase {
+ protected:
+  static HeapObjectName GetNameFromTypeSignature(const char*);
+};
+
+// Trait that specifies how the garbage collector retrieves the name for a
+// given object.
+template <typename T>
+class NameTrait final : public NameTraitBase {
+ public:
+  static HeapObjectName GetName(const void* obj) {
+    return GetNameFor(static_cast<const T*>(obj));
+  }
+
+ private:
+  static HeapObjectName GetNameFor(const NameProvider* name_provider) {
+    return {name_provider->GetName(), false};
+  }
+
+  static HeapObjectName GetNameFor(...) {
+#if CPPGC_SUPPORTS_OBJECT_NAMES
+
+#if defined(V8_CC_GNU)
+#define PRETTY_FUNCTION_VALUE __PRETTY_FUNCTION__
+#elif defined(V8_CC_MSVC)
+#define PRETTY_FUNCTION_VALUE __FUNCSIG__
+#else
+#define PRETTY_FUNCTION_VALUE nullptr
+#endif
+
+    static const HeapObjectName leaky_name =
+        GetNameFromTypeSignature(PRETTY_FUNCTION_VALUE);
+    return leaky_name;
+
+#undef PRETTY_FUNCTION_VALUE
+
+#else   // !CPPGC_SUPPORTS_OBJECT_NAMES
+    return {NameProvider::kHiddenName, true};
+#endif  // !CPPGC_SUPPORTS_OBJECT_NAMES
+  }
+};
+
+using NameCallback = HeapObjectName (*)(const void*);
+
+}  // namespace internal
+}  // namespace cppgc
+
+#endif  // INCLUDE_CPPGC_INTERNAL_NAME_TRAIT_H_

include/cppgc/name-provider.h

@@ -0,0 +1,65 @@
+// Copyright 2020 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.
+
+#ifndef INCLUDE_CPPGC_NAME_PROVIDER_H_
+#define INCLUDE_CPPGC_NAME_PROVIDER_H_
+
+#include "v8config.h"  // NOLINT(build/include_directory)
+
+namespace cppgc {
+
+/**
+ * NameProvider allows for providing a human-readable name for garbage-collected
+ * objects.
+ *
+ * There's two cases of names to distinguish:
+ * a. Explicitly specified names via using NameProvider. Such names are always
+ *    preserved in the system.
+ * b. Internal names that Oilpan infers from a C++ type on the class hierarchy
+ *    of the object. This is not necessarily the type of the actually
+ *    instantiated object.
+ *
+ * Depending on the build configuration, Oilpan may hide names, i.e., represent
+ * them with kHiddenName, of case b. to avoid exposing internal details.
+ */
+class V8_EXPORT NameProvider {
+ public:
+  /**
+   * Name that is used when hiding internals.
+   */
+  static constexpr const char kHiddenName[] = "InternalNode";
+
+  /**
+   * Name that is used in case compiler support is missing for composing a name
+   * from C++ types.
+   */
+  static constexpr const char kNoNameDeducible[] = "<No name>";
+
+  /**
+   * Indicating whether internal names are hidden or not.
+   *
+   * @returns true if C++ names should be hidden and represented by kHiddenName.
+   */
+  static constexpr bool HideInternalNames() {
+#if CPPGC_SUPPORTS_OBJECT_NAMES
+    return false;
+#else   // !CPPGC_SUPPORTS_OBJECT_NAMES
+    return true;
+#endif  // !CPPGC_SUPPORTS_OBJECT_NAMES
+  }
+
+  virtual ~NameProvider() = default;
+
+  /**
+   * Specifies a name for the garbage-collected object. Such names will never
+   * be hidden, as they are explicitly specified by the user of this API.
+   *
+   * @returns a human readable name for the object.
+   */
+  virtual const char* GetName() const = 0;
+};
+
+}  // namespace cppgc
+
+#endif  // INCLUDE_CPPGC_NAME_PROVIDER_H_

src/heap/cppgc/gc-info-table.h

@@ -23,9 +23,8 @@ namespace internal {
 struct GCInfo final {
   FinalizationCallback finalize;
   TraceCallback trace;
+  NameCallback name;
   bool has_v_table;
-  // Keep sizeof(GCInfo) a power of 2.
-  size_t padding = 0;
 };
 
 class V8_EXPORT GCInfoTable final {

src/heap/cppgc/gc-info.cc

@@ -11,9 +11,10 @@ namespace internal {
 
 RegisteredGCInfoIndex::RegisteredGCInfoIndex(
     FinalizationCallback finalization_callback, TraceCallback trace_callback,
-    bool has_v_table)
+    NameCallback name_callback, bool has_v_table)
     : index_(GlobalGCInfoTable::GetMutable().RegisterNewGCInfo(
-          {finalization_callback, trace_callback, has_v_table})) {}
+          {finalization_callback, trace_callback, name_callback,
+           has_v_table})) {}
 
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/heap-object-header.cc

@@ -27,5 +27,10 @@ void HeapObjectHeader::Finalize() {
   }
 }
 
+HeapObjectName HeapObjectHeader::GetName() const {
+  const GCInfo& gc_info = GlobalGCInfoTable::GCInfoFromIndex(GetGCInfoIndex());
+  return gc_info.name(Payload());
+}
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/heap-object-header.h

@@ -11,6 +11,7 @@
 
 #include "include/cppgc/allocation.h"
 #include "include/cppgc/internal/gc-info.h"
+#include "include/cppgc/internal/name-trait.h"
 #include "src/base/atomic-utils.h"
 #include "src/base/bit-field.h"
 #include "src/base/logging.h"
@@ -93,6 +94,8 @@ class HeapObjectHeader {
   inline bool IsFinalizable() const;
   void Finalize();
 
+  V8_EXPORT_PRIVATE HeapObjectName GetName() const;
+
  private:
   enum class EncodedHalf : uint8_t { kLow, kHigh };
 

src/heap/cppgc/marking-verifier.cc

@@ -39,7 +39,15 @@ void MarkingVerifier::VerifyChild(const void* base_object_payload) {
   const HeapObjectHeader& child_header =
       HeapObjectHeader::FromPayload(base_object_payload);
 
-  CHECK(child_header.IsMarked());
+  if (!child_header.IsMarked()) {
+    FATAL(
+        "MarkingVerifier: Encountered unmarked object.\n"
+        "#\n"
+        "# Hint:\n"
+        "#   %s\n"
+        "#     \\-> %s",
+        parent_->GetName().value, child_header.GetName().value);
+  }
 }
 
 void MarkingVerifier::VisitConservatively(
@@ -59,6 +67,8 @@ bool MarkingVerifier::VisitHeapObjectHeader(HeapObjectHeader* header) {
 
   DCHECK(!header->IsFree());
 
+  parent_ = header;
+
   if (!header->IsInConstruction()) {
     GlobalGCInfoTable::GCInfoFromIndex(header->GetGCInfoIndex())
         .trace(this, header->Payload());

src/heap/cppgc/marking-verifier.h

@@ -37,6 +37,8 @@ class V8_EXPORT_PRIVATE MarkingVerifier final
 
   bool VisitHeapObjectHeader(HeapObjectHeader*);
 
+  HeapObjectHeader* parent_ = nullptr;
+
   std::unordered_set<const HeapObjectHeader*> in_construction_objects_heap_;
   std::unordered_set<const HeapObjectHeader*> in_construction_objects_stack_;
   std::unordered_set<const HeapObjectHeader*>* in_construction_objects_ =

src/heap/cppgc/name-trait.cc

@@ -0,0 +1,41 @@
+// Copyright 2020 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.
+
+#include "include/cppgc/internal/name-trait.h"
+
+#include <stdio.h>
+
+#include "src/base/logging.h"
+#include "src/base/macros.h"
+
+namespace cppgc {
+
+// static
+constexpr const char NameProvider::kHiddenName[];
+
+// static
+constexpr const char NameProvider::kNoNameDeducible[];
+
+namespace internal {
+
+// static
+HeapObjectName NameTraitBase::GetNameFromTypeSignature(const char* signature) {
+  // Parsing string of structure:
+  //    static HeapObjectName NameTrait<int>::GetNameFor(...) [T = int]
+  if (!signature) return {NameProvider::kNoNameDeducible, true};
+
+  const std::string raw(signature);
+  const auto start_pos = raw.rfind("T = ") + 4;
+  DCHECK_NE(std::string::npos, start_pos);
+  const auto len = raw.length() - start_pos - 1;
+  const std::string name = raw.substr(start_pos, len).c_str();
+  char* name_buffer = new char[name.length() + 1];
+  int written = snprintf(name_buffer, name.length() + 1, "%s", name.c_str());
+  DCHECK_EQ(static_cast<size_t>(written), name.length());
+  USE(written);
+  return {name_buffer, false};
+}
+
+}  // namespace internal
+}  // namespace cppgc

test/unittests/BUILD.gn

@@ -99,6 +99,7 @@ v8_source_set("cppgc_unittests_sources") {
     "heap/cppgc/marking-visitor-unittest.cc",
     "heap/cppgc/member-unittest.cc",
     "heap/cppgc/minor-gc-unittest.cc",
+    "heap/cppgc/name-trait-unittest.cc",
     "heap/cppgc/object-start-bitmap-unittest.cc",
     "heap/cppgc/page-memory-unittest.cc",
     "heap/cppgc/persistent-unittest.cc",

test/unittests/heap/cppgc/gc-info-unittest.cc

@@ -14,6 +14,12 @@
 namespace cppgc {
 namespace internal {
 
+namespace {
+
+constexpr GCInfo GetEmptyGCInfo() { return {nullptr, nullptr, nullptr, false}; }
+
+}  // namespace
+
 TEST(GCInfoTableTest, InitialEmpty) {
   v8::base::PageAllocator page_allocator;
   GCInfoTable table(&page_allocator);
@@ -23,7 +29,7 @@ TEST(GCInfoTableTest, InitialEmpty) {
 TEST(GCInfoTableTest, ResizeToMaxIndex) {
   v8::base::PageAllocator page_allocator;
   GCInfoTable table(&page_allocator);
-  GCInfo info = {nullptr, nullptr, false};
+  GCInfo info = GetEmptyGCInfo();
   for (GCInfoIndex i = GCInfoTable::kMinIndex; i < GCInfoTable::kMaxIndex;
        i++) {
     GCInfoIndex index = table.RegisterNewGCInfo(info);
@@ -34,7 +40,7 @@ TEST(GCInfoTableTest, ResizeToMaxIndex) {
 TEST(GCInfoTableDeathTest, MoreThanMaxIndexInfos) {
   v8::base::PageAllocator page_allocator;
   GCInfoTable table(&page_allocator);
-  GCInfo info = {nullptr, nullptr, false};
+  GCInfo info = GetEmptyGCInfo();
   // Create GCInfoTable::kMaxIndex entries.
   for (GCInfoIndex i = GCInfoTable::kMinIndex; i < GCInfoTable::kMaxIndex;
        i++) {
@@ -46,7 +52,7 @@ TEST(GCInfoTableDeathTest, MoreThanMaxIndexInfos) {
 TEST(GCInfoTableDeathTest, OldTableAreaIsReadOnly) {
   v8::base::PageAllocator page_allocator;
   GCInfoTable table(&page_allocator);
-  GCInfo info = {nullptr, nullptr, false};
+  GCInfo info = GetEmptyGCInfo();
   // Use up all slots until limit.
   GCInfoIndex limit = table.LimitForTesting();
   // Bail out if initial limit is already the maximum because of large committed
@@ -76,7 +82,7 @@ class ThreadRegisteringGCInfoObjects final : public v8::base::Thread {
         num_registrations_(num_registrations) {}
 
   void Run() final {
-    GCInfo info = {nullptr, nullptr, false};
+    GCInfo info = GetEmptyGCInfo();
     for (GCInfoIndex i = 0; i < num_registrations_; i++) {
       table_->RegisterNewGCInfo(info);
     }
@@ -101,7 +107,7 @@ TEST(GCInfoTableTest, MultiThreadedResizeToMaxIndex) {
 
   v8::base::PageAllocator page_allocator;
   GCInfoTable table(&page_allocator);
-  GCInfo info = {nullptr, nullptr, false};
+  GCInfo info = GetEmptyGCInfo();
   for (size_t i = 0; i < main_thread_initialized; i++) {
     table.RegisterNewGCInfo(info);
   }

test/unittests/heap/cppgc/name-trait-unittest.cc

@@ -0,0 +1,133 @@
+// Copyright 2020 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.
+
+#include "include/cppgc/internal/name-trait.h"
+
+#include "include/cppgc/allocation.h"
+#include "include/cppgc/garbage-collected.h"
+#include "src/base/build_config.h"
+#include "test/unittests/heap/cppgc/tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cppgc {
+namespace internal {
+
+namespace {
+
+struct NoName : public GarbageCollected<NoName> {
+  virtual void Trace(Visitor*) const {}
+};
+
+struct OtherNoName : public GarbageCollected<OtherNoName> {
+  virtual void Trace(Visitor*) const {}
+};
+
+class ClassWithName final : public GarbageCollected<OtherNoName>,
+                            public NameProvider {
+ public:
+  explicit ClassWithName(const char* name) : name_(name) {}
+  virtual void Trace(Visitor*) const {}
+  const char* GetName() const final { return name_; }
+
+ private:
+  const char* name_;
+};
+
+}  // namespace
+
+TEST(NameTraitTest, InternalNamesHiddenInOfficialBuild) {
+  // Use a runtime test instead of static_assert to allow local builds but block
+  // enabling the feature accidentally through the waterfall.
+  //
+  // Do not include such type information in official builds to
+  // (a) save binary size on string literals, and
+  // (b) avoid exposing internal types until it has been clarified whether
+  //     exposing internals in DevTools is fine.
+#if defined(OFFICIAL_BUILD)
+  EXPECT_TRUE(NameProvider::HideInternalNames());
+#endif
+}
+
+TEST(NameTraitTest, DefaultName) {
+  EXPECT_STREQ(NameProvider::HideInternalNames()
+                   ? "InternalNode"
+                   : "cppgc::internal::(anonymous namespace)::NoName",
+               NameTrait<NoName>::GetName(nullptr).value);
+  EXPECT_STREQ(NameProvider::HideInternalNames()
+                   ? "InternalNode"
+                   : "cppgc::internal::(anonymous namespace)::OtherNoName",
+               NameTrait<OtherNoName>::GetName(nullptr).value);
+}
+
+TEST(NameTraitTest, CustomName) {
+  ClassWithName with_name("CustomName");
+  const char* name = NameTrait<ClassWithName>::GetName(&with_name).value;
+  EXPECT_STREQ("CustomName", name);
+}
+
+namespace {
+
+class TraitTester : public NameTraitBase {
+ public:
+  // Expose type signature parser to allow testing various inputs.
+  using NameTraitBase::GetNameFromTypeSignature;
+};
+
+}  // namespace
+
+TEST(NameTraitTest, NoTypeAvailable) {
+  HeapObjectName name = TraitTester::GetNameFromTypeSignature(nullptr);
+  EXPECT_STREQ(NameProvider::kNoNameDeducible, name.value);
+  EXPECT_TRUE(name.name_was_hidden);
+}
+
+TEST(NameTraitTest, ParsingPrettyFunction) {
+  // Test assumes that __PRETTY_FUNCTION__ and friends return a string
+  // containing the the type as [T = <type>].
+  HeapObjectName name = TraitTester::GetNameFromTypeSignature(
+      "Some signature of a method [T = ClassNameInSignature]");
+  EXPECT_STREQ("ClassNameInSignature", name.value);
+  EXPECT_FALSE(name.name_was_hidden);
+  // While object names are generally leaky, the test needs to be cleaned up
+  // gracefully.
+  delete[] name.value;
+}
+
+class HeapObjectHeaderNameTest : public testing::TestWithHeap {};
+
+TEST_F(HeapObjectHeaderNameTest, LookupNameThroughGCInfo) {
+  auto* no_name = MakeGarbageCollected<NoName>(GetAllocationHandle());
+  auto no_name_tuple = HeapObjectHeader::FromPayload(no_name).GetName();
+  if (NameProvider::HideInternalNames()) {
+    EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
+    EXPECT_TRUE(no_name_tuple.name_was_hidden);
+  } else {
+    EXPECT_STREQ("cppgc::internal::(anonymous namespace)::NoName",
+                 no_name_tuple.value);
+    EXPECT_FALSE(no_name_tuple.name_was_hidden);
+  }
+
+  auto* other_no_name =
+      MakeGarbageCollected<OtherNoName>(GetAllocationHandle());
+  auto other_no_name_tuple =
+      HeapObjectHeader::FromPayload(other_no_name).GetName();
+  if (NameProvider::HideInternalNames()) {
+    EXPECT_STREQ(NameProvider::kHiddenName, no_name_tuple.value);
+    EXPECT_TRUE(no_name_tuple.name_was_hidden);
+  } else {
+    EXPECT_STREQ("cppgc::internal::(anonymous namespace)::OtherNoName",
+                 other_no_name_tuple.value);
+    EXPECT_FALSE(other_no_name_tuple.name_was_hidden);
+  }
+
+  auto* class_with_name =
+      MakeGarbageCollected<ClassWithName>(GetAllocationHandle(), "CustomName");
+  auto class_with_name_tuple =
+      HeapObjectHeader::FromPayload(class_with_name).GetName();
+  EXPECT_STREQ("CustomName", class_with_name_tuple.value);
+  EXPECT_FALSE(class_with_name_tuple.name_was_hidden);
+}
+
+}  // namespace internal
+}  // namespace cppgc