Reland "Reland "[snapshot] Prepare for builtin SFIs in RO space""

This reverts commit 40e5dd0.

Reason for revert: web_test failures seen on win-rel bot after revert (and before previous reland).

Original change's description:
> Revert "Reland "[snapshot] Prepare for builtin SFIs in RO space""
>
> This reverts commit 4df825d.
>
> Reason for revert: speculative revert due to blink web_test failures blocking the roll:
> See win-rel tryjobs on https://crrev.com/c/4598368
>
> Original change's description:
> > Reland "[snapshot] Prepare for builtin SFIs in RO space"
> >
> > This is a reland of commit 232ce8b
> >
> > The reland fixes a stale variable found by GCMole.
> >
> > Original change's description:
> > > [snapshot] Prepare for builtin SFIs in RO space
> > >
> > > .. but do not enable ReadOnly allocation yet since builtin
> > > SharedFunctionInfos are not yet fully immutable. The debugger can
> > > attach/detach DebugInfo objects at runtime. In a separate CL, we'll move
> > > DebugInfo objects into a sidetable and remove them from the SFI.
> > >
> > > Bug: v8:13789
> > > Change-Id: I8e8f4fa7d9cb2cc68a2f0c76c3b2cb481e4e4ca6
> > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4575203
> > > Auto-Submit: Jakob Linke <jgruber@chromium.org>
> > > Reviewed-by: Igor Sheludko <ishell@chromium.org>
> > > Commit-Queue: Jakob Linke <jgruber@chromium.org>
> > > Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> > > Cr-Commit-Position: refs/heads/main@{#88101}
> >
> > Bug: v8:13789
> > Change-Id: I7ec1704f33923539f5afabcd4b485ad6aff16654
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4596061
> > Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
> > Commit-Queue: Jakob Linke <jgruber@chromium.org>
> > Reviewed-by: Igor Sheludko <ishell@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#88111}
>
> Bug: v8:13789
> Change-Id: Ia583bdd1857bb520c7102750cfe901dbb451bc2a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4600032
> Auto-Submit: Adam Klein <adamk@chromium.org>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#88129}

Bug: v8:13789
Change-Id: Id7761c980da43bec8b9a6dca35842b30c593c8f8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4601951
Commit-Queue: Adam Klein <adamk@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#88134}

src/diagnostics/objects-debug.cc

@@ -973,13 +973,30 @@ void JSFunction::JSFunctionVerify(Isolate* isolate) {
 
 void SharedFunctionInfo::SharedFunctionInfoVerify(Isolate* isolate) {
   // TODO(leszeks): Add a TorqueGeneratedClassVerifier for LocalIsolate.
-  this->SharedFunctionInfoVerify(ReadOnlyRoots(isolate));
+  SharedFunctionInfoVerify(ReadOnlyRoots(isolate));
 }
 
 void SharedFunctionInfo::SharedFunctionInfoVerify(LocalIsolate* isolate) {
-  this->SharedFunctionInfoVerify(ReadOnlyRoots(isolate));
+  SharedFunctionInfoVerify(ReadOnlyRoots(isolate));
 }
 
+namespace {
+
+bool ShouldVerifySharedFunctionInfoFunctionIndex(SharedFunctionInfo sfi) {
+  if (!sfi.HasBuiltinId()) return true;
+  switch (sfi.builtin_id()) {
+    case Builtin::kPromiseCapabilityDefaultReject:
+    case Builtin::kPromiseCapabilityDefaultResolve:
+      // For these we manually set custom function indices.
+      return false;
+    default:
+      return true;
+  }
+  UNREACHABLE();
+}
+
+}  // namespace
+
 void SharedFunctionInfo::SharedFunctionInfoVerify(ReadOnlyRoots roots) {
   Object value = name_or_scope_info(kAcquireLoad);
   if (value.IsScopeInfo()) {
@@ -1012,12 +1029,14 @@ void SharedFunctionInfo::SharedFunctionInfoVerify(ReadOnlyRoots roots) {
     CHECK(feedback_metadata().IsFeedbackMetadata());
   }
 
-  int expected_map_index =
-      Context::FunctionMapIndex(language_mode(), kind(), HasSharedName());
-  CHECK_EQ(expected_map_index, function_map_index());
+  if (ShouldVerifySharedFunctionInfoFunctionIndex(*this)) {
+    int expected_map_index =
+        Context::FunctionMapIndex(language_mode(), kind(), HasSharedName());
+    CHECK_EQ(expected_map_index, function_map_index());
+  }
 
-  if (!scope_info().IsEmpty()) {
-    ScopeInfo info = scope_info();
+  ScopeInfo info = EarlyScopeInfo(kAcquireLoad);
+  if (!info.IsEmpty()) {
     CHECK(kind() == info.function_kind());
     CHECK_EQ(internal::IsModule(kind()), info.scope_type() == MODULE_SCOPE);
   }

src/execution/local-isolate.h

@@ -100,6 +100,7 @@ class V8_EXPORT_PRIVATE LocalIsolate final : private HiddenLocalFactory {
   AccountingAllocator* allocator() { return isolate_->allocator(); }
 
   bool has_pending_exception() const { return false; }
+  bool serializer_enabled() const { return isolate_->serializer_enabled(); }
 
   void RegisterDeserializerStarted();
   void RegisterDeserializerFinished();

src/heap/factory-base.cc

@@ -443,19 +443,53 @@ FactoryBase<Impl>::NewUncompiledDataWithPreparseDataAndJob(
       AllocationType::kOld);
 }
 
+namespace {
+
+template <typename IsolateT>
+bool ShouldAllocateSharedFunctionInfoInReadOnlySpace(IsolateT* isolate,
+                                                     Builtin builtin) {
+  if (!isolate->serializer_enabled()) return false;
+  switch (builtin) {
+    case Builtin::kNoBuiltinId:
+      // Only builtin SFIs can be in RO space (for now).
+      return false;
+    case Builtin::kEmptyFunction:
+      // The empty function SFI points at a (non-RO) Script object.
+      return false;
+    default:
+      return true;
+  }
+  UNREACHABLE();
+}
+
+}  // namespace
+
 template <typename Impl>
 Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfo(
     MaybeHandle<String> maybe_name, MaybeHandle<HeapObject> maybe_function_data,
     Builtin builtin, FunctionKind kind) {
-  Handle<SharedFunctionInfo> shared = NewSharedFunctionInfo();
+  AllocationType allocation = AllocationType::kOld;
+  WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER;
+
+  // TODO(jgruber): Enable RO allocation once DebugInfos are no longer attached
+  // to the SFI.
+  // if (ShouldAllocateSharedFunctionInfoInReadOnlySpace(isolate(), builtin)) {
+  //   allocation = AllocationType::kReadOnly;
+  //   barrier_mode = SKIP_WRITE_BARRIER;
+  // }
+  USE(ShouldAllocateSharedFunctionInfoInReadOnlySpace(isolate(), builtin));
+
+  Handle<SharedFunctionInfo> shared = NewSharedFunctionInfo(allocation);
   DisallowGarbageCollection no_gc;
   SharedFunctionInfo raw = *shared;
   // Function names are assumed to be flat elsewhere.
   Handle<String> shared_name;
   bool has_shared_name = maybe_name.ToHandle(&shared_name);
   if (has_shared_name) {
     DCHECK(shared_name->IsFlat());
-    raw.set_name_or_scope_info(*shared_name, kReleaseStore);
+    DCHECK_IMPLIES(allocation == AllocationType::kReadOnly,
+                   ReadOnlyHeap::Contains(*shared_name));
+    raw.set_name_or_scope_info(*shared_name, kReleaseStore, barrier_mode);
   } else {
     DCHECK_EQ(raw.name_or_scope_info(kAcquireLoad),
               SharedFunctionInfo::kNoSharedNameSentinel);
@@ -467,7 +501,9 @@ Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfo(
     // the function_data should not be code with a builtin.
     DCHECK(!Builtins::IsBuiltinId(builtin));
     DCHECK(!function_data->IsInstructionStream());
-    raw.set_function_data(*function_data, kReleaseStore);
+    DCHECK_IMPLIES(allocation == AllocationType::kReadOnly,
+                   ReadOnlyHeap::Contains(*function_data));
+    raw.set_function_data(*function_data, kReleaseStore, barrier_mode);
   } else if (Builtins::IsBuiltinId(builtin)) {
     raw.set_builtin_id(builtin);
   } else {
@@ -1028,11 +1064,12 @@ Handle<SourceTextModuleInfo> FactoryBase<Impl>::NewSourceTextModuleInfo() {
 }
 
 template <typename Impl>
-Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfo() {
+Handle<SharedFunctionInfo> FactoryBase<Impl>::NewSharedFunctionInfo(
+    AllocationType allocation) {
   Map map = read_only_roots().shared_function_info_map();
 
   SharedFunctionInfo shared =
-      SharedFunctionInfo::cast(NewWithImmortalMap(map, AllocationType::kOld));
+      SharedFunctionInfo::cast(NewWithImmortalMap(map, allocation));
   DisallowGarbageCollection no_gc;
   int unique_id = -1;
 #if V8_SFI_HAS_UNIQUE_ID

src/heap/factory-base.h

@@ -350,7 +350,7 @@ class FactoryBase : public TorqueGeneratedFactory<Impl> {
                                              Handle<HeapObject> filler,
                                              AllocationType allocation);
 
-  Handle<SharedFunctionInfo> NewSharedFunctionInfo();
+  Handle<SharedFunctionInfo> NewSharedFunctionInfo(AllocationType allocation);
   Handle<SharedFunctionInfo> NewSharedFunctionInfo(
       MaybeHandle<String> maybe_name,
       MaybeHandle<HeapObject> maybe_function_data, Builtin builtin,

src/heap/heap.h

@@ -1789,6 +1789,10 @@ class Heap final {
                                 GarbageCollectionReason gc_reason,
                                 const char* collector_reason);
 
+  // For static-roots builds, pads the object to the required size.
+  void StaticRootsEnsureAllocatedSize(Handle<HeapObject> obj, int required);
+  // TODO(jgruber): Remove this once the created SFIs are allocated in RO space.
+  void CreateImportantSharedFunctionInfos();
   bool CreateEarlyReadOnlyMaps();
   bool CreateImportantReadOnlyObjects();
   bool CreateLateReadOnlyNonJSReceiverMaps();