Separates ETW Stack Walking Events from Generic ETW Events

All ETW Events are controlled by v8_enable_system_instrumentation.
This flag is turned off when perfetto is enabled since traces events
flowing through TRACE_EVENT macros can't be intercepted by Recorder.

Since, stack walking Events don't use TRACE_EVENT it can be turned
back on, when using perfetto. Hence, creating a separate Build Flag
for emitting stack walking event until the recorder is ported.


Bug: v8:11043
Change-Id: I6cdb81400780e54fddf6d6e2476cad29c60483d2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3704465
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Suraj Sharma <surshar@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#81879}

BUILD.gn

@@ -40,9 +40,12 @@ declare_args() {
   # Sets -DV8_ENABLE_FUTURE.
   v8_enable_future = false
 
-  # Sets -DSYSTEM_INSTRUMENTATION. Enables OS-dependent event tracing
+  # Sets -DENABLE_SYSTEM_INSTRUMENTATION. Enables OS-dependent event tracing
   v8_enable_system_instrumentation = (is_win || is_mac) && !v8_use_perfetto
 
+  # Sets -DV8_ENABLE_ETW_STACK_WALKING. Enables ETW Stack Walking
+  v8_enable_etw_stack_walking = is_win
+
   # Sets the GUID for the ETW provider
   v8_etw_guid = ""
 
@@ -1017,6 +1020,9 @@ config("features") {
   if (v8_enable_system_instrumentation) {
     defines += [ "V8_ENABLE_SYSTEM_INSTRUMENTATION" ]
   }
+  if (v8_enable_etw_stack_walking) {
+    defines += [ "V8_ENABLE_ETW_STACK_WALKING" ]
+  }
   if (v8_etw_guid != "") {
     defines += [ "V8_ETW_GUID=\"$v8_etw_guid\"" ]
   }
@@ -5028,11 +5034,10 @@ v8_source_set("v8_base_without_compiler") {
 
   # Architecture independent but platform-specific sources
   if (is_win) {
-    if (v8_enable_system_instrumentation) {
+    if (v8_enable_etw_stack_walking) {
       sources += [
-        "src/diagnostics/system-jit-metadata-win.h",
-        "src/diagnostics/system-jit-win.cc",
-        "src/diagnostics/system-jit-win.h",
+        "src/diagnostics/etw-jit-win.cc",
+        "src/diagnostics/etw-jit-win.h",
       ]
     }
   }
@@ -5491,7 +5496,7 @@ v8_component("v8_libbase") {
       "ws2_32.lib",
     ]
 
-    if (v8_enable_system_instrumentation) {
+    if (v8_enable_etw_stack_walking) {
       libs += [ "advapi32.lib" ]  # Needed for TraceLoggingProvider.h
     }
 

src/DEPS

@@ -90,7 +90,10 @@ specific_include_rules = {
   "builtins-trace\.cc": [
     "+protos/perfetto",
   ],
-  "system-jit-win\.cc": [
+  "etw-jit-win\.cc": [
     "+src/libplatform/tracing/recorder.h",
   ],
+  "etw-jit-metadata-win\.h": [
+    "+src/libplatform/etw/etw-provider-win.h",
+  ]
 }

src/api/api.cc

@@ -162,8 +162,8 @@
 #endif  // V8_OS_WIN64
 #endif  // V8_OS_WIN
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#include "src/diagnostics/system-jit-win.h"
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+#include "src/diagnostics/etw-jit-win.h"
 #endif
 
 // Has to be the last include (doesn't have include guards):
@@ -2104,7 +2104,7 @@ MaybeLocal<Value> Script::Run(Local<Context> context,
   i::AggregatingHistogramTimerScope histogram_timer(
       i_isolate->counters()->compile_lazy());
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
   // In case ETW has been activated, tasks to log existing code are
   // created. But in case the task runner does not run those before
   // starting to execute code (as it happens in d8, that will run
@@ -2113,7 +2113,7 @@ MaybeLocal<Value> Script::Run(Local<Context> context,
   //
   // To avoid this, on running scripts check first if JIT code log is
   // pending and generate immediately.
-  if (i::FLAG_enable_system_instrumentation) {
+  if (i::FLAG_enable_etw_stack_walking) {
     i::ETWJITInterface::MaybeSetHandlerNow(i_isolate);
   }
 #endif

src/d8/d8.cc

@@ -4832,12 +4832,16 @@ bool Shell::SetOptions(int argc, char* argv[]) {
       DCHECK(options.fuzzy_module_file_extensions);
       options.fuzzy_module_file_extensions = false;
       argv[i] = nullptr;
+#if defined(V8_ENABLE_ETW_STACK_WALKING)
+    } else if (strcmp(argv[i], "--enable-etw-stack-walking") == 0) {
+      options.enable_etw_stack_walking = true;
+      // This needs to be manually triggered for JIT ETW events to work.
+      i::FLAG_enable_etw_stack_walking = true;
 #if defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
     } else if (strcmp(argv[i], "--enable-system-instrumentation") == 0) {
       options.enable_system_instrumentation = true;
       options.trace_enabled = true;
-      // This needs to be manually triggered for JIT ETW events to work.
-      i::FLAG_enable_system_instrumentation = true;
+#endif
 #if defined(V8_OS_WIN)
       // Guard this bc the flag has a lot of overhead and is not currently used
       // by macos
@@ -5537,7 +5541,7 @@ int Shell::Main(int argc, char* argv[]) {
   if (options.trace_enabled && !i::FLAG_verify_predictable) {
     tracing = std::make_unique<platform::tracing::TracingController>();
 
-    if (!options.enable_system_instrumentation) {
+    if (!options.enable_etw_stack_walking) {
       const char* trace_path =
           options.trace_path ? options.trace_path : "v8_trace.json";
       trace_file.open(trace_path);

src/d8/d8.h

@@ -464,6 +464,8 @@ class ShellOptions {
       "fuzzy-module-file-extensions", true};
   DisallowReassignment<bool> enable_system_instrumentation = {
       "enable-system-instrumentation", false};
+  DisallowReassignment<bool> enable_etw_stack_walking = {
+      "enable-etw-stack-walking", false};
   DisallowReassignment<const char*> web_snapshot_config = {
       "web-snapshot-config", nullptr};
   DisallowReassignment<const char*> web_snapshot_output = {

src/diagnostics/etw-jit-metadata-win.h

@@ -2,21 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_DIAGNOSTICS_SYSTEM_JIT_METADATA_WIN_H_
-#define V8_DIAGNOSTICS_SYSTEM_JIT_METADATA_WIN_H_
+#ifndef V8_DIAGNOSTICS_ETW_JIT_METADATA_WIN_H_
+#define V8_DIAGNOSTICS_ETW_JIT_METADATA_WIN_H_
 
-#include <Windows.h>
-#ifndef VOID
-#define VOID void
-#endif
-#include <TraceLoggingProvider.h>
-#include <evntprov.h>
-#include <evntrace.h>  // defines TRACE_LEVEL_* and EVENT_TRACE_TYPE_*
-
-#include <cstdint>
-#include <string>
-#include <unordered_set>
-#include <utility>
+#include "src/libplatform/etw/etw-provider-win.h"
 
 namespace v8 {
 namespace internal {
@@ -245,4 +234,4 @@ void LogEventData(const TraceLoggingHProvider provider,
 }  // namespace internal
 }  // namespace v8
 
-#endif  // V8_DIAGNOSTICS_SYSTEM_JIT_METADATA_WIN_H_
+#endif  // V8_DIAGNOSTICS_ETW_JIT_METADATA_WIN_H_

src/diagnostics/etw-jit-win.cc

@@ -1,8 +1,7 @@
 // Copyright 2010 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/system-jit-win.h"
+#include "src/diagnostics/etw-jit-win.h"
 
 #include <atomic>
 
@@ -15,27 +14,21 @@
 #include "src/base/lazy-instance.h"
 #include "src/base/logging.h"
 #include "src/base/platform/platform.h"
-#include "src/diagnostics/system-jit-metadata-win.h"
-#include "src/libplatform/tracing/recorder.h"
+#include "src/diagnostics/etw-jit-metadata-win.h"
 #include "src/objects/shared-function-info.h"
 #include "src/tasks/cancelable-task.h"
 #include "src/tasks/task-utils.h"
 
-#if !defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#error "This file is only compiled if v8_enable_system_instrumentation"
-#endif
-
-#if defined(__clang__)
-#pragma clang diagnostic ignored "-Wc++98-compat-extra-semi"
+#if !defined(V8_ENABLE_ETW_STACK_WALKING)
+#error "This file is only compiled if v8_enable_etw_stack_walking"
 #endif
 
 namespace v8 {
 namespace internal {
 namespace ETWJITInterface {
 
-TRACELOGGING_DECLARE_PROVIDER(g_v8Provider);
-
-TRACELOGGING_DEFINE_PROVIDER(g_v8Provider, "V8.js", (V8_ETW_GUID));
+V8_DECLARE_TRACELOGGING_PROVIDER(g_v8Provider);
+V8_DEFINE_TRACELOGGING_PROVIDER(g_v8Provider);
 
 std::atomic<bool> is_etw_enabled = false;
 

src/diagnostics/etw-jit-win.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_DIAGNOSTICS_SYSTEM_JIT_WIN_H_
-#define V8_DIAGNOSTICS_SYSTEM_JIT_WIN_H_
+#ifndef V8_DIAGNOSTICS_ETW_JIT_WIN_H_
+#define V8_DIAGNOSTICS_ETW_JIT_WIN_H_
 
 namespace v8 {
 
@@ -22,4 +22,4 @@ void MaybeSetHandlerNow(Isolate* isolate);
 }  // namespace internal
 }  // namespace v8
 
-#endif  // V8_DIAGNOSTICS_SYSTEM_JIT_WIN_H_
+#endif  // V8_DIAGNOSTICS_ETW_JIT_WIN_H_

src/execution/isolate.cc

@@ -131,8 +131,8 @@
 #include "src/wasm/wasm-objects.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#include "src/diagnostics/system-jit-win.h"
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+#include "src/diagnostics/etw-jit-win.h"
 #endif
 
 #if defined(V8_OS_WIN64)
@@ -3541,8 +3541,8 @@ void Isolate::Deinit() {
   FILE* logfile = v8_file_logger_->TearDownAndGetLogFile();
   if (logfile != nullptr) base::Fclose(logfile);
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (i::FLAG_enable_system_instrumentation) {
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+  if (i::FLAG_enable_etw_stack_walking) {
     ETWJITInterface::RemoveIsolate(this);
   }
 #endif  // defined(V8_OS_WIN)
@@ -4150,8 +4150,8 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
   wasm::GetWasmEngine()->AddIsolate(this);
 #endif  // V8_ENABLE_WEBASSEMBLY
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (i::FLAG_enable_system_instrumentation) {
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+  if (i::FLAG_enable_etw_stack_walking) {
     ETWJITInterface::AddIsolate(this);
   }
 #endif  // defined(V8_OS_WIN)

src/flags/flag-definitions.h

@@ -2188,13 +2188,12 @@ DEFINE_BOOL(interpreted_frames_native_stack, false,
             "profilers).")
 #endif
 
-DEFINE_BOOL(enable_system_instrumentation, false,
-            "Enable platform-specific profiling.")
+DEFINE_BOOL(enable_etw_stack_walking, false,
+            "Enable etw stack walking for windows")
 // Don't move code objects.
-DEFINE_NEG_IMPLICATION(enable_system_instrumentation, compact_code_space)
+DEFINE_NEG_IMPLICATION(enable_etw_stack_walking, compact_code_space)
 #ifndef V8_TARGET_ARCH_ARM
-DEFINE_IMPLICATION(enable_system_instrumentation,
-                   interpreted_frames_native_stack)
+DEFINE_IMPLICATION(enable_etw_stack_walking, interpreted_frames_native_stack)
 #endif
 
 //

src/init/v8.cc

@@ -31,8 +31,8 @@
 #include "src/wasm/wasm-engine.h"
 #endif  // V8_ENABLE_WEBASSEMBLY
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#include "src/diagnostics/system-jit-win.h"
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+#include "src/diagnostics/etw-jit-win.h"
 #endif
 
 namespace v8 {
@@ -97,8 +97,8 @@ void V8::InitializePlatform(v8::Platform* platform) {
   platform_ = platform;
   v8::base::SetPrintStackTrace(platform_->GetStackTracePrinter());
   v8::tracing::TracingCategoryObserver::SetUp();
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (FLAG_enable_system_instrumentation) {
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+  if (FLAG_enable_etw_stack_walking) {
     // TODO(sartang@microsoft.com): Move to platform specific diagnostics object
     v8::internal::ETWJITInterface::Register();
   }
@@ -150,8 +150,8 @@ void V8::Initialize() {
     // Profiling flags depend on logging.
     FLAG_log = FLAG_log || FLAG_perf_prof || FLAG_perf_basic_prof ||
                FLAG_ll_prof || FLAG_prof || FLAG_prof_cpp || FLAG_gdbjit;
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-    FLAG_log = FLAG_log || FLAG_enable_system_instrumentation;
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+    FLAG_log = FLAG_log || FLAG_enable_etw_stack_walking;
 #endif
   }
 
@@ -282,8 +282,8 @@ void V8::Dispose() {
 void V8::DisposePlatform() {
   AdvanceStartupState(V8StartupState::kPlatformDisposing);
   CHECK(platform_);
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (FLAG_enable_system_instrumentation) {
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+  if (FLAG_enable_etw_stack_walking) {
     v8::internal::ETWJITInterface::Unregister();
   }
 #endif

src/libplatform/etw/etw-provider-win.h

@@ -0,0 +1,37 @@
+// Copyright 2022 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 V8_LIBPLATFORM_ETW_ETW_PROVIDER_WIN_H_
+#define V8_LIBPLATFORM_ETW_ETW_PROVIDER_WIN_H_
+
+// This file defines all the ETW Provider functions.
+#include <windows.h>
+#ifndef VOID
+#define VOID void
+#endif
+#include <TraceLoggingProvider.h>
+#include <evntprov.h>
+#include <evntrace.h>  // defines TRACE_LEVEL_* and EVENT_TRACE_TYPE_*
+
+#include <cstdint>
+#include <string>
+#include <unordered_set>
+#include <utility>
+
+#if defined(__clang__)
+#pragma clang diagnostic ignored "-Wc++98-compat-extra-semi"
+#endif
+
+#ifndef V8_ETW_GUID
+#define V8_ETW_GUID \
+  0x57277741, 0x3638, 0x4A4B, 0xBD, 0xBA, 0x0A, 0xC6, 0xE4, 0x5D, 0xA5, 0x6C
+#endif  // V8_ETW_GUID
+
+#define V8_DECLARE_TRACELOGGING_PROVIDER(v8Provider) \
+  TRACELOGGING_DECLARE_PROVIDER(v8Provider);
+
+#define V8_DEFINE_TRACELOGGING_PROVIDER(v8Provider) \
+  TRACELOGGING_DEFINE_PROVIDER(v8Provider, "V8.js", (V8_ETW_GUID));
+
+#endif  // V8_LIBPLATFORM_ETW_ETW_PROVIDER_WIN_H_

src/libplatform/tracing/recorder-win.cc

@@ -4,22 +4,15 @@
 #ifndef V8_LIBPLATFORM_TRACING_RECORDER_WIN_H_
 #define V8_LIBPLATFORM_TRACING_RECORDER_WIN_H_
 
-#include <Windows.h>
-#include <TraceLoggingProvider.h>
-
+#include "src/libplatform/etw/etw-provider-win.h"
 #include "src/libplatform/tracing/recorder.h"
 
-#if defined(__clang__)
-#pragma clang diagnostic ignored "-Wc++98-compat-extra-semi"
-#endif
-
 namespace v8 {
 namespace platform {
 namespace tracing {
 
-TRACELOGGING_DECLARE_PROVIDER(g_v8LibProvider);
-
-TRACELOGGING_DEFINE_PROVIDER(g_v8LibProvider, "V8.js", (V8_ETW_GUID));
+V8_DECLARE_TRACELOGGING_PROVIDER(g_v8LibProvider);
+V8_DEFINE_TRACELOGGING_PROVIDER(g_v8LibProvider);
 
 Recorder::Recorder() { TraceLoggingRegister(g_v8LibProvider); }
 

src/libplatform/tracing/recorder.h

@@ -9,22 +9,15 @@
 
 #include "include/libplatform/v8-tracing.h"
 
-#if !defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#error "only include this file if V8_ENABLE_SYSTEM_INSTRUMENTATION"
-#endif
-
 #if V8_OS_DARWIN
 #include <os/signpost.h>
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wunguarded-availability"
 #endif
 
-#if V8_OS_WIN
-#ifndef V8_ETW_GUID
-#define V8_ETW_GUID \
-  0x57277741, 0x3638, 0x4A4B, 0xBD, 0xBA, 0x0A, 0xC6, 0xE4, 0x5D, 0xA5, 0x6C
-#endif
-#endif
+#if !defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+#error "only include this file if V8_ENABLE_SYSTEM_INSTRUMENTATION"
+#endif  // V8_ENABLE_SYSTEM_INSTRUMENTATION
 
 namespace v8 {
 namespace platform {

src/logging/log.cc

@@ -56,8 +56,8 @@
 #endif  // V8_ENABLE_WEBASSEMBLY
 
 #if V8_OS_WIN
-#if defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-#include "src/diagnostics/system-jit-win.h"
+#if defined(V8_ENABLE_ETW_STACK_WALKING)
+#include "src/diagnostics/etw-jit-win.h"
 #endif
 #endif  // V8_OS_WIN
 
@@ -2113,8 +2113,8 @@ bool V8FileLogger::SetUp(Isolate* isolate) {
   }
 #endif  // ENABLE_GDB_JIT_INTERFACE
 
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
-  if (i::FLAG_enable_system_instrumentation) {
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
+  if (i::FLAG_enable_etw_stack_walking) {
     etw_jit_logger_ =
         std::make_unique<JitLogger>(isolate, i::ETWJITInterface::EventHandler);
     AddLogEventListener(etw_jit_logger_.get());

src/logging/log.h

@@ -351,7 +351,7 @@ class V8FileLogger : public LogEventListener {
 #ifdef ENABLE_GDB_JIT_INTERFACE
   std::unique_ptr<JitLogger> gdb_jit_logger_;
 #endif
-#if defined(V8_OS_WIN) && defined(V8_ENABLE_SYSTEM_INSTRUMENTATION)
+#if defined(V8_OS_WIN) && defined(V8_ENABLE_ETW_STACK_WALKING)
   std::unique_ptr<JitLogger> etw_jit_logger_;
 #endif
   std::set<int> logged_source_code_;