Use a std::shared_ptr for CompilationStats

This fixes a flaky crash when running with --turbo-stats or
--turbo-stats-wasm.
With dynamic tiering, it can happen that a compilation job is started
shortly before the program/test/benchmark terminates and the main thread
goes through its teardown sequence. When such a late job finishes, it
still wants to report its statistics, which currently crashes due to
UAF if the CompilationStats object, which is owned by the main thread,
has already been deleted.

Change-Id: Ie25a97299fdf40ece8f286487063feadcfa2eea9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3645410
Auto-Submit: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80560}

src/compiler/pipeline-statistics.cc

@@ -47,9 +47,10 @@ void PipelineStatistics::CommonStats::End(
   timer_.Stop();
 }
 
-PipelineStatistics::PipelineStatistics(OptimizedCompilationInfo* info,
-                                       CompilationStatistics* compilation_stats,
-                                       ZoneStats* zone_stats)
+PipelineStatistics::PipelineStatistics(
+    OptimizedCompilationInfo* info,
+    std::shared_ptr<CompilationStatistics> compilation_stats,
+    ZoneStats* zone_stats)
     : outer_zone_(info->zone()),
       zone_stats_(zone_stats),
       compilation_stats_(compilation_stats),

src/compiler/pipeline-statistics.h

@@ -23,7 +23,8 @@ class PhaseScope;
 class PipelineStatistics : public Malloced {
  public:
   PipelineStatistics(OptimizedCompilationInfo* info,
-                     CompilationStatistics* turbo_stats, ZoneStats* zone_stats);
+                     std::shared_ptr<CompilationStatistics> turbo_stats,
+                     ZoneStats* zone_stats);
   ~PipelineStatistics();
   PipelineStatistics(const PipelineStatistics&) = delete;
   PipelineStatistics& operator=(const PipelineStatistics&) = delete;
@@ -67,7 +68,7 @@ class PipelineStatistics : public Malloced {
 
   Zone* outer_zone_;
   ZoneStats* zone_stats_;
-  CompilationStatistics* compilation_stats_;
+  std::shared_ptr<CompilationStatistics> compilation_stats_;
   CodeKind code_kind_;
   std::string function_name_;
 

src/execution/isolate.cc

@@ -4356,19 +4356,18 @@ void Isolate::DumpAndResetStats() {
       stack_access_count_map = nullptr;
     }
   }
-  if (turbo_statistics() != nullptr) {
+  if (turbo_statistics_ != nullptr) {
     DCHECK(FLAG_turbo_stats || FLAG_turbo_stats_nvp);
     StdoutStream os;
     if (FLAG_turbo_stats) {
-      AsPrintableStatistics ps = {*turbo_statistics(), false};
+      AsPrintableStatistics ps = {*turbo_statistics_, false};
       os << ps << std::endl;
     }
     if (FLAG_turbo_stats_nvp) {
-      AsPrintableStatistics ps = {*turbo_statistics(), true};
+      AsPrintableStatistics ps = {*turbo_statistics_, true};
       os << ps << std::endl;
     }
-    delete turbo_statistics_;
-    turbo_statistics_ = nullptr;
+    turbo_statistics_.reset();
   }
 #if V8_ENABLE_WEBASSEMBLY
   // TODO(7424): There is no public API for the {WasmEngine} yet. So for now we
@@ -4400,10 +4399,11 @@ void Isolate::AbortConcurrentOptimization(BlockingBehavior behavior) {
   }
 }
 
-CompilationStatistics* Isolate::GetTurboStatistics() {
-  if (turbo_statistics() == nullptr)
-    set_turbo_statistics(new CompilationStatistics());
-  return turbo_statistics();
+std::shared_ptr<CompilationStatistics> Isolate::GetTurboStatistics() {
+  if (turbo_statistics_ == nullptr) {
+    turbo_statistics_.reset(new CompilationStatistics());
+  }
+  return turbo_statistics_;
 }
 
 CodeTracer* Isolate::GetCodeTracer() {

src/execution/isolate.h

@@ -505,7 +505,6 @@ using DebugObjectCache = std::vector<Handle<HeapObject>>;
   V(AddressToIndexHashMap*, external_reference_map, nullptr)                  \
   V(HeapObjectToIndexHashMap*, root_index_map, nullptr)                       \
   V(MicrotaskQueue*, default_microtask_queue, nullptr)                        \
-  V(CompilationStatistics*, turbo_statistics, nullptr)                        \
   V(CodeTracer*, code_tracer, nullptr)                                        \
   V(PromiseRejectCallback, promise_reject_callback, nullptr)                  \
   V(const v8::StartupData*, snapshot_blob, nullptr)                           \
@@ -1540,7 +1539,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
     was_locker_ever_used_.store(true, std::memory_order_relaxed);
   }
 
-  CompilationStatistics* GetTurboStatistics();
+  std::shared_ptr<CompilationStatistics> GetTurboStatistics();
   CodeTracer* GetCodeTracer();
 
   void DumpAndResetStats();
@@ -2320,6 +2319,7 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   v8::Isolate::UseCounterCallback use_counter_callback_ = nullptr;
 
+  std::shared_ptr<CompilationStatistics> turbo_statistics_;
   std::shared_ptr<metrics::Recorder> metrics_recorder_;
   uintptr_t last_recorder_context_id_ = 0;
   std::unordered_map<

src/wasm/wasm-engine.cc

@@ -881,12 +881,13 @@ Handle<WasmModuleObject> WasmEngine::ImportNativeModule(
   return module_object;
 }
 
-CompilationStatistics* WasmEngine::GetOrCreateTurboStatistics() {
+std::shared_ptr<CompilationStatistics>
+WasmEngine::GetOrCreateTurboStatistics() {
   base::MutexGuard guard(&mutex_);
   if (compilation_stats_ == nullptr) {
     compilation_stats_.reset(new CompilationStatistics());
   }
-  return compilation_stats_.get();
+  return compilation_stats_;
 }
 
 void WasmEngine::DumpAndResetTurboStatistics() {

src/wasm/wasm-engine.h

@@ -222,8 +222,10 @@ class V8_EXPORT_PRIVATE WasmEngine {
 
   AccountingAllocator* allocator() { return &allocator_; }
 
-  // Compilation statistics for TurboFan compilations.
-  CompilationStatistics* GetOrCreateTurboStatistics();
+  // Compilation statistics for TurboFan compilations. Returns a shared_ptr
+  // so that background compilation jobs can hold on to it while the main thread
+  // shuts down.
+  std::shared_ptr<CompilationStatistics> GetOrCreateTurboStatistics();
 
   // Prints the gathered compilation statistics, then resets them.
   void DumpAndResetTurboStatistics();
@@ -409,7 +411,7 @@ class V8_EXPORT_PRIVATE WasmEngine {
   std::unordered_map<AsyncCompileJob*, std::unique_ptr<AsyncCompileJob>>
       async_compile_jobs_;
 
-  std::unique_ptr<CompilationStatistics> compilation_stats_;
+  std::shared_ptr<CompilationStatistics> compilation_stats_;
   std::unique_ptr<CodeTracer> code_tracer_;
 
   // Set of isolates which use this WasmEngine.