[turbofan] Don't allocate JSHeapBroker in the zone

This fixes a memory leak.

Bug: v8:9191, v8:7790
Change-Id: I0df49cd3a6791600638a67b4b7ad9687562e500b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1588426
Commit-Queue: Georg Neis <neis@chromium.org>
Auto-Submit: Georg Neis <neis@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61166}

src/compiler/js-heap-broker.h

@@ -776,7 +776,7 @@ struct FeedbackSource {
   };
 };
 
-class V8_EXPORT_PRIVATE JSHeapBroker : public NON_EXPORTED_BASE(ZoneObject) {
+class V8_EXPORT_PRIVATE JSHeapBroker {
  public:
   JSHeapBroker(Isolate* isolate, Zone* broker_zone);
 

src/compiler/pipeline.cc

@@ -113,6 +113,7 @@ class PipelineData {
         instruction_zone_(instruction_zone_scope_.zone()),
         codegen_zone_scope_(zone_stats_, ZONE_NAME),
         codegen_zone_(codegen_zone_scope_.zone()),
+        broker_(new JSHeapBroker(isolate_, info_->zone())),
         register_allocation_zone_scope_(zone_stats_, ZONE_NAME),
         register_allocation_zone_(register_allocation_zone_scope_.zone()),
         assembler_options_(AssemblerOptions::Default(isolate)) {
@@ -131,7 +132,6 @@ class PipelineData {
     javascript_ = new (graph_zone_) JSOperatorBuilder(graph_zone_);
     jsgraph_ = new (graph_zone_)
         JSGraph(isolate_, graph_, common_, javascript_, simplified_, machine_);
-    broker_ = new (info_->zone()) JSHeapBroker(isolate_, info_->zone());
     dependencies_ =
         new (info_->zone()) CompilationDependencies(broker_, info_->zone());
   }
@@ -226,7 +226,6 @@ class PipelineData {
     delete code_generator_;
     code_generator_ = nullptr;
     DeleteTyper();
-
     DeleteRegisterAllocationZone();
     DeleteInstructionZone();
     DeleteCodegenZone();
@@ -274,6 +273,11 @@ class PipelineData {
   }
 
   JSHeapBroker* broker() const { return broker_; }
+  std::unique_ptr<JSHeapBroker> ReleaseBroker() {
+    std::unique_ptr<JSHeapBroker> broker(broker_);
+    broker_ = nullptr;
+    return broker;
+  }
 
   Schedule* schedule() const { return schedule_; }
   void set_schedule(Schedule* schedule) {
@@ -362,6 +366,7 @@ class PipelineData {
     codegen_zone_scope_.Destroy();
     codegen_zone_ = nullptr;
     dependencies_ = nullptr;
+    delete broker_;
     broker_ = nullptr;
     frame_ = nullptr;
   }
@@ -419,7 +424,6 @@ class PipelineData {
   void InitializeCodeGenerator(Linkage* linkage,
                                std::unique_ptr<AssemblerBuffer> buffer) {
     DCHECK_NULL(code_generator_);
-
     code_generator_ = new CodeGenerator(
         codegen_zone(), frame(), linkage, sequence(), info(), isolate(),
         osr_helper_, start_source_position_, jump_optimization_info_,
@@ -2370,16 +2374,12 @@ MaybeHandle<Code> Pipeline::GenerateCodeForWasmHeapStub(
 // static
 MaybeHandle<Code> Pipeline::GenerateCodeForTesting(
     OptimizedCompilationInfo* info, Isolate* isolate,
-    JSHeapBroker** out_broker) {
+    std::unique_ptr<JSHeapBroker>* out_broker) {
   ZoneStats zone_stats(isolate->allocator());
   std::unique_ptr<PipelineStatistics> pipeline_statistics(
       CreatePipelineStatistics(Handle<Script>::null(), info, isolate,
                                &zone_stats));
   PipelineData data(&zone_stats, isolate, info, pipeline_statistics.get());
-  if (out_broker != nullptr) {
-    *out_broker = data.broker();
-  }
-
   PipelineImpl pipeline(&data);
 
   Linkage linkage(Linkage::ComputeIncoming(data.instruction_zone(), info));
@@ -2391,6 +2391,7 @@ MaybeHandle<Code> Pipeline::GenerateCodeForTesting(
   Handle<Code> code;
   if (pipeline.FinalizeCode(out_broker == nullptr).ToHandle(&code) &&
       pipeline.CommitDependencies(code)) {
+    if (out_broker != nullptr) *out_broker = data.ReleaseBroker();
     return code;
   }
   return MaybeHandle<Code>();

src/compiler/pipeline.h

@@ -79,12 +79,12 @@ class Pipeline : public AllStatic {
   // The following methods are for testing purposes only. Avoid production use.
   // ---------------------------------------------------------------------------
 
-  // Run the pipeline on JavaScript bytecode and generate code.
-  // If requested, hands out the heap broker, which is allocated
-  // in {info}'s zone.
+  // Run the pipeline on JavaScript bytecode and generate code.  If requested,
+  // hands out the heap broker on success, transferring its ownership to the
+  // caller.
   V8_EXPORT_PRIVATE static MaybeHandle<Code> GenerateCodeForTesting(
       OptimizedCompilationInfo* info, Isolate* isolate,
-      JSHeapBroker** out_broker = nullptr);
+      std::unique_ptr<JSHeapBroker>* out_broker = nullptr);
 
   // Run the pipeline on a machine graph and generate code. If {schedule} is
   // {nullptr}, then compute a new schedule for code generation.

test/cctest/cctest.cc

@@ -225,10 +225,9 @@ HandleAndZoneScope::HandleAndZoneScope()
 
 HandleAndZoneScope::~HandleAndZoneScope() = default;
 
-i::Handle<i::JSFunction> Optimize(i::Handle<i::JSFunction> function,
-                                  i::Zone* zone, i::Isolate* isolate,
-                                  uint32_t flags,
-                                  i::compiler::JSHeapBroker** out_broker) {
+i::Handle<i::JSFunction> Optimize(
+    i::Handle<i::JSFunction> function, i::Zone* zone, i::Isolate* isolate,
+    uint32_t flags, std::unique_ptr<i::compiler::JSHeapBroker>* out_broker) {
   i::Handle<i::SharedFunctionInfo> shared(function->shared(), isolate);
   i::IsCompiledScope is_compiled_scope(shared->is_compiled_scope());
   CHECK(is_compiled_scope.is_compiled() ||

test/cctest/cctest.h

@@ -500,11 +500,13 @@ static inline v8::Local<v8::Value> CompileRunWithOrigin(
 
 // Takes a JSFunction and runs it through the test version of the optimizing
 // pipeline, allocating the temporary compilation artifacts in a given Zone.
-// For possible {flags} values, look at OptimizedCompilationInfo::Flag.
-// If passed a non-null pointer for {broker}, outputs the JSHeapBroker to it.
+// For possible {flags} values, look at OptimizedCompilationInfo::Flag.  If
+// {out_broker} is not nullptr, returns the JSHeapBroker via that (transferring
+// ownership to the caller).
 i::Handle<i::JSFunction> Optimize(
     i::Handle<i::JSFunction> function, i::Zone* zone, i::Isolate* isolate,
-    uint32_t flags, i::compiler::JSHeapBroker** out_broker = nullptr);
+    uint32_t flags,
+    std::unique_ptr<i::compiler::JSHeapBroker>* out_broker = nullptr);
 
 static inline void ExpectString(const char* code, const char* expected) {
   v8::Local<v8::Value> result = CompileRun(code);

test/cctest/compiler/serializer-tester.cc

@@ -45,7 +45,7 @@ SerializerTester::SerializerTester(const char* source)
                    i::OptimizedCompilationInfo::kSplittingEnabled |
                    i::OptimizedCompilationInfo::kAnalyzeEnvironmentLiveness;
   Optimize(function, main_zone(), main_isolate(), flags, &broker_);
-  function_ = JSFunctionRef(broker_, function);
+  function_ = JSFunctionRef(broker(), function);
 }
 
 TEST(SerializeEmptyFunction) {

test/cctest/compiler/serializer-tester.h

@@ -27,13 +27,13 @@ class SerializerTester : public HandleAndZoneScope {
   explicit SerializerTester(const char* source);
 
   JSFunctionRef function() const { return function_.value(); }
-  JSHeapBroker* broker() const { return broker_; }
+  JSHeapBroker* broker() const { return broker_.get(); }
   Isolate* isolate() { return main_isolate(); }
 
  private:
   CanonicalHandleScope canonical_;
   base::Optional<JSFunctionRef> function_;
-  JSHeapBroker* broker_ = nullptr;
+  std::unique_ptr<JSHeapBroker> broker_;
 };
 }  // namespace compiler
 }  // namespace internal