From a61aa4919ff7e9c14e8e47cdb447dd48913526dc Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Wed, 12 May 2021 08:42:06 -0700
Subject: [PATCH] [ptr-cage] Better support sharing CodeRange with re-embedded
 builtins

If a shared CodeRange is already allocated when creating an Isolate in
jitless mode, the CodeRange will be used. This is to better support the
following use pattern:

```
FLAG_jitless = false;
v8::Isolate::New();
FLAG_jitless = true;
v8::Isolate::New();
```

Note that the other direction of toggling jitless from true to false is
unsupported and may have undefined behavior.

Bug: v8:11460
Change-Id: I1c451c53bc160be4122056d8b309323a94d4b8b6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2890591
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74535}
---
 src/execution/isolate.cc           |  8 +++---
 test/cctest/test-ptr-compr-cage.cc | 43 ++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
index 5f1185f740..cadeec89cb 100644
--- a/src/execution/isolate.cc
+++ b/src/execution/isolate.cc
@@ -3657,8 +3657,7 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
     is_short_builtin_calls_enabled_ = (heap_.MaxOldGenerationSize() >=
                                        kShortBuiltinCallsOldSpaceSizeThreshold);
     if (COMPRESS_POINTERS_IN_SHARED_CAGE_BOOL) {
-      std::shared_ptr<CodeRange> code_range =
-          CodeRange::GetProcessWideCodeRange();
+      CodeRange* code_range = CodeRange::GetProcessWideCodeRange().get();
       if (code_range && code_range->embedded_blob_code_copy() != nullptr) {
         is_short_builtin_calls_enabled_ = true;
       }
@@ -4974,7 +4973,10 @@ void Isolate::AddCodeRange(Address begin, size_t length_in_bytes) {
 }
 
 bool Isolate::RequiresCodeRange() const {
-  return kPlatformRequiresCodeRange && !jitless_;
+  if (kPlatformRequiresCodeRange && !jitless_) return true;
+
+  return COMPRESS_POINTERS_IN_SHARED_CAGE_BOOL &&
+         CodeRange::GetProcessWideCodeRange().get() != nullptr;
 }
 
 v8::metrics::Recorder::ContextId Isolate::GetOrRegisterRecorderContextId(
diff --git a/test/cctest/test-ptr-compr-cage.cc b/test/cctest/test-ptr-compr-cage.cc
index e5be7fb29c..213c2285c6 100644
--- a/test/cctest/test-ptr-compr-cage.cc
+++ b/test/cctest/test-ptr-compr-cage.cc
@@ -101,6 +101,49 @@ UNINITIALIZED_TEST(SharedPtrComprCageCodeRange) {
   isolate2->Dispose();
 }
 
+UNINITIALIZED_TEST(SharedPtrComprCageRemappedBuiltinsJitlessFalseToTrue) {
+  // Testing that toggling jitless from false to true use the same re-embedded
+  // builtins. Toggling jitless from false to true with shared pointer
+  // compression cage is not supported.
+
+  if (!V8_SHORT_BUILTIN_CALLS_BOOL) return;
+  FLAG_short_builtin_calls = true;
+  FLAG_jitless = false;
+
+  constexpr uint64_t kMemoryGB = 4;
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  create_params.constraints.ConfigureDefaults(kMemoryGB * GB, kMemoryGB * GB);
+
+  v8::Isolate* isolate1 = v8::Isolate::New(create_params);
+  Isolate* i_isolate1 = reinterpret_cast<Isolate*>(isolate1);
+  v8::Isolate* isolate2 = v8::Isolate::New(create_params);
+  Isolate* i_isolate2 = reinterpret_cast<Isolate*>(isolate2);
+
+  CHECK_EQ(i_isolate1->embedded_blob_code(), i_isolate2->embedded_blob_code());
+  CodeRange* shared_code_range = CodeRange::GetProcessWideCodeRange().get();
+  if (shared_code_range &&
+      shared_code_range->embedded_blob_code_copy() != nullptr) {
+    CHECK_EQ(shared_code_range->embedded_blob_code_copy(),
+             i_isolate1->embedded_blob_code());
+    CHECK_EQ(shared_code_range->embedded_blob_code_copy(),
+             i_isolate2->embedded_blob_code());
+  }
+
+  FLAG_jitless = true;
+  v8::Isolate* isolate3 = v8::Isolate::New(create_params);
+  Isolate* i_isolate3 = reinterpret_cast<Isolate*>(isolate3);
+  if (shared_code_range &&
+      shared_code_range->embedded_blob_code_copy() != nullptr) {
+    CHECK_EQ(shared_code_range->embedded_blob_code_copy(),
+             i_isolate3->embedded_blob_code());
+  }
+
+  isolate1->Dispose();
+  isolate2->Dispose();
+  isolate3->Dispose();
+}
+
 namespace {
 constexpr int kIsolatesToAllocate = 25;