Make adding crash keys a platform API

The current integration of crash keys into v8 got the dependencies wrong: it introduced into v8 a dependency on components and base. This change will allow moving the implementation into "gin" (via Platform's abstraction), which is ok to depend on components and base, while providing the default noop implementation for the embedders that don't care to collect crash keys. Gin's side: https://chromium-review.googlesource.com/c/chromium/src/+/1690003.

Bug: v8:9323
Change-Id: I7b6e3e2cdc4b5f14f61ad20d2c362344d53896c6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1689834
Commit-Queue: Irina Yatsenko <irinayat@microsoft.com>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62579}

BUILD.gn

@@ -2152,7 +2152,6 @@ v8_source_set("v8_base_without_compiler") {
     "src/diagnostics/code-tracer.h",
     "src/diagnostics/compilation-statistics.cc",
     "src/diagnostics/compilation-statistics.h",
-    "src/diagnostics/crash-key.h",
     "src/diagnostics/disasm.h",
     "src/diagnostics/disassembler.cc",
     "src/diagnostics/disassembler.h",
@@ -3301,33 +3300,10 @@ v8_source_set("v8_base_without_compiler") {
   }
 }
 
-v8_source_set("v8_crash_keys") {
-  visibility = [ ":*" ]  # Only targets in this file can depend on this.
-
-  # TODO(irinayat): crash_key brings dependency on //base and memory allocators,
-  # which causes linker errors picking up memory shims on Android. Figure out
-  # how to make this work so android could also collect crash keys.
-  if (build_with_chromium && !is_android) {
-    deps = [
-      "//components/crash/core/common:crash_key",
-    ]
-    sources = [
-      "src/diagnostics/crash-key.cc",
-    ]
-  } else {
-    sources = [
-      "src/diagnostics/crash-key-noop.cc",
-    ]
-  }
-
-  configs = [ ":internal_config" ]
-}
-
 group("v8_base") {
   public_deps = [
     ":v8_base_without_compiler",
     ":v8_compiler",
-    ":v8_crash_keys",
   ]
 }
 
@@ -3809,7 +3785,6 @@ if (v8_use_snapshot && current_toolchain == v8_snapshot_toolchain) {
     visibility = [ ":*" ]  # Only targets in this file can depend on this.
 
     sources = [
-      "src/diagnostics/crash-key-noop.cc",
       "src/snapshot/embedded/embedded-file-writer.cc",
       "src/snapshot/embedded/embedded-file-writer.h",
       "src/snapshot/embedded/platform-embedded-file-writer-aix.cc",

include/v8-platform.h

@@ -439,6 +439,14 @@ class Platform {
    */
   virtual void DumpWithoutCrashing() {}
 
+  /**
+   * Lets the embedder to add crash keys.
+   */
+  virtual void AddCrashKey(int id, const char* name, uintptr_t value) {
+    // "noop" is a valid implementation if the embedder doesn't care to log
+    // additional data for crashes.
+  }
+
  protected:
   /**
    * Default implementation of current wall-clock time in milliseconds

src/diagnostics/DEPS

 include_rules = [
-  "+components/crash/core/common/crash_key.h",
   "+src/compiler/node.h",
 ]

src/diagnostics/crash-key-noop.cc

-// 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.
-
-// Noop implementation of crash keys to be used by targets that don't support
-// crashpad.
-
-#include "src/diagnostics/crash-key.h"
-
-namespace v8 {
-namespace internal {
-namespace crash {
-
-void AddCrashKey(int id, const char* name, uintptr_t value) {
-}
-
-}  // namespace crash
-}  // namespace internal
-}  // namespace v8

src/diagnostics/crash-key.cc

-// 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.
-
-#include "src/diagnostics/crash-key.h"
-#include "components/crash/core/common/crash_key.h"
-
-#include <atomic>
-#include <sstream>
-#include <string>
-
-namespace v8 {
-namespace internal {
-namespace crash {
-
-using CrashKeyInstance = crash_reporter::CrashKeyString<kKeySize>;
-static CrashKeyInstance crash_keys[] = {
-  {"v8-0", CrashKeyInstance::Tag::kArray},
-  {"v8-1", CrashKeyInstance::Tag::kArray},
-  {"v8-2", CrashKeyInstance::Tag::kArray},
-  {"v8-3", CrashKeyInstance::Tag::kArray},
-  {"v8-4", CrashKeyInstance::Tag::kArray},
-  {"v8-5", CrashKeyInstance::Tag::kArray},
-  {"v8-6", CrashKeyInstance::Tag::kArray},
-  {"v8-7", CrashKeyInstance::Tag::kArray},
-  {"v8-8", CrashKeyInstance::Tag::kArray},
-  {"v8-9", CrashKeyInstance::Tag::kArray},
-  {"v8-10", CrashKeyInstance::Tag::kArray},
-  {"v8-11", CrashKeyInstance::Tag::kArray},
-  {"v8-12", CrashKeyInstance::Tag::kArray},
-  {"v8-13", CrashKeyInstance::Tag::kArray},
-  {"v8-14", CrashKeyInstance::Tag::kArray},
-  {"v8-15", CrashKeyInstance::Tag::kArray},
-};
-
-void AddCrashKey(int id, const char* name, uintptr_t value) {
-  static std::atomic<int> last{-1};
-  const int current = ++last;
-
-  if (current > kMaxCrashKeysCount) {
-    return;
-  }
-
-  if (current == kMaxCrashKeysCount) {
-    static crash_reporter::CrashKeyString<1> over("v8-too-many-keys");
-    over.Set("1");
-    return;
-  }
-
-  auto& trace_key = crash_keys[current];
-
-  std::stringstream stream;
-  stream << name << " " << id << " 0x" << std::hex << value;
-  trace_key.Set(stream.str().substr(0, kKeySize));
-}
-
-}  // namespace crash
-}  // namespace internal
-}  // namespace v8

src/diagnostics/crash-key.h

-// 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.
-
-// Conflicts between v8/src/base and base prevent from including
-// components/crash/core/common/crash_key.h into most of v8's files, so have to
-// provide wrappers to localize the include to v8/src/base/crash-key.cc only.
-
-#ifndef V8_DIAGNOSTICS_CRASH_KEY_H_
-#define V8_DIAGNOSTICS_CRASH_KEY_H_
-
-#include <stdint.h>
-
-namespace v8 {
-namespace internal {
-namespace crash {
-
-// Crash keys must be statically allocated so we'll have a few slots for
-// pointer values and will log if we run out of space. The pointer value will
-// be combined with the given name and id. Names should be sufficiently short
-// to fit key_size limit.
-// The crash key in the dump will look similar to:
-//   {"v8-0", "isolate 0 0x21951a41d90"}
-// (we assume a pointer is being logged and we convert it to hex).
-constexpr int kKeySize = 64;
-constexpr int kMaxCrashKeysCount = 16;
-
-void AddCrashKey(int id, const char* name, uintptr_t value);
-
-}  // namespace crash
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_DIAGNOSTICS_CRASH_KEY_H_

src/execution/isolate.cc

@@ -32,7 +32,6 @@
 #include "src/debug/debug.h"
 #include "src/deoptimizer/deoptimizer.h"
 #include "src/diagnostics/compilation-statistics.h"
-#include "src/diagnostics/crash-key.h"
 #include "src/execution/frames-inl.h"
 #include "src/execution/isolate-inl.h"
 #include "src/execution/messages.h"
@@ -3302,15 +3301,17 @@ bool Isolate::InitWithSnapshot(ReadOnlyDeserializer* read_only_deserializer,
 }
 
 static void AddCrashKeysForIsolateAndHeapPointers(Isolate* isolate) {
+  v8::Platform* platform = V8::GetCurrentPlatform();
+
   const int id = isolate->id();
-  crash::AddCrashKey(id, "isolate", reinterpret_cast<uintptr_t>(isolate));
+  platform->AddCrashKey(id, "isolate", reinterpret_cast<uintptr_t>(isolate));
 
   auto heap = isolate->heap();
-  crash::AddCrashKey(id, "ro_space",
+  platform->AddCrashKey(id, "ro_space",
     reinterpret_cast<uintptr_t>(heap->read_only_space()->first_page()));
-  crash::AddCrashKey(id, "map_space",
+  platform->AddCrashKey(id, "map_space",
     reinterpret_cast<uintptr_t>(heap->map_space()->first_page()));
-  crash::AddCrashKey(id, "code_space",
+  platform->AddCrashKey(id, "code_space",
     reinterpret_cast<uintptr_t>(heap->code_space()->first_page()));
 }