[snapshot] Fix source positions deserialized from cache

When deserializing from a code snapshot with logging enabled (e.g. when
profiling) then this ensures source positions are collected before
creating code events that need them.

Bug: chromium:994673, v8:9504
Change-Id: Iad7644e983d3004c4889615cf2104dc4ef40da46
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1762023Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63286}

src/snapshot/code-serializer.cc

@@ -312,6 +312,8 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
                                              log_code_creation);
 #endif  // V8_TARGET_ARCH_ARM
 
+  bool needs_source_positions = isolate->NeedsSourcePositionsForProfiling();
+
   if (log_code_creation || FLAG_log_function_events) {
     Handle<Script> script(Script::cast(result->script()), isolate);
     Handle<String> name(script->name().IsString()
@@ -328,22 +330,29 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
     if (log_code_creation) {
       Script::InitLineEnds(script);
 
-      DisallowHeapAllocation no_gc;
       SharedFunctionInfo::ScriptIterator iter(isolate, *script);
-      for (i::SharedFunctionInfo info = iter.Next(); !info.is_null();
+      for (SharedFunctionInfo info = iter.Next(); !info.is_null();
            info = iter.Next()) {
         if (info.is_compiled()) {
-          int line_num = script->GetLineNumber(info.StartPosition()) + 1;
-          int column_num = script->GetColumnNumber(info.StartPosition()) + 1;
+          Handle<SharedFunctionInfo> shared_info(info, isolate);
+          if (needs_source_positions) {
+            SharedFunctionInfo::EnsureSourcePositionsAvailable(isolate,
+                                                               shared_info);
+          }
+          DisallowHeapAllocation no_gc;
+          int line_num =
+              script->GetLineNumber(shared_info->StartPosition()) + 1;
+          int column_num =
+              script->GetColumnNumber(shared_info->StartPosition()) + 1;
           PROFILE(isolate, CodeCreateEvent(CodeEventListener::SCRIPT_TAG,
-                                           info.abstract_code(), info, *name,
-                                           line_num, column_num));
+                                           info.abstract_code(), *shared_info,
+                                           *name, line_num, column_num));
         }
       }
     }
   }
 
-  if (isolate->NeedsSourcePositionsForProfiling()) {
+  if (needs_source_positions) {
     Handle<Script> script(Script::cast(result->script()), isolate);
     Script::InitLineEnds(script);
   }

test/cctest/test-serialize.cc

@@ -1516,6 +1516,49 @@ static Handle<SharedFunctionInfo> CompileScriptAndProduceCache(
   return sfi;
 }
 
+TEST(CodeSerializerWithProfiler) {
+  FLAG_enable_lazy_source_positions = true;
+
+  LocalContext context;
+  Isolate* isolate = CcTest::i_isolate();
+  isolate->compilation_cache()->Disable();  // Disable same-isolate code cache.
+
+  v8::HandleScope scope(CcTest::isolate());
+
+  const char* source = "1 + 1";
+
+  Handle<String> orig_source = isolate->factory()
+                                   ->NewStringFromUtf8(CStrVector(source))
+                                   .ToHandleChecked();
+  Handle<String> copy_source = isolate->factory()
+                                   ->NewStringFromUtf8(CStrVector(source))
+                                   .ToHandleChecked();
+  CHECK(!orig_source.is_identical_to(copy_source));
+  CHECK(orig_source->Equals(*copy_source));
+
+  ScriptData* cache = nullptr;
+
+  Handle<SharedFunctionInfo> orig = CompileScriptAndProduceCache(
+      isolate, orig_source, Handle<String>(), &cache,
+      v8::ScriptCompiler::kNoCompileOptions);
+
+  CHECK(!orig->GetBytecodeArray().HasSourcePositionTable());
+
+  isolate->set_is_profiling(true);
+
+  // This does not assert that no compilation can happen as source position
+  // collection could trigger it.
+  Handle<SharedFunctionInfo> copy =
+      CompileScript(isolate, copy_source, Handle<String>(), cache,
+                    v8::ScriptCompiler::kConsumeCodeCache);
+
+  // Since the profiler is now enabled, source positions should be collected
+  // after deserialization.
+  CHECK(copy->GetBytecodeArray().HasSourcePositionTable());
+
+  delete cache;
+}
+
 void TestCodeSerializerOnePlusOneImpl(bool verify_builtins_count = true) {
   LocalContext context;
   Isolate* isolate = CcTest::i_isolate();