Refactors JitLogger initialization for ETW and GDB.

Goal is to ensure that the JitLogger for ETW (on Windows) is more inline
with other CodeEventLoggers such as PerfJitLogger.

The new design ensures that initial Builtin and BytecodHandler events
are emitted to and received by the ETWJITInterface::EventHandler.

Bug: v8:11043
Change-Id: I5741053c387b9ac63a42de61c99f4ea4ae4bdb96
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3581769Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Jakob Linke <jgruber@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79949}

src/api/api.cc

@@ -50,7 +50,6 @@
 #endif  // V8_ENABLE_WEBASSEMBLY
 #include "src/debug/liveedit.h"
 #include "src/deoptimizer/deoptimizer.h"
-#include "src/diagnostics/gdb-jit.h"
 #include "src/execution/embedder-state.h"
 #include "src/execution/execution.h"
 #include "src/execution/frames-inl.h"
@@ -157,9 +156,6 @@
 #include "src/base/platform/wrappers.h"
 #include "src/diagnostics/unwinding-info-win64.h"
 #endif  // V8_OS_WIN64
-#if defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#include "src/diagnostics/system-jit-win.h"
-#endif
 #endif  // V8_OS_WIN
 
 // Has to be the last include (doesn't have include guards):
@@ -8655,18 +8651,6 @@ void Isolate::Initialize(Isolate* isolate,
     // Set up code event handlers. Needs to be after i::Snapshot::Initialize
     // because that is where we add the isolate to WasmEngine.
     auto code_event_handler = params.code_event_handler;
-#ifdef ENABLE_GDB_JIT_INTERFACE
-    if (code_event_handler == nullptr && i::FLAG_gdbjit) {
-      code_event_handler = i::GDBJITInterface::EventHandler;
-    }
-#endif  // ENABLE_GDB_JIT_INTERFACE
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-    if (code_event_handler == nullptr &&
-        i::FLAG_enable_system_instrumentation) {
-      code_event_handler = i::ETWJITInterface::EventHandler;
-    }
-#endif  // defined(V8_OS_WIN)
-
     if (code_event_handler) {
       isolate->SetJitCodeEventHandler(kJitCodeEventEnumExisting,
                                       code_event_handler);

src/execution/isolate.cc

@@ -4121,7 +4121,9 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
   setup_delegate_ = nullptr;
 
   Builtins::InitializeIsolateDataTables(this);
-  Builtins::EmitCodeCreateEvents(this);
+
+  // Extra steps in the logger after the heap has been set up.
+  logger_->LateSetup(this);
 
 #ifdef DEBUG
   // Verify that the current heap state (usually deserialized from the snapshot)

src/init/v8.cc

@@ -159,6 +159,10 @@ void V8::Initialize() {
     // Profiling flags depend on logging.
     FLAG_log |= FLAG_perf_prof || FLAG_perf_basic_prof || FLAG_ll_prof ||
                 FLAG_prof || FLAG_prof_cpp;
+    FLAG_log |= FLAG_gdbjit;
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+    FLAG_log |= FLAG_enable_system_instrumentation;
+#endif
   }
 
   FlagList::EnforceFlagImplications();

src/logging/log.cc

@@ -44,12 +44,22 @@
 #include "src/utils/memcopy.h"
 #include "src/utils/version.h"
 
+#ifdef ENABLE_GDB_JIT_INTERFACE
+#include "src/diagnostics/gdb-jit.h"
+#endif  // ENABLE_GDB_JIT_INTERFACE
+
 #if V8_ENABLE_WEBASSEMBLY
 #include "src/wasm/wasm-code-manager.h"
 #include "src/wasm/wasm-engine.h"
 #include "src/wasm/wasm-objects-inl.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
 
+#if V8_OS_WIN
+#if defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+#include "src/diagnostics/system-jit-win.h"
+#endif
+#endif  // V8_OS_WIN
+
 namespace v8 {
 namespace internal {
 
@@ -203,6 +213,7 @@ CodeEventLogger::~CodeEventLogger() = default;
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       Handle<AbstractCode> code,
                                       const char* comment) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(tag);
   name_buffer_->AppendBytes(comment);
   LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
@@ -212,6 +223,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
 void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       Handle<AbstractCode> code,
                                       Handle<Name> name) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(tag);
   name_buffer_->AppendName(*name);
   LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
@@ -222,6 +234,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       Handle<AbstractCode> code,
                                       Handle<SharedFunctionInfo> shared,
                                       Handle<Name> script_name) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(tag);
   name_buffer_->AppendBytes(ComputeMarker(*shared, *code));
   name_buffer_->AppendByte(' ');
@@ -234,6 +247,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       Handle<SharedFunctionInfo> shared,
                                       Handle<Name> script_name, int line,
                                       int column) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(tag);
   name_buffer_->AppendBytes(ComputeMarker(*shared, *code));
   name_buffer_->AppendBytes(shared->DebugNameCStr().get());
@@ -256,6 +270,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
                                       wasm::WasmName name,
                                       const char* source_url,
                                       int /*code_offset*/, int /*script_id*/) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(tag);
   DCHECK(!name.empty());
   name_buffer_->AppendBytes(name.begin(), name.length());
@@ -273,6 +288,7 @@ void CodeEventLogger::CodeCreateEvent(LogEventsAndTags tag,
 
 void CodeEventLogger::RegExpCodeCreateEvent(Handle<AbstractCode> code,
                                             Handle<String> source) {
+  DCHECK(is_listening_to_code_events());
   name_buffer_->Init(CodeEventListener::REG_EXP_TAG);
   name_buffer_->AppendString(*source);
   LogRecordedBuffer(code, MaybeHandle<SharedFunctionInfo>(),
@@ -1994,6 +2010,26 @@ bool Logger::SetUp(Isolate* isolate) {
       "--perf-basic-prof should be statically disabled on non-Linux platforms");
 #endif
 
+#ifdef ENABLE_GDB_JIT_INTERFACE
+  if (i::FLAG_gdbjit) {
+    auto code_event_handler = i::GDBJITInterface::EventHandler;
+    DCHECK_NOT_NULL(code_event_handler);
+    gdb_jit_logger_ = std::make_unique<JitLogger>(isolate, code_event_handler);
+    AddCodeEventListener(gdb_jit_logger_.get());
+    CHECK(isolate->code_event_dispatcher()->IsListeningToCodeEvents());
+  }
+#endif  // ENABLE_GDB_JIT_INTERFACE
+
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+  if (i::FLAG_enable_system_instrumentation) {
+    auto code_event_handler = i::ETWJITInterface::EventHandler;
+    DCHECK_NOT_NULL(code_event_handler);
+    etw_jit_logger_ = std::make_unique<JitLogger>(isolate, code_event_handler);
+    AddCodeEventListener(etw_jit_logger_.get());
+    CHECK(isolate->code_event_dispatcher()->IsListeningToCodeEvents());
+  }
+#endif  // defined(V8_OS_WIN)
+
   if (FLAG_ll_prof) {
     ll_logger_ =
         std::make_unique<LowLevelLogger>(isolate, log_file_name.str().c_str());
@@ -2012,6 +2048,14 @@ bool Logger::SetUp(Isolate* isolate) {
   return true;
 }
 
+void Logger::LateSetup(Isolate* isolate) {
+  if (!isolate->code_event_dispatcher()->IsListeningToCodeEvents()) return;
+  Builtins::EmitCodeCreateEvents(isolate);
+#if V8_ENABLE_WEBASSEMBLY
+  wasm::GetWasmEngine()->EnableCodeLogging(isolate);
+#endif
+}
+
 void Logger::SetCodeEventHandler(uint32_t options,
                                  JitCodeEventHandler event_handler) {
   if (jit_logger_) {

src/logging/log.h

@@ -123,6 +123,9 @@ class Logger : public CodeEventListener {
   // Acquires resources for logging if the right flags are set.
   bool SetUp(Isolate* isolate);
 
+  // Additional steps taken after the logger has been set up.
+  void LateSetup(Isolate* isolate);
+
   // Frees resources acquired in SetUp.
   // When a temporary file is used for the log, returns its stream descriptor,
   // leaving the file open.
@@ -344,6 +347,12 @@ class Logger : public CodeEventListener {
 #endif
   std::unique_ptr<LowLevelLogger> ll_logger_;
   std::unique_ptr<JitLogger> jit_logger_;
+#ifdef ENABLE_GDB_JIT_INTERFACE
+  std::unique_ptr<JitLogger> gdb_jit_logger_;
+#endif
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+  std::unique_ptr<JitLogger> etw_jit_logger_;
+#endif
   std::set<int> logged_source_code_;
   uint32_t next_source_info_id_ = 0;
 
@@ -429,6 +438,8 @@ class V8_EXPORT_PRIVATE CodeEventLogger : public CodeEventListener {
                                  const char* reason) override {}
   void WeakCodeClearEvent() override {}
 
+  bool is_listening_to_code_events() override { return true; }
+
  protected:
   Isolate* isolate_;