[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/1310195Reviewed-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}

BUILD.gn

@@ -24,6 +24,8 @@ 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
@@ -83,9 +85,8 @@ declare_args() {
   v8_enable_fast_mksnapshot = false
 
   # Enable embedded builtins.
-  # TODO(v8:6666): Support no-snapshot builds, aix and MSVC.
-  v8_enable_embedded_builtins =
-      v8_use_snapshot && !is_aix && (!is_win || is_clang)
+  # TODO(v8:6666): Support aix and MSVC.
+  v8_enable_embedded_builtins = !is_aix && !is_msvc
 
   # Enable code-generation-time checking of types in the CodeStubAssembler.
   v8_enable_verify_csa = false
@@ -215,8 +216,6 @@ 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,17 +65,20 @@ AssemblerOptions AssemblerOptions::EnableV8AgnosticCode() const {
 AssemblerOptions AssemblerOptions::Default(
     Isolate* isolate, bool explicitly_support_serialization) {
   AssemblerOptions options;
-  bool serializer =
+  const 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;
+  options.enable_root_array_delta_access =
+      !serializer && !generating_embedded_builtin;
 #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;
-
+  options.inline_offheap_trampolines =
+      !serializer && !generating_embedded_builtin;
 #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,7 +296,6 @@ 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 serializing.
-  DCHECK(isolate_->serializer_enabled());
+  // Must be generating embedded builtin code.
+  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
 #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_->serializer_enabled());
+  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
 
   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_->serializer_enabled());
+  DCHECK(isolate_->ShouldLoadConstantsFromRootList());
 
   // An empty map means there's nothing to do.
   if (map_.size() == 0) return;

src/compiler/pipeline.cc

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

src/heap/factory.cc

@@ -2681,7 +2681,6 @@ 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,24 +108,103 @@ 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
-  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());
-  }
+  // 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_; }
 
@@ -2723,17 +2802,7 @@ Isolate::Isolate(std::unique_ptr<i::IsolateAllocator> isolate_allocator)
   InitializeLoggingAndCounters();
   debug_ = new Debug(this);
 
-  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
-  }
+  InitializeDefaultEmbeddedBlob();
 }
 
 void Isolate::CheckIsolateLayout() {
@@ -2836,14 +2905,7 @@ void Isolate::Deinit() {
     wasm_engine_.reset();
   }
 
-  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_);
-    }
-  }
+  TearDownEmbeddedBlob();
 
   delete interpreter_;
   interpreter_ = nullptr;
@@ -3015,7 +3077,6 @@ 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());
 
@@ -3047,23 +3108,75 @@ void CreateOffHeapTrampolines(Isolate* isolate) {
 }
 }  // namespace
 
-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);
+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);
+  }
+
   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);
 
@@ -3166,19 +3279,16 @@ bool Isolate::Init(StartupDeserializer* des) {
 
   bootstrapper_->Initialize(create_heap_objects);
 
-  if (FLAG_embedded_builtins) {
-    if (create_heap_objects && serializer_enabled()) {
-      builtins_constants_table_builder_ =
-          new BuiltinsConstantsTableBuilder(this);
-    }
+  if (FLAG_embedded_builtins && create_heap_objects) {
+    builtins_constants_table_builder_ = new BuiltinsConstantsTableBuilder(this);
   }
   setup_delegate_->SetupBuiltins(this);
-  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 (FLAG_embedded_builtins && create_heap_objects) {
+    builtins_constants_table_builder_->Finalize();
+    delete builtins_constants_table_builder_;
+    builtins_constants_table_builder_ = nullptr;
+
+    CreateAndSetEmbeddedBlob();
   }
 
   if (create_heap_objects) heap_.CreateFixedStubs();

src/isolate.h

@@ -335,6 +335,11 @@ 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
@@ -1462,20 +1467,12 @@ 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 {
-    if (FLAG_embedded_builtins) {
-      return (serializer_enabled() &&
-              builtins_constants_table_builder() != nullptr);
-    } else {
-      return false;
-    }
+    return FLAG_embedded_builtins &&
+           builtins_constants_table_builder() != nullptr;
   }
 
-  // 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_;
   }
@@ -1863,7 +1860,12 @@ 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,9 +418,7 @@ v8::StartupData WarmUpSnapshotDataBlob(v8::SnapshotCreator* snapshot_creator,
   return result;
 }
 
-void WriteEmbeddedFile(v8::SnapshotCreator* creator, SnapshotWriter* writer) {
-  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(creator->GetIsolate());
-  isolate->PrepareEmbeddedBlobForSerialization();
+void WriteEmbeddedFile(SnapshotWriter* writer) {
   i::EmbeddedData embedded_blob = i::EmbeddedData::FromBlob();
   writer->WriteEmbedded(&embedded_blob);
 }
@@ -463,6 +461,7 @@ 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();
@@ -478,13 +477,7 @@ 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) {
-        // 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);
-      }
+      if (i::FLAG_embedded_builtins) WriteEmbeddedFile(&writer);
       blob = CreateSnapshotDataBlob(&snapshot_creator, embed_script.get());
     }
 
@@ -492,9 +485,6 @@ 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;
     }
@@ -503,6 +493,7 @@ 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,6 +176,13 @@
   '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,6 +14582,8 @@ class SetFunctionEntryHookTest {
   SymbolLocationMap symbol_locations_;
   InvocationMap invocations_;
 
+  i::Isolate* isolate_ = nullptr;
+
   static SetFunctionEntryHookTest* instance_;
 };
 SetFunctionEntryHookTest* SetFunctionEntryHookTest::instance_ = nullptr;
@@ -14668,8 +14670,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 =
-      i::Code::GetCodeFromTargetAddress(static_cast<i::Address>(function));
+  i::Code function_code = isolate_->heap()->GcSafeFindCodeForInnerPointer(
+      static_cast<i::Address>(function));
   CHECK(!function_code.is_null());
 
   // Then try and look up the caller's code object.
@@ -14795,7 +14797,9 @@ 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::New(create_params);
+  v8::Isolate* isolate = v8::Isolate::Allocate();
+  isolate_ = reinterpret_cast<i::Isolate*>(isolate);
+  v8::Isolate::Initialize(isolate, create_params);
 
   {
     v8::Isolate::Scope scope(isolate);
@@ -14838,7 +14842,9 @@ 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::New(create_params);
+  isolate = v8::Isolate::Allocate();
+  isolate_ = reinterpret_cast<i::Isolate*>(isolate);
+  v8::Isolate::Initialize(isolate, create_params);
   {
     v8::Isolate::Scope scope(isolate);
 
@@ -14852,8 +14858,7 @@ void SetFunctionEntryHookTest::RunTest() {
   isolate->Dispose();
 }
 
-
-TEST(SetFunctionEntryHook) {
+UNINITIALIZED_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,6 +85,7 @@ 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);
@@ -127,25 +128,10 @@ 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);
@@ -180,6 +166,7 @@ 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;
@@ -316,6 +303,7 @@ void TestStartupSerializerOnceImpl() {
   }
   isolate->Dispose();
   blobs.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerOnce) {
@@ -419,6 +407,7 @@ UNINITIALIZED_TEST(StartupSerializerTwice) {
   }
   isolate->Dispose();
   blobs2.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerOnceRunScript) {
@@ -444,6 +433,7 @@ UNINITIALIZED_TEST(StartupSerializerOnceRunScript) {
   }
   isolate->Dispose();
   blobs.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 UNINITIALIZED_TEST(StartupSerializerTwiceRunScript) {
@@ -470,6 +460,7 @@ UNINITIALIZED_TEST(StartupSerializerTwiceRunScript) {
   }
   isolate->Dispose();
   blobs2.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 static void PartiallySerializeContext(Vector<const byte>* startup_blob_out,
@@ -576,6 +567,7 @@ UNINITIALIZED_TEST(PartialSerializerContext) {
   }
   v8_isolate->Dispose();
   blobs.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 static void PartiallySerializeCustomContext(
@@ -753,6 +745,7 @@ UNINITIALIZED_TEST(PartialSerializerCustomContext) {
   }
   v8_isolate->Dispose();
   blobs.Dispose();
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlob1) {
@@ -779,6 +772,7 @@ TEST(CustomSnapshotDataBlob1) {
   }
   isolate1->Dispose();
   delete[] data1.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(SnapshotChecksum) {
@@ -790,6 +784,7 @@ 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 {
@@ -845,6 +840,7 @@ void TypedArrayTestHelper(
     const Int32Expectations& after_restore_expectations = Int32Expectations()) {
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -886,6 +882,7 @@ void TypedArrayTestHelper(
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobWithOffHeapTypedArray) {
@@ -970,6 +967,7 @@ TEST(CustomSnapshotDataBlobNeuteredArrayBuffer) {
 
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -1014,6 +1012,7 @@ TEST(CustomSnapshotDataBlobNeuteredArrayBuffer) {
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 i::Handle<i::JSArrayBuffer> GetBufferFromTypedArray(
@@ -1040,6 +1039,7 @@ TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
 
   DisableAlwaysOpt();
   i::FLAG_allow_natives_syntax = true;
+  DisableEmbeddedBlobRefcounting();
   v8::StartupData blob;
   {
     v8::SnapshotCreator creator;
@@ -1088,6 +1088,7 @@ TEST(CustomSnapshotDataBlobOnOrOffHeapTypedArray) {
   }
   isolate->Dispose();
   delete[] blob.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlob2) {
@@ -1117,6 +1118,7 @@ TEST(CustomSnapshotDataBlob2) {
   }
   isolate2->Dispose();
   delete[] data2.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 static void SerializationFunctionTemplate(
@@ -1168,12 +1170,14 @@ 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);
@@ -1207,6 +1211,7 @@ TEST(CustomSnapshotDataBlobWithLocker) {
   }
   isolate1->Dispose();
   delete[] data1.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobStackOverflow) {
@@ -1247,6 +1252,7 @@ TEST(CustomSnapshotDataBlobStackOverflow) {
   }
   isolate->Dispose();
   delete[] data.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
 }
 
 bool IsCompiled(const char* name) {
@@ -1283,6 +1289,7 @@ TEST(SnapshotDataBlobWithWarmup) {
   }
   isolate->Dispose();
   delete[] warm.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobWithWarmup) {
@@ -1320,6 +1327,7 @@ TEST(CustomSnapshotDataBlobWithWarmup) {
   }
   isolate->Dispose();
   delete[] warm.data;
+  FreeCurrentEmbeddedBlob();
 }
 
 TEST(CustomSnapshotDataBlobImmortalImmovableRoots) {
@@ -1351,6 +1359,7 @@ 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,6 +21,15 @@
 #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;
@@ -1074,6 +1083,7 @@ 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);
 
@@ -1087,49 +1097,56 @@ 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();
+  {
+    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();
+                });
 
-  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);
+    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));
+    }
 
-  task_runners.push_back(&frontend_runner);
-  task_runners.push_back(&backend_runner);
+    frontend_runner.Join();
+    backend_runner.Join();
 
-  for (int i = 1; i < argc; ++i) {
-    // Ignore unknown flags.
-    if (argv[i] == nullptr || argv[i][0] == '-') continue;
+    UtilsExtension::ClearAllSessions();
+    delete startup_data.data;
 
-    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));
+    // TaskRunners go out of scope here, which causes Isolate teardown and all
+    // running background tasks to be properly joined.
   }
 
-  frontend_runner.Join();
-  backend_runner.Join();
-
-  delete startup_data.data;
-  UtilsExtension::ClearAllSessions();
+  i::FreeCurrentEmbeddedBlob();
   return 0;
 }