[off-thread] Initialise placeholder SFI as full copy

Do a full copy of all fields when initialising and copying from the
placeholder SharedFunctionInfo that is used in off-thread function
compilation. This guarantees that all fields are correct both in the
on-thread and off-thread cases.

Change-Id: If1807c6f56fe38fea40ed39596f85634356e2623
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3260518Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77720}

src/codegen/compiler.cc

@@ -1565,10 +1565,9 @@ void BackgroundCompileTask::Run() {
         shared_info =
             CreateTopLevelSharedFunctionInfo(info_.get(), script_, &isolate);
       } else {
-        // Create a second, placeholder SFI for storing the results.
-        shared_info =
-            isolate.factory()->NewPlaceholderSharedFunctionInfoForLazyLiteral(
-                info_->literal(), script_);
+        // Clone into a placeholder SFI for storing the results.
+        shared_info = isolate.factory()->CloneSharedFunctionInfo(
+            input_shared_info_.ToHandleChecked());
       }
 
       if (IterativelyExecuteAndFinalizeUnoptimizedCompilationJobs(
@@ -1677,34 +1676,9 @@ bool BackgroundCompileTask::FinalizeFunction(
   FinalizeUnoptimizedCompilation(isolate, script_, flags_, &compile_state_,
                                  finalize_unoptimized_compilation_data_);
 
-  // Move the compiled data from the placeholder SFI to the real SFI.
+  // Move the compiled data from the placeholder SFI back to the real SFI.
   input_shared_info->CopyFrom(*result);
 
-#ifdef DEBUG
-  {
-    // Validate that tagged fields are equal.
-    STATIC_ASSERT(HeapObject::kHeaderSize ==
-                  SharedFunctionInfo::kStartOfWeakFieldsOffset);
-    STATIC_ASSERT(SharedFunctionInfo::kEndOfWeakFieldsOffset ==
-                  SharedFunctionInfo::kStartOfStrongFieldsOffset);
-    for (int offset = SharedFunctionInfo::kStartOfWeakFieldsOffset;
-         offset < SharedFunctionInfo::kEndOfStrongFieldsOffset;
-         offset += kTaggedSize) {
-      DCHECK_EQ(TaggedField<Object>::load(isolate, *input_shared_info, offset),
-                TaggedField<Object>::load(isolate, *result, offset));
-    }
-
-    // Validate that untagged fields are equal.
-    for (int offset = SharedFunctionInfo::kEndOfStrongFieldsOffset;
-         offset < SharedFunctionInfo::kSize; offset += kInt32Size) {
-      DCHECK_EQ(base::ReadUnalignedValue<int32_t>(input_shared_info->ptr() +
-                                                  offset - kHeapObjectTag),
-                base::ReadUnalignedValue<int32_t>(input_shared_info->ptr() +
-                                                  offset - kHeapObjectTag));
-    }
-  }
-#endif
-
   return true;
 }
 

src/heap/factory-base.cc

@@ -306,21 +306,18 @@ Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfoForLiteral(
 }
 
 template <typename Impl>
-Handle<SharedFunctionInfo>
-FactoryBase<Impl>::NewPlaceholderSharedFunctionInfoForLazyLiteral(
-    FunctionLiteral* literal, Handle<Script> script) {
-  FunctionKind kind = literal->kind();
-  Handle<SharedFunctionInfo> shared =
-      NewSharedFunctionInfo(literal->GetName(isolate()), MaybeHandle<Code>(),
-                            Builtin::kCompileLazy, kind);
-  // Don't fully initialise the SFI from the function literal, since we e.g.
-  // might not have the scope info, but initialise just enough to work for
-  // compilation/finalization.
-  shared->set_function_literal_id(literal->function_literal_id());
-  // Set the script on the SFI, but don't make the script's SFI list point back
-  // to this SFI.
-  shared->set_script(*script);
-  return shared;
+Handle<SharedFunctionInfo> FactoryBase<Impl>::CloneSharedFunctionInfo(
+    Handle<SharedFunctionInfo> other) {
+  Map map = read_only_roots().shared_function_info_map();
+
+  SharedFunctionInfo shared =
+      SharedFunctionInfo::cast(NewWithImmortalMap(map, AllocationType::kOld));
+  DisallowGarbageCollection no_gc;
+
+  shared.CopyFrom(*other);
+  shared.clear_padding();
+
+  return handle(shared, isolate());
 }
 
 template <typename Impl>

src/heap/factory-base.h

@@ -162,8 +162,10 @@ class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) FactoryBase
   Handle<SharedFunctionInfo> NewSharedFunctionInfoForLiteral(
       FunctionLiteral* literal, Handle<Script> script, bool is_toplevel);
 
-  Handle<SharedFunctionInfo> NewPlaceholderSharedFunctionInfoForLazyLiteral(
-      FunctionLiteral* literal, Handle<Script> script);
+  // Create a copy of a given SharedFunctionInfo for use as a placeholder in
+  // off-thread compilation
+  Handle<SharedFunctionInfo> CloneSharedFunctionInfo(
+      Handle<SharedFunctionInfo> other);
 
   Handle<PreparseData> NewPreparseData(int data_length, int children_length);
 

src/objects/shared-function-info.cc

@@ -227,20 +227,32 @@ void SharedFunctionInfo::SetScript(ReadOnlyRoots roots,
 }
 
 void SharedFunctionInfo::CopyFrom(SharedFunctionInfo other) {
-  DCHECK_EQ(script(), other.script());
-  DCHECK_EQ(function_literal_id(), other.function_literal_id());
-  DCHECK_EQ(Script::cast(script())
-                .shared_function_infos()
-                .Get(function_literal_id())
-                .GetHeapObject(),
-            *this);
-
   PtrComprCageBase cage_base = GetPtrComprCageBase(*this);
-  set_raw_scope_info(other.scope_info(cage_base, kAcquireLoad));
   set_function_data(other.function_data(cage_base, kAcquireLoad),
                     kReleaseStore);
-  set_feedback_metadata(other.feedback_metadata(cage_base, kAcquireLoad),
+  set_name_or_scope_info(other.name_or_scope_info(cage_base, kAcquireLoad),
+                         kReleaseStore);
+  set_outer_scope_info_or_feedback_metadata(
+      other.outer_scope_info_or_feedback_metadata(cage_base));
+  set_script_or_debug_info(other.script_or_debug_info(cage_base, kAcquireLoad),
                            kReleaseStore);
+
+  set_length(other.length());
+  set_formal_parameter_count(other.formal_parameter_count());
+  set_function_token_offset(other.function_token_offset());
+  set_expected_nof_properties(other.expected_nof_properties());
+  set_flags2(other.flags2());
+  set_flags(other.flags(kRelaxedLoad), kRelaxedStore);
+  set_function_literal_id(other.function_literal_id());
+#if V8_SFI_HAS_UNIQUE_ID
+  set_unique_id(other.unique_id());
+#endif
+
+  // This should now be byte-for-byte identical to the input.
+  DCHECK_EQ(memcmp(reinterpret_cast<void*>(address()),
+                   reinterpret_cast<void*>(other.address()),
+                   SharedFunctionInfo::kSize),
+            0);
 }
 
 bool SharedFunctionInfo::HasBreakInfo() const {
@@ -517,8 +529,8 @@ void SharedFunctionInfo::InitFromFunctionLiteral(
 
   // For lazy parsed functions, the following flags will be inaccurate since we
   // don't have the information yet. They're set later in
-  // SetSharedFunctionFlagsFromLiteral (compiler.cc), when the function is
-  // really parsed and compiled.
+  // UpdateSharedFunctionFlagsAfterCompilation (compiler.cc), when the function
+  // is really parsed and compiled.
   if (lit->ShouldEagerCompile()) {
     shared_info->set_has_duplicate_parameters(lit->has_duplicate_parameters());
     shared_info->UpdateAndFinalizeExpectedNofPropertiesFromEstimate(lit);

src/objects/shared-function-info.h

@@ -194,9 +194,9 @@ class SharedFunctionInfo
                                    int function_literal_id,
                                    bool reset_preparsed_scope_data = true);
 
-  // Copy the data from another SharedFunctionInfo. Used for copying from a
-  // placeholder SharedFunctionInfo after an off-thread compilation into the
-  // actual SharedFunctionInfo.
+  // Copy the data from another SharedFunctionInfo. Used for copying data into
+  // and out of a placeholder SharedFunctionInfo, for off-thread compilation
+  // which is not allowed to touch a main-thread-visible SharedFunctionInfo.
   void CopyFrom(SharedFunctionInfo other);
 
   // Layout description of the optimized code map.
@@ -559,7 +559,8 @@ class SharedFunctionInfo
   // TODO(caitp): make this a flag set during parsing
   inline bool has_simple_parameters();
 
-  // Initialize a SharedFunctionInfo from a parsed function literal.
+  // Initialize a SharedFunctionInfo from a parsed or preparsed function
+  // literal.
   template <typename IsolateT>
   static void InitFromFunctionLiteral(IsolateT* isolate,
                                       Handle<SharedFunctionInfo> shared_info,