[cpu-profiler] Delete deopt_frames array if CodeMap entry not found

If code is deoptimized while CPU profiling, a deoptimization event
record is sent to the profiler. But if the profiler could not find
the associated CodeMap entry in CodeDeoptEventRecord::UpdateCodeMap
it would simply return without freeing the deopt_frames array.
This change frees the deopt_frames array no matter what in
CodeDeoptEventRecord::UpdateCodeMap, eliminating a storage leak.

Bug: v8:10861
Change-Id: I4e68566bb91dff13b38e255ddfed24b85b7a1d57
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2386332Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69901}

src/profiler/cpu-profiler-inl.h

@@ -34,10 +34,11 @@ void CodeDisableOptEventRecord::UpdateCodeMap(CodeMap* code_map) {
 
 void CodeDeoptEventRecord::UpdateCodeMap(CodeMap* code_map) {
   CodeEntry* entry = code_map->FindEntry(instruction_start);
-  if (entry == nullptr) return;
-  std::vector<CpuProfileDeoptFrame> frames_vector(
-      deopt_frames, deopt_frames + deopt_frame_count);
-  entry->set_deopt_info(deopt_reason, deopt_id, std::move(frames_vector));
+  if (entry != nullptr) {
+    std::vector<CpuProfileDeoptFrame> frames_vector(
+        deopt_frames, deopt_frames + deopt_frame_count);
+    entry->set_deopt_info(deopt_reason, deopt_id, std::move(frames_vector));
+  }
   delete[] deopt_frames;
 }
 

test/cctest/test-cpu-profiler.cc

@@ -1243,7 +1243,7 @@ static const char* call_function_test_source =
     "  } while (Date.now() - start < duration);\n"
     "}";
 
-// Test that if we sampled thread when it was inside FunctionCall buitin then
+// Test that if we sampled thread when it was inside FunctionCall builtin then
 // its caller frame will be '(unresolved function)' as we have no reliable way
 // to resolve it.
 //
@@ -2212,6 +2212,84 @@ TEST(FunctionDetailsInlining) {
                        script_b->GetUnboundScript()->GetId(), 1, 14, alpha);
 }
 
+static const char* pre_profiling_osr_script = R"(
+    function whenPass(pass, optDuration) {
+      if (pass == 5) startProfiling();
+    }
+    function hot(optDuration, deoptDuration) {
+      const startTime = Date.now();
+      %PrepareFunctionForOptimization(hot);
+      for (let pass = 0; pass <= optDuration + deoptDuration; pass++) {
+        // Let a few passes go by to ensure we have enough feeback info
+        if (pass == 3) %OptimizeOsr();
+        // Force deoptimization. %DeoptimizeNow and %DeoptimizeFunction don't
+        // doptimize OSRs.
+        if (pass == optDuration) whenPass = () => {};
+        whenPass(pass, optDuration);
+        for (let i = 0; i < 1e5; i++) {
+          for (let j = 0; j < 1000; j++) {
+            x = Math.random() * j;
+          }
+          if ((Date.now() - startTime) > pass) break;
+        }
+      }
+    }
+    function notHot(optDuration, deoptDuration) {
+      hot(optDuration, deoptDuration);
+      stopProfiling()
+    }
+  )";
+
+// Testing profiling of OSR code that was OSR optimized before profiling
+// started. Currently the behavior is not quite right so we're currently
+// testing a deopt event being sent to the sampling thread for a function
+// it knows nothing about. This deopt does mean we start getting samples
+// for hot so we expect some samples, just fewer than for notHot.
+//
+// We should get something like:
+//     0  (root):0 3 0 #1
+//    12    (garbage collector):0 3 0 #5
+//     5    notHot:22 0 4 #2
+//    85      hot:5 0 4 #6
+//     0      whenPass:2 0 4 #3
+//     0        startProfiling:0 2 0 #4
+//
+// But currently get something like:
+//     0  (root):0 3 0 #1
+//    12    (garbage collector):0 3 0 #5
+//    57    notHot:22 0 4 #2
+//    33      hot:5 0 4 #6
+//     0      whenPass:2 0 4 #3
+//     0        startProfiling:0 2 0 #4
+
+TEST(StartProfilingAfterOsr) {
+  i::FLAG_allow_natives_syntax = true;
+  v8::HandleScope scope(CcTest::isolate());
+  v8::Local<v8::Context> env = CcTest::NewContext({PROFILER_EXTENSION_ID});
+  v8::Context::Scope context_scope(env);
+  ProfilerHelper helper(env);
+  CompileRun(pre_profiling_osr_script);
+  v8::Local<v8::Function> function = GetFunction(env, "notHot");
+
+  int32_t profiling_optimized_ms = 80;
+  int32_t profiling_deoptimized_ms = 40;
+  v8::Local<v8::Value> args[] = {
+      v8::Integer::New(env->GetIsolate(), profiling_optimized_ms),
+      v8::Integer::New(env->GetIsolate(), profiling_deoptimized_ms)};
+  function->Call(env, env->Global(), arraysize(args), args).ToLocalChecked();
+  const v8::CpuProfile* profile = i::ProfilerExtension::last_profile;
+  CHECK(profile);
+  reinterpret_cast<const i::CpuProfile*>(profile)->Print();
+
+  const CpuProfileNode* root = profile->GetTopDownRoot();
+  const v8::CpuProfileNode* notHotNode = GetChild(env, root, "notHot");
+  const v8::CpuProfileNode* hotNode = GetChild(env, notHotNode, "hot");
+  USE(hotNode);
+  // If/when OSR sampling is fixed the following CHECK_GT could/should be
+  // uncommented and the node = node line deleted.
+  // CHECK_GT(hotNode->GetHitCount(), notHotNode->GetHitCount());
+}
+
 TEST(DontStopOnFinishedProfileDelete) {
   v8::HandleScope scope(CcTest::isolate());
   v8::Local<v8::Context> env = CcTest::NewContext({PROFILER_EXTENSION_ID});