Revert "[builtins] Support embedded builtins in nosnapshot builds"

This reverts commit bf2f0a02.

Reason for revert: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20nosnap%20-%20debug/21753

Original change's description:
> [builtins] Support embedded builtins in nosnapshot builds
> 
> This CL adds support for embedded builtins in nosnap builds by creating
> and setting an 'embedded blob' after builtin generation. Unlike
> snapshot builds, the blob is not embedded into the .text section but
> located on the C++ heap.
> 
> This makes nosnap builds more consistent with mksnapshot, and allows us
> to simplify there and in serializer cctests.
> 
> Complications arise from the different workflows we need to support:
> 
> 1. the standard mksnapshot build process,
> 2. nosnap builds (which reuse the blob created by the first Isolate),
> 2. and tests with various complicated serialization workflows.
> 
> To cover all of these cases, this CL introduces two knobs to twiddle:
> 
> 1. A 'sticky' embedded blob which overrides compiled-in default
>    embedded blobs at Isolate setup.
> 2. The blob lifecycle can be managed manually or through refcounting.
> 
> These are described in more detail in isolate.cc.
> 
> Tbr: ulan@chromium.org
> Bug: v8:6666, v8:8350
> Change-Id: I3842e40cdaf45d2cadd05c6eb1ec2f5e3d83568d
> Reviewed-on: https://chromium-review.googlesource.com/c/1310195
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#57523}

TBR=ulan@chromium.org,yangguo@chromium.org,jgruber@chromium.org

Change-Id: I6e35a0cb7186fb50f1012f5c618fb8b48b24a813
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6666, v8:8350
Reviewed-on: https://chromium-review.googlesource.com/c/1337577Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57529}

BUILD.gn

@@ -24,8 +24,6 @@ import("snapshot_toolchain.gni")
 is_target_simulator = (target_cpu != v8_target_cpu && !v8_multi_arch_build) ||
                       (current_cpu != v8_current_cpu && v8_multi_arch_build)
 
-is_msvc = is_win && !is_clang
-
 declare_args() {
   # Print to stdout on Android.
   v8_android_log_stdout = false
@@ -85,8 +83,9 @@ declare_args() {
   v8_enable_fast_mksnapshot = false
 
   # Enable embedded builtins.
-  # TODO(v8:6666): Support aix and MSVC.
-  v8_enable_embedded_builtins = !is_aix && !is_msvc
+  # TODO(v8:6666): Support no-snapshot builds, aix and MSVC.
+  v8_enable_embedded_builtins =
+      v8_use_snapshot && !is_aix && (!is_win || is_clang)
 
   # Enable code-generation-time checking of types in the CodeStubAssembler.
   v8_enable_verify_csa = false
@@ -216,6 +215,8 @@ if (v8_check_microtasks_scopes_consistency == "") {
       v8_enable_debugging_features || dcheck_always_on
 }
 
+assert(!v8_enable_embedded_builtins || v8_use_snapshot,
+       "Embedded builtins only work with snapshots")
 assert(
     v8_current_cpu != "x86" || !v8_enable_embedded_builtins ||
         !v8_untrusted_code_mitigations,

src/assembler.cc

@@ -65,20 +65,17 @@ AssemblerOptions AssemblerOptions::EnableV8AgnosticCode() const {
 AssemblerOptions AssemblerOptions::Default(
     Isolate* isolate, bool explicitly_support_serialization) {
   AssemblerOptions options;
-  const bool serializer =
+  bool serializer =
       isolate->serializer_enabled() || explicitly_support_serialization;
-  const bool generating_embedded_builtin =
-      isolate->ShouldLoadConstantsFromRootList();
   options.record_reloc_info_for_serialization = serializer;
-  options.enable_root_array_delta_access =
-      !serializer && !generating_embedded_builtin;
+  options.enable_root_array_delta_access = !serializer;
 #ifdef USE_SIMULATOR
   // Don't generate simulator specific code if we are building a snapshot, which
   // might be run on real hardware.
   options.enable_simulator_code = !serializer;
 #endif
-  options.inline_offheap_trampolines =
-      !serializer && !generating_embedded_builtin;
+  options.inline_offheap_trampolines = !serializer;
+
 #if V8_TARGET_ARCH_X64 || V8_TARGET_ARCH_ARM64
   const base::AddressRegion& code_range =
       isolate->heap()->memory_allocator()->code_range();

src/builtins/builtins.cc

@@ -296,6 +296,7 @@ class OffHeapTrampolineGenerator {
 // static
 Handle<Code> Builtins::GenerateOffHeapTrampolineFor(Isolate* isolate,
                                                     Address off_heap_entry) {
+  DCHECK(isolate->serializer_enabled());
   DCHECK_NOT_NULL(isolate->embedded_blob());
   DCHECK_NE(0, isolate->embedded_blob_size());
 

src/builtins/constants-table-builder.cc

@@ -35,8 +35,8 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
   // Must be on the main thread.
   DCHECK(ThreadId::Current().Equals(isolate_->thread_id()));
 
-  // Must be generating embedded builtin code.
-  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
+  // Must be serializing.
+  DCHECK(isolate_->serializer_enabled());
 #endif
 
   uint32_t* maybe_key = map_.Find(object);
@@ -62,7 +62,7 @@ void BuiltinsConstantsTableBuilder::PatchSelfReference(
   DCHECK_EQ(ReadOnlyRoots(isolate_).empty_fixed_array(),
             isolate_->heap()->builtins_constants_table());
 
-  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
+  DCHECK(isolate_->serializer_enabled());
 
   DCHECK(self_reference->IsOddball());
   DCHECK(Oddball::cast(*self_reference)->kind() ==
@@ -86,7 +86,7 @@ void BuiltinsConstantsTableBuilder::Finalize() {
 
   DCHECK_EQ(ReadOnlyRoots(isolate_).empty_fixed_array(),
             isolate_->heap()->builtins_constants_table());
-  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
+  DCHECK(isolate_->serializer_enabled());
 
   // An empty map means there's nothing to do.
   if (map_.size() == 0) return;

src/compiler/pipeline.cc

@@ -1578,8 +1578,7 @@ struct InstructionSelectionPhase {
         FLAG_turbo_instruction_scheduling
             ? InstructionSelector::kEnableScheduling
             : InstructionSelector::kDisableScheduling,
-        !data->isolate() || data->isolate()->serializer_enabled() ||
-                data->isolate()->ShouldLoadConstantsFromRootList()
+        !data->isolate() || data->isolate()->serializer_enabled()
             ? InstructionSelector::kDisableRootsRelativeAddressing
             : InstructionSelector::kEnableRootsRelativeAddressing,
         data->info()->GetPoisoningMitigationLevel(),

src/heap/factory.cc

@@ -2681,6 +2681,7 @@ Handle<Code> Factory::NewCode(
 
 Handle<Code> Factory::NewOffHeapTrampolineFor(Handle<Code> code,
                                               Address off_heap_entry) {
+  CHECK(isolate()->serializer_enabled());
   CHECK_NOT_NULL(isolate()->embedded_blob());
   CHECK_NE(0, isolate()->embedded_blob_size());
   CHECK(Builtins::IsIsolateIndependentBuiltin(*code));

src/isolate.cc

@@ -108,103 +108,24 @@ namespace {
 
 std::atomic<const uint8_t*> current_embedded_blob_(nullptr);
 std::atomic<uint32_t> current_embedded_blob_size_(0);
-
-// The various workflows around embedded snapshots are fairly complex. We need
-// to support plain old snapshot builds, nosnap builds, and the requirements of
-// subtly different serialization tests. There's two related knobs to twiddle:
-//
-// - The default embedded blob may be overridden by setting the sticky embedded
-// blob. This is set automatically whenever we create a new embedded blob.
-//
-// - Lifecycle management can be either manual or set to refcounting.
-//
-// A few situations to demonstrate their use:
-//
-// - A plain old snapshot build neither overrides the default blob nor
-// refcounts.
-//
-// - mksnapshot sets the sticky blob and manually frees the embedded
-// blob once done.
-//
-// - Most serializer tests do the same.
-//
-// - Nosnapshot builds set the sticky blob and enable refcounting.
-
-// This mutex protects access to the following variables:
-// - sticky_embedded_blob_
-// - sticky_embedded_blob_size_
-// - enable_embedded_blob_refcounting_
-// - current_embedded_blob_refs_
-base::LazyMutex current_embedded_blob_refcount_mutex_ = LAZY_MUTEX_INITIALIZER;
-
-const uint8_t* sticky_embedded_blob_ = nullptr;
-uint32_t sticky_embedded_blob_size_ = 0;
-
-bool enable_embedded_blob_refcounting_ = true;
-int current_embedded_blob_refs_ = 0;
-
-const uint8_t* StickyEmbeddedBlob() { return sticky_embedded_blob_; }
-uint32_t StickyEmbeddedBlobSize() { return sticky_embedded_blob_size_; }
-
-void SetStickyEmbeddedBlob(const uint8_t* blob, uint32_t blob_size) {
-  sticky_embedded_blob_ = blob;
-  sticky_embedded_blob_size_ = blob_size;
-}
-
 }  // namespace
 
-void DisableEmbeddedBlobRefcounting() {
-  base::MutexGuard guard(current_embedded_blob_refcount_mutex_.Pointer());
-  enable_embedded_blob_refcounting_ = false;
-}
-
-void FreeCurrentEmbeddedBlob() {
-  CHECK(!enable_embedded_blob_refcounting_);
-  base::MutexGuard guard(current_embedded_blob_refcount_mutex_.Pointer());
-
-  if (StickyEmbeddedBlob() == nullptr) return;
-
-  CHECK_EQ(StickyEmbeddedBlob(), Isolate::CurrentEmbeddedBlob());
-
-  InstructionStream::FreeOffHeapInstructionStream(
-      const_cast<uint8_t*>(Isolate::CurrentEmbeddedBlob()),
-      Isolate::CurrentEmbeddedBlobSize());
-
-  current_embedded_blob_.store(nullptr, std::memory_order_relaxed);
-  current_embedded_blob_size_.store(0, std::memory_order_relaxed);
-  sticky_embedded_blob_ = nullptr;
-  sticky_embedded_blob_size_ = 0;
-}
-
 void Isolate::SetEmbeddedBlob(const uint8_t* blob, uint32_t blob_size) {
-  CHECK_NOT_NULL(blob);
-
   embedded_blob_ = blob;
   embedded_blob_size_ = blob_size;
   current_embedded_blob_.store(blob, std::memory_order_relaxed);
   current_embedded_blob_size_.store(blob_size, std::memory_order_relaxed);
 
 #ifdef DEBUG
-  // Verify that the contents of the embedded blob are unchanged from
-  // serialization-time, just to ensure the compiler isn't messing with us.
-  EmbeddedData d = EmbeddedData::FromBlob();
-  CHECK_EQ(d.Hash(), d.CreateHash());
+  if (blob != nullptr) {
+    // Verify that the contents of the embedded blob are unchanged from
+    // serialization-time, just to ensure the compiler isn't messing with us.
+    EmbeddedData d = EmbeddedData::FromBlob();
+    CHECK_EQ(d.Hash(), d.CreateHash());
+  }
 #endif  // DEBUG
 }
 
-void Isolate::ClearEmbeddedBlob() {
-  CHECK(enable_embedded_blob_refcounting_);
-  CHECK_EQ(embedded_blob_, CurrentEmbeddedBlob());
-  CHECK_EQ(embedded_blob_, StickyEmbeddedBlob());
-
-  embedded_blob_ = nullptr;
-  embedded_blob_size_ = 0;
-  current_embedded_blob_.store(nullptr, std::memory_order_relaxed);
-  current_embedded_blob_size_.store(0, std::memory_order_relaxed);
-  sticky_embedded_blob_ = nullptr;
-  sticky_embedded_blob_size_ = 0;
-}
-
 const uint8_t* Isolate::embedded_blob() const { return embedded_blob_; }
 uint32_t Isolate::embedded_blob_size() const { return embedded_blob_size_; }
 
@@ -2802,7 +2723,17 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator)
   InitializeLoggingAndCounters();
   debug_ = new Debug(this);
 
-  InitializeDefaultEmbeddedBlob();
+  if (FLAG_embedded_builtins) {
+#ifdef V8_MULTI_SNAPSHOTS
+  if (FLAG_untrusted_code_mitigations) {
+    SetEmbeddedBlob(DefaultEmbeddedBlob(), DefaultEmbeddedBlobSize());
+  } else {
+    SetEmbeddedBlob(TrustedEmbeddedBlob(), TrustedEmbeddedBlobSize());
+  }
+#else
+  SetEmbeddedBlob(DefaultEmbeddedBlob(), DefaultEmbeddedBlobSize());
+#endif
+  }
 }
 
 void Isolate::CheckIsolateLayout() {
@@ -2905,7 +2836,14 @@ void Isolate::Deinit() {
     wasm_engine_.reset();
   }
 
-  TearDownEmbeddedBlob();
+  if (FLAG_embedded_builtins) {
+    if (DefaultEmbeddedBlob() == nullptr && embedded_blob() != nullptr) {
+      // We own the embedded blob. Free it.
+      uint8_t* data = const_cast<uint8_t*>(embedded_blob_);
+      InstructionStream::FreeOffHeapInstructionStream(data,
+                                                      embedded_blob_size_);
+    }
+  }
 
   delete interpreter_;
   interpreter_ = nullptr;
@@ -3077,6 +3015,7 @@ void PrintBuiltinSizes(Isolate* isolate) {
 }
 
 void CreateOffHeapTrampolines(Isolate* isolate) {
+  DCHECK(isolate->serializer_enabled());
   DCHECK_NOT_NULL(isolate->embedded_blob());
   DCHECK_NE(0, isolate->embedded_blob_size());
 
@@ -3108,75 +3047,23 @@ void CreateOffHeapTrampolines(Isolate* isolate) {
 }
 }  // namespace
 
-void Isolate::InitializeDefaultEmbeddedBlob() {
-  const uint8_t* blob = DefaultEmbeddedBlob();
-  uint32_t size = DefaultEmbeddedBlobSize();
-
-#ifdef V8_MULTI_SNAPSHOTS
-  if (!FLAG_untrusted_code_mitigations) {
-    blob = TrustedEmbeddedBlob();
-    size = TrustedEmbeddedBlobSize();
-  }
-#endif
-
-  if (StickyEmbeddedBlob() != nullptr) {
-    base::MutexGuard guard(current_embedded_blob_refcount_mutex_.Pointer());
-    // Check again now that we hold the lock.
-    if (StickyEmbeddedBlob() != nullptr) {
-      blob = StickyEmbeddedBlob();
-      size = StickyEmbeddedBlobSize();
-      current_embedded_blob_refs_++;
-    }
-  }
-
-  if (blob == nullptr) {
-    CHECK_EQ(0, size);
-  } else {
-    SetEmbeddedBlob(blob, size);
-  }
-}
-
-void Isolate::CreateAndSetEmbeddedBlob() {
-  base::MutexGuard guard(current_embedded_blob_refcount_mutex_.Pointer());
-
-  // If a sticky blob has been set, we reuse it.
-  if (StickyEmbeddedBlob() != nullptr) {
-    CHECK_EQ(embedded_blob(), StickyEmbeddedBlob());
-    CHECK_EQ(CurrentEmbeddedBlob(), StickyEmbeddedBlob());
-  } else {
-    // Create and set a new embedded blob.
-    uint8_t* data;
-    uint32_t size;
-    InstructionStream::CreateOffHeapInstructionStream(this, &data, &size);
-
-    CHECK_EQ(0, current_embedded_blob_refs_);
-    const uint8_t* const_data = const_cast<const uint8_t*>(data);
-    SetEmbeddedBlob(const_data, size);
-    current_embedded_blob_refs_++;
-
-    SetStickyEmbeddedBlob(const_data, size);
-  }
-
+void Isolate::PrepareEmbeddedBlobForSerialization() {
+  // When preparing the embedded blob, ensure it doesn't exist yet.
+  DCHECK_NULL(embedded_blob());
+  DCHECK_NULL(DefaultEmbeddedBlob());
+  DCHECK(serializer_enabled());
+
+  // The isolate takes ownership of this pointer into an executable mmap'd
+  // area. We muck around with const-casts because the standard use-case in
+  // shipping builds is for embedded_blob_ to point into a read-only
+  // .text-embedded section.
+  uint8_t* data;
+  uint32_t size;
+  InstructionStream::CreateOffHeapInstructionStream(this, &data, &size);
+  SetEmbeddedBlob(const_cast<const uint8_t*>(data), size);
   CreateOffHeapTrampolines(this);
 }
 
-void Isolate::TearDownEmbeddedBlob() {
-  // Nothing to do in case the blob is embedded into the binary or unset.
-  if (StickyEmbeddedBlob() == nullptr) return;
-
-  CHECK_EQ(embedded_blob(), StickyEmbeddedBlob());
-  CHECK_EQ(CurrentEmbeddedBlob(), StickyEmbeddedBlob());
-
-  base::MutexGuard guard(current_embedded_blob_refcount_mutex_.Pointer());
-  current_embedded_blob_refs_--;
-  if (current_embedded_blob_refs_ == 0 && enable_embedded_blob_refcounting_) {
-    // We own the embedded blob and are the last holder. Free it.
-    InstructionStream::FreeOffHeapInstructionStream(
-        const_cast<uint8_t*>(embedded_blob()), embedded_blob_size());
-    ClearEmbeddedBlob();
-  }
-}
-
 bool Isolate::Init(StartupDeserializer* des) {
   TRACE_ISOLATE(init);
 
@@ -3279,16 +3166,19 @@ bool Isolate::Init(StartupDeserializer* des) {
 
   bootstrapper_->Initialize(create_heap_objects);
 
-  if (FLAG_embedded_builtins && create_heap_objects) {
-    builtins_constants_table_builder_ = new BuiltinsConstantsTableBuilder(this);
+  if (FLAG_embedded_builtins) {
+    if (create_heap_objects && serializer_enabled()) {
+      builtins_constants_table_builder_ =
+          new BuiltinsConstantsTableBuilder(this);
+    }
   }
   setup_delegate_->SetupBuiltins(this);
-  if (FLAG_embedded_builtins && create_heap_objects) {
-    builtins_constants_table_builder_->Finalize();
-    delete builtins_constants_table_builder_;
-    builtins_constants_table_builder_ = nullptr;
-
-    CreateAndSetEmbeddedBlob();
+  if (FLAG_embedded_builtins) {
+    if (create_heap_objects && serializer_enabled()) {
+      builtins_constants_table_builder_->Finalize();
+      delete builtins_constants_table_builder_;
+      builtins_constants_table_builder_ = nullptr;
+    }
   }
 
   if (create_heap_objects) heap_.CreateFixedStubs();

src/isolate.h

@@ -335,11 +335,6 @@ class WasmEngine;
   inline void set_##name(type v) { name##_ = v; }  \
   inline type name() const { return name##_; }
 
-// Controls for manual embedded blob lifecycle management, used by tests and
-// mksnapshot.
-V8_EXPORT_PRIVATE void DisableEmbeddedBlobRefcounting();
-V8_EXPORT_PRIVATE void FreeCurrentEmbeddedBlob();
-
 class ThreadLocalTop {
  public:
   // Does early low-level initialization that does not depend on the
@@ -1467,12 +1462,20 @@ class Isolate final : private HiddenFactory {
 
   // Off-heap builtins cannot embed constants within the code object itself,
   // and thus need to load them from the root list.
-  // TODO(jgruber): Rename to IsGeneratingEmbeddedBuiltins().
   bool ShouldLoadConstantsFromRootList() const {
-    return FLAG_embedded_builtins &&
-           builtins_constants_table_builder() != nullptr;
+    if (FLAG_embedded_builtins) {
+      return (serializer_enabled() &&
+              builtins_constants_table_builder() != nullptr);
+    } else {
+      return false;
+    }
   }
 
+  // Called only prior to serialization.
+  // This function copies off-heap-safe builtins off the heap, creates off-heap
+  // trampolines, and sets up this isolate's embedded blob.
+  void PrepareEmbeddedBlobForSerialization();
+
   BuiltinsConstantsTableBuilder* builtins_constants_table_builder() const {
     return builtins_constants_table_builder_;
   }
@@ -1860,12 +1863,7 @@ class Isolate final : private HiddenFactory {
   // which is stored on the root list prior to serialization.
   BuiltinsConstantsTableBuilder* builtins_constants_table_builder_ = nullptr;
 
-  void InitializeDefaultEmbeddedBlob();
-  void CreateAndSetEmbeddedBlob();
-  void TearDownEmbeddedBlob();
-
   void SetEmbeddedBlob(const uint8_t* blob, uint32_t blob_size);
-  void ClearEmbeddedBlob();
 
   const uint8_t* embedded_blob_ = nullptr;
   uint32_t embedded_blob_size_ = 0;

src/snapshot/mksnapshot.cc

@@ -418,7 +418,9 @@ v8::StartupData WarmUpSnapshotDataBlob(v8::SnapshotCreator* snapshot_creator,
   return result;
 }
 
-void WriteEmbeddedFile(SnapshotWriter* writer) {
+void WriteEmbeddedFile(v8::SnapshotCreator* creator, SnapshotWriter* writer) {
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(creator->GetIsolate());
+  isolate->PrepareEmbeddedBlobForSerialization();
   i::EmbeddedData embedded_blob = i::EmbeddedData::FromBlob();
   writer->WriteEmbedded(&embedded_blob);
 }
@@ -461,7 +463,6 @@ int main(int argc, char** argv) {
     std::unique_ptr<char> warmup_script(
         GetExtraCode(argc >= 3 ? argv[2] : nullptr, "warm up"));
 
-    i::DisableEmbeddedBlobRefcounting();
     v8::StartupData blob;
     {
       v8::Isolate* isolate = v8::Isolate::Allocate();
@@ -477,7 +478,13 @@ int main(int argc, char** argv) {
         i_isolate->heap()->ConfigureHeap(0, 0, code_range_size);
       }
       v8::SnapshotCreator snapshot_creator(isolate);
-      if (i::FLAG_embedded_builtins) WriteEmbeddedFile(&writer);
+      if (i::FLAG_embedded_builtins) {
+        // This process is a bit tricky since we might go on to make a second
+        // snapshot if a warmup script is passed. In that case, create the first
+        // snapshot without off-heap trampolines and only move code off-heap for
+        // the warmed-up snapshot.
+        if (!warmup_script) WriteEmbeddedFile(&snapshot_creator, &writer);
+      }
       blob = CreateSnapshotDataBlob(&snapshot_creator, embed_script.get());
     }
 
@@ -485,6 +492,9 @@ int main(int argc, char** argv) {
       CHECK(blob.raw_size > 0 && blob.data != nullptr);
       v8::StartupData cold = blob;
       v8::SnapshotCreator snapshot_creator(nullptr, &cold);
+      if (i::FLAG_embedded_builtins) {
+        WriteEmbeddedFile(&snapshot_creator, &writer);
+      }
       blob = WarmUpSnapshotDataBlob(&snapshot_creator, warmup_script.get());
       delete[] cold.data;
     }
@@ -493,7 +503,6 @@ int main(int argc, char** argv) {
     writer.WriteSnapshot(blob);
     delete[] blob.data;
   }
-  i::FreeCurrentEmbeddedBlob();
 
   v8::V8::Dispose();
   v8::V8::ShutdownPlatform();

test/cctest/cctest.status

@@ -176,13 +176,6 @@
   'test-api/Float64Array': [SKIP],
 }],  # 'arch == arm64 and mode == debug and simulator_run'
 
-##############################################################################
-['arch == ia32', {
-  # Requires call to non-isolate-independent code target, unsupported by
-  # MacroAssembler::Call() on ia32.
-  'test-api/SetFunctionEntryHook': [SKIP],
-}],  # 'no_snap == False'
-
 ##############################################################################
 ['variant == nooptimization and (arch == arm or arch == arm64) and simulator_run', {
   # Slow tests: https://crbug.com/v8/7783

test/cctest/test-api.cc

@@ -14582,8 +14582,6 @@ class SetFunctionEntryHookTest {
   SymbolLocationMap symbol_locations_;
   InvocationMap invocations_;
 
-  i::Isolate* isolate_ = nullptr;
-
   static SetFunctionEntryHookTest* instance_;
 };
 SetFunctionEntryHookTest* SetFunctionEntryHookTest::instance_ = nullptr;
@@ -14670,8 +14668,8 @@ void SetFunctionEntryHookTest::OnJitEvent(const v8::JitCodeEvent* event) {
 void SetFunctionEntryHookTest::OnEntryHook(
     uintptr_t function, uintptr_t return_addr_location) {
   // Get the function's code object.
-  i::Code function_code = isolate_->heap()->GcSafeFindCodeForInnerPointer(
-      static_cast<i::Address>(function));
+  i::Code function_code =
+      i::Code::GetCodeFromTargetAddress(static_cast<i::Address>(function));
   CHECK(!function_code.is_null());
 
   // Then try and look up the caller's code object.
@@ -14797,9 +14795,7 @@ void SetFunctionEntryHookTest::RunTest() {
   create_params.entry_hook = EntryHook;
   create_params.code_event_handler = JitEvent;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  v8::Isolate* isolate = v8::Isolate::Allocate();
-  isolate_ = reinterpret_cast<i::Isolate*>(isolate);
-  v8::Isolate::Initialize(isolate, create_params);
+  v8::Isolate* isolate = v8::Isolate::New(create_params);
 
   {
     v8::Isolate::Scope scope(isolate);
@@ -14842,9 +14838,7 @@ void SetFunctionEntryHookTest::RunTest() {
   // Make sure a second isolate is unaffected by the previous entry hook.
   create_params = v8::Isolate::CreateParams();
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  isolate = v8::Isolate::Allocate();
-  isolate_ = reinterpret_cast<i::Isolate*>(isolate);
-  v8::Isolate::Initialize(isolate, create_params);
+  isolate = v8::Isolate::New(create_params);
   {
     v8::Isolate::Scope scope(isolate);
 
@@ -14858,7 +14852,8 @@ void SetFunctionEntryHookTest::RunTest() {
   isolate->Dispose();
 }
 
-UNINITIALIZED_TEST(SetFunctionEntryHook) {
+
+TEST(SetFunctionEntryHook) {
   // FunctionEntryHook does not work well with experimental natives.
   // Experimental natives are compiled during snapshot deserialization.
   // This test breaks because InstallGetter (function from snapshot that

test/cctest/test-serialize.cc

@@ -85,7 +85,6 @@ class TestSerializer {
   static v8::Isolate* NewIsolateInitialized() {
     const bool kEnableSerializer = true;
     const bool kGenerateHeap = true;
-    DisableEmbeddedBlobRefcounting();
     v8::Isolate* v8_isolate = NewIsolate(kEnableSerializer, kGenerateHeap);
     v8::Isolate::Scope isolate_scope(v8_isolate);
     i::Isolate* isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
@@ -128,10 +127,25 @@ class TestSerializer {
     isolate->set_array_buffer_allocator(CcTest::array_buffer_allocator());
     isolate->setup_delegate_ = new SetupIsolateDelegateForTests(generate_heap);
 
+    if (FLAG_embedded_builtins) {
+      if (generate_heap || clear_embedded_blob_) {
+        // We're generating the heap, including new builtins. Act as if we don't
+        // have an embedded blob.
+        clear_embedded_blob_ = true;
+        isolate->SetEmbeddedBlob(nullptr, 0);
+      }
+    }
     return v8_isolate;
   }
+
+  // A sticky flag that ensures the embedded blob is remains cleared after it
+  // has been cleared once. E.g.: after creating & serializing a complete heap
+  // snapshot, future isolates also expect the embedded blob to be cleared.
+  static bool clear_embedded_blob_;
 };
 
+bool TestSerializer::clear_embedded_blob_ = false;
+
 static Vector<const byte> WritePayload(const Vector<const byte>& payload) {
   int length = payload.length();
   byte* blob = NewArray<byte>(length);
@@ -166,7 +180,6 @@ bool RunExtraCode(v8::Isolate* isolate, v8::Local<v8::Context> context,
 v8::StartupData CreateSnapshotDataBlob(const char* embedded_source = nullptr) {
   // Create a new isolate and a new context from scratch, optionally run
   // a script to embed, and serialize to create a snapshot blob.
-  DisableEmbeddedBlobRefcounting();
   v8::StartupData result = {nullptr, 0};
   {
     v8::SnapshotCreator snapshot_creator;
@@ -303,7 +316,6 @@ void TestStartupSerializerOnceImpl() {
   }
   isolate->Dispose();
   blobs.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerOnce) {
@@ -407,7 +419,6 @@ UNINITIALIZED_TEST(StartupSerializerTwice) {
   }
   isolate->Dispose();
   blobs2.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerOnceRunScript) {
@@ -433,7 +444,6 @@ UNINITIALIZED_TEST(StartupSerializerOnceRunScript) {
   }
   isolate->Dispose();
   blobs.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerTwiceRunScript) {
@@ -460,7 +470,6 @@ UNINITIALIZED_TEST(StartupSerializerTwiceRunScript) {
   }
   isolate->Dispose();
   blobs2.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 static void PartiallySerializeContext(Vector<const byte>* startup_blob_out,
@@ -567,7 +576,6 @@ UNINITIALIZED_TEST(PartialSerializerContext) {
   }
   v8_isolate->Dispose();
   blobs.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 static void PartiallySerializeCustomContext(
@@ -745,7 +753,6 @@ UNINITIALIZED_TEST(PartialSerializerCustomContext) {
   }
   v8_isolate->Dispose();
   blobs.Dispose();
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlob1) {
@@ -772,7 +779,6 @@ TEST(CustomSnapshotDataBlob1) {
   }
   isolate1->Dispose();
   delete[] data1.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(SnapshotChecksum) {
@@ -784,7 +790,6 @@ TEST(SnapshotChecksum) {
   const_cast<char*>(data1.data)[142] = data1.data[142] ^ 4;  // Flip a bit.
   CHECK(!i::Snapshot::VerifyChecksum(&data1));
   delete[] data1.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 struct InternalFieldData {
@@ -840,7 +845,6 @@ void TypedArrayTestHelper(
     const Int32Expectations& after_restore_expectations = Int32Expectations()) {
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
-  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -882,7 +886,6 @@ void TypedArrayTestHelper(
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobWithOffHeapTypedArray) {
@@ -967,7 +970,6 @@ TEST(CustomSnapshotDataBlobNeuteredArrayBuffer) {
 
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
-  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -1012,7 +1014,6 @@ TEST(CustomSnapshotDataBlobNeuteredArrayBuffer) {
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 i::Handle<i::JSArrayBuffer> GetBufferFromTypedArray(
@@ -1039,7 +1040,6 @@ TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
 
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
-  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -1088,7 +1088,6 @@ TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlob2) {
@@ -1118,7 +1117,6 @@ TEST(CustomSnapshotDataBlob2) {
   }
   isolate2->Dispose();
   delete[] data2.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 static void SerializationFunctionTemplate(
@@ -1170,14 +1168,12 @@ TEST(CustomSnapshotDataBlobOutdatedContextWithOverflow) {
   }
   isolate->Dispose();
   delete[] data.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobWithLocker) {
   DisableAlwaysOpt();
   v8::Isolate::CreateParams create_params;
   create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
-  DisableEmbeddedBlobRefcounting();
   v8::Isolate* isolate0 = v8::Isolate::New(create_params);
   {
     v8::Locker locker(isolate0);
@@ -1211,7 +1207,6 @@ TEST(CustomSnapshotDataBlobWithLocker) {
   }
   isolate1->Dispose();
   delete[] data1.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobStackOverflow) {
@@ -1252,7 +1247,6 @@ TEST(CustomSnapshotDataBlobStackOverflow) {
   }
   isolate->Dispose();
   delete[] data.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 bool IsCompiled(const char* name) {
@@ -1289,7 +1283,6 @@ TEST(SnapshotDataBlobWithWarmup) {
   }
   isolate->Dispose();
   delete[] warm.data;
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobWithWarmup) {
@@ -1327,7 +1320,6 @@ TEST(CustomSnapshotDataBlobWithWarmup) {
   }
   isolate->Dispose();
   delete[] warm.data;
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobImmortalImmovableRoots) {
@@ -1359,7 +1351,6 @@ TEST(CustomSnapshotDataBlobImmortalImmovableRoots) {
   isolate->Dispose();
   source.Dispose();
   delete[] data.data;  // We can dispose of the snapshot blob now.
-  FreeCurrentEmbeddedBlob();
 }
 
 TEST(TestThatAlwaysSucceeds) {

test/inspector/inspector-test.cc

@@ -21,15 +21,6 @@
 #include "test/inspector/isolate-data.h"
 #include "test/inspector/task-runner.h"
 
-namespace v8 {
-namespace internal {
-
-extern void DisableEmbeddedBlobRefcounting();
-extern void FreeCurrentEmbeddedBlob();
-
-}  // namespace internal
-}  // namespace v8
-
 namespace {
 
 std::vector<TaskRunner*> task_runners;
@@ -1083,7 +1074,6 @@ int main(int argc, char* argv[]) {
   v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
   v8::V8::InitializeExternalStartupData(argv[0]);
   v8::V8::Initialize();
-  i::DisableEmbeddedBlobRefcounting();
 
   v8::base::Semaphore ready_semaphore(0);
 
@@ -1097,56 +1087,49 @@ int main(int argc, char* argv[]) {
     }
   }
 
-  {
-    IsolateData::SetupGlobalTasks frontend_extensions;
-    frontend_extensions.emplace_back(new UtilsExtension());
-    TaskRunner frontend_runner(std::move(frontend_extensions), true,
-                               &ready_semaphore, nullptr, false);
-    ready_semaphore.Wait();
-
-    int frontend_context_group_id = 0;
-    RunSyncTask(&frontend_runner,
-                [&frontend_context_group_id](IsolateData* data) {
-                  frontend_context_group_id = data->CreateContextGroup();
-                });
+  IsolateData::SetupGlobalTasks frontend_extensions;
+  frontend_extensions.emplace_back(new UtilsExtension());
+  TaskRunner frontend_runner(std::move(frontend_extensions), true,
+                             &ready_semaphore, nullptr, false);
+  ready_semaphore.Wait();
 
-    IsolateData::SetupGlobalTasks backend_extensions;
-    backend_extensions.emplace_back(new SetTimeoutExtension());
-    backend_extensions.emplace_back(new InspectorExtension());
-    TaskRunner backend_runner(
-        std::move(backend_extensions), false, &ready_semaphore,
-        startup_data.data ? &startup_data : nullptr, true);
-    ready_semaphore.Wait();
-    UtilsExtension::set_backend_task_runner(&backend_runner);
-
-    task_runners.push_back(&frontend_runner);
-    task_runners.push_back(&backend_runner);
-
-    for (int i = 1; i < argc; ++i) {
-      // Ignore unknown flags.
-      if (argv[i] == nullptr || argv[i][0] == '-') continue;
-
-      bool exists = false;
-      std::string chars = v8::internal::ReadFile(argv[i], &exists, true);
-      if (!exists) {
-        fprintf(stderr, "Internal error: script file doesn't exists: %s\n",
-                argv[i]);
-        Exit();
-      }
-      frontend_runner.Append(
-          new ExecuteStringTask(chars, frontend_context_group_id));
-    }
+  int frontend_context_group_id = 0;
+  RunSyncTask(&frontend_runner,
+              [&frontend_context_group_id](IsolateData* data) {
+                frontend_context_group_id = data->CreateContextGroup();
+              });
+
+  IsolateData::SetupGlobalTasks backend_extensions;
+  backend_extensions.emplace_back(new SetTimeoutExtension());
+  backend_extensions.emplace_back(new InspectorExtension());
+  TaskRunner backend_runner(std::move(backend_extensions), false,
+                            &ready_semaphore,
+                            startup_data.data ? &startup_data : nullptr, true);
+  ready_semaphore.Wait();
+  UtilsExtension::set_backend_task_runner(&backend_runner);
 
-    frontend_runner.Join();
-    backend_runner.Join();
+  task_runners.push_back(&frontend_runner);
+  task_runners.push_back(&backend_runner);
 
-    UtilsExtension::ClearAllSessions();
-    delete startup_data.data;
+  for (int i = 1; i < argc; ++i) {
+    // Ignore unknown flags.
+    if (argv[i] == nullptr || argv[i][0] == '-') continue;
 
-    // TaskRunners go out of scope here, which causes Isolate teardown and all
-    // running background tasks to be properly joined.
+    bool exists = false;
+    std::string chars = v8::internal::ReadFile(argv[i], &exists, true);
+    if (!exists) {
+      fprintf(stderr, "Internal error: script file doesn't exists: %s\n",
+              argv[i]);
+      Exit();
+    }
+    frontend_runner.Append(
+        new ExecuteStringTask(chars, frontend_context_group_id));
   }
 
-  i::FreeCurrentEmbeddedBlob();
+  frontend_runner.Join();
+  backend_runner.Join();
+
+  delete startup_data.data;
+  UtilsExtension::ClearAllSessions();
   return 0;
 }