[builtins] Enable embedded builtins and add testing variants

This enables the v8_enable_embedded_builtins gn flag on non-ia32 builds
and adds a new --stress-off-heap-code test mode to fyi bots.

v8_enable_embedded_builtins=true changes accesses to constants and
external references to go through the root list in builtins code.

--stress-off-heap-code copies builtins code off-heap on isolate
creation.

A few drive-by-fixes:
- ensure that we actually inspect the correct builtin during
  isolate-independence testing.
- relax tests to decrease maintenance (now we only fail if a builtin
  should be isolate-independent but isn't).
- switch to a different off-heap-trampoline register on arm due to
  conflicts with custom stub linkages.

Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel_ng
Bug: v8:6666
Change-Id: I09ad3c75cb4342f4c548ea780f275993730896c8
Reviewed-on: https://chromium-review.googlesource.com/934281
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Hablich <hablich@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51717}

BUILD.gn

@@ -65,8 +65,8 @@ declare_args() {
   v8_enable_fast_mksnapshot = false
 
   # Enable embedded builtins.
-  # TODO(jgruber,v8:6666): Support ia32.
-  v8_enable_embedded_builtins = false
+  # TODO(jgruber,v8:6666): Support ia32 and maybe MSVC.
+  v8_enable_embedded_builtins = v8_current_cpu != "x86" && (!is_win || is_clang)
 
   # Enable code-generation-time checking of types in the CodeStubAssembler.
   v8_enable_verify_csa = false

infra/testing/client.v8.pyl

@@ -36,12 +36,14 @@
   'V8 Linux64 - fyi': [
     {'name': 'v8testing', 'variant': 'infra_staging', 'shards': 1},
     {'name': 'test262_variants', 'variant': 'infra_staging', 'shards': 2},
+    {'name': 'v8testing', 'variant': 'stress_off_heap_code', 'shards': 1},
     {'name': 'mjsunit', 'variant': 'stress_sampling', 'shards': 1},
     {'name': 'webkit', 'variant': 'stress_sampling', 'shards': 1},
   ],
   'V8 Linux64 - debug - fyi': [
     {'name': 'v8testing', 'variant': 'infra_staging', 'shards': 2},
     {'name': 'test262_variants', 'variant': 'infra_staging', 'shards': 3},
+    {'name': 'v8testing', 'variant': 'stress_off_heap_code', 'shards': 1},
     {'name': 'mjsunit', 'variant': 'stress_sampling', 'shards': 1},
     {'name': 'webkit', 'variant': 'stress_sampling', 'shards': 1},
   ],

infra/testing/tryserver.v8.pyl

@@ -11,6 +11,7 @@
   'v8_linux64_fyi_rel_ng_triggered': [
     {'name': 'v8testing', 'variant': 'infra_staging', 'shards': 2},
     {'name': 'test262_variants', 'variant': 'infra_staging', 'shards': 2},
+    {'name': 'v8testing', 'variant': 'stress_off_heap_code', 'shards': 1},
     {'name': 'mjsunit', 'variant': 'stress_sampling', 'shards': 1},
     {'name': 'webkit', 'variant': 'stress_sampling', 'shards': 1},
   ],

src/arm/macro-assembler-arm.h

@@ -28,7 +28,7 @@ constexpr Register kInterpreterDispatchTableRegister = r8;
 constexpr Register kJavaScriptCallArgCountRegister = r0;
 constexpr Register kJavaScriptCallCodeStartRegister = r2;
 constexpr Register kJavaScriptCallNewTargetRegister = r3;
-constexpr Register kOffHeapTrampolineRegister = r4;
+constexpr Register kOffHeapTrampolineRegister = r6;
 constexpr Register kRuntimeCallFunctionRegister = r1;
 constexpr Register kRuntimeCallArgCountRegister = r0;
 

src/isolate.cc

@@ -2671,7 +2671,6 @@ void Isolate::Deinit() {
   ClearSerializerData();
 
   for (InstructionStream* stream : off_heap_code_) {
-    CHECK(FLAG_stress_off_heap_code);
     delete stream;
   }
   off_heap_code_.clear();

test/cctest/test-isolate-independent-builtins.cc

@@ -20,12 +20,31 @@ namespace internal {
 namespace test_isolate_independent_builtins {
 
 #ifdef V8_EMBEDDED_BUILTINS
-TEST(VerifyBuiltinsIsolateIndependence) {
-  Isolate* isolate = CcTest::i_isolate();
+UNINITIALIZED_TEST(VerifyBuiltinsIsolateIndependence) {
+  FLAG_stress_off_heap_code = false;  // Disable off-heap trampolines.
+
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* v8_isolate = v8::Isolate::New(create_params);
+
+  {
+    v8::Isolate::Scope isolate_scope(v8_isolate);
+    v8::internal::Isolate* isolate =
+        reinterpret_cast<v8::internal::Isolate*>(v8_isolate);
     HandleScope handle_scope(isolate);
 
     Snapshot::EnsureAllBuiltinsAreDeserialized(isolate);
 
+    // TODO(jgruber,v8:6666): Investigate CONST_POOL and VENEER_POOL kinds.
+    // CONST_POOL is currently relevant on {arm,arm64,mips,mips64,ppc,s390}.
+    //            Rumors are it will also become relevant on x64. My
+    //            understanding is that we should be fine if we ensure it
+    //            doesn't contain heap constants and we use pc-relative
+    //            addressing.
+    // VENEER_POOL is arm64-only. From what I've seen, jumps are pc-relative
+    //             and stay within the same code object and thus should be
+    //             isolate-independent.
+
     // Build a white-list of all isolate-independent RelocInfo entry kinds.
     constexpr int all_real_modes_mask =
         (1 << (RelocInfo::LAST_REAL_RELOC_MODE + 1)) - 1;
@@ -56,7 +75,8 @@ TEST(VerifyBuiltinsIsolateIndependence) {
       Code* code = isolate->builtins()->builtin(i);
 
       if (kVerbose) {
-      printf("%s %s\n", Builtins::KindNameOf(i), isolate->builtins()->name(i));
+        printf("%s %s\n", Builtins::KindNameOf(i),
+               isolate->builtins()->name(i));
       }
 
       bool is_isolate_independent = true;
@@ -71,20 +91,35 @@ TEST(VerifyBuiltinsIsolateIndependence) {
 #endif
       }
 
-    const bool expected_result = Builtins::IsIsolateIndependent(i);
-    if (is_isolate_independent != expected_result) {
+      // Relaxed condition only checks whether the isolate-independent list is
+      // valid, not whether it is complete. This is to avoid constant work
+      // updating the list.
+      bool should_be_isolate_independent = Builtins::IsIsolateIndependent(i);
+      if (should_be_isolate_independent && !is_isolate_independent) {
         found_mismatch = true;
         printf("%s %s expected: %d, is: %d\n", Builtins::KindNameOf(i),
-             isolate->builtins()->name(i), expected_result,
+               isolate->builtins()->name(i), should_be_isolate_independent,
                is_isolate_independent);
       }
     }
 
     CHECK(!found_mismatch);
+  }
+
+  v8_isolate->Dispose();
 }
 
-TEST(VerifyBuiltinsOffHeapSafety) {
-  Isolate* isolate = CcTest::i_isolate();
+UNINITIALIZED_TEST(VerifyBuiltinsOffHeapSafety) {
+  FLAG_stress_off_heap_code = false;  // Disable off-heap trampolines.
+
+  v8::Isolate::CreateParams create_params;
+  create_params.array_buffer_allocator = CcTest::array_buffer_allocator();
+  v8::Isolate* v8_isolate = v8::Isolate::New(create_params);
+
+  {
+    v8::Isolate::Scope isolate_scope(v8_isolate);
+    v8::internal::Isolate* isolate =
+        reinterpret_cast<v8::internal::Isolate*>(v8_isolate);
     HandleScope handle_scope(isolate);
 
     Snapshot::EnsureAllBuiltinsAreDeserialized(isolate);
@@ -118,7 +153,8 @@ TEST(VerifyBuiltinsOffHeapSafety) {
       Code* code = isolate->builtins()->builtin(i);
 
       if (kVerbose) {
-      printf("%s %s\n", Builtins::KindNameOf(i), isolate->builtins()->name(i));
+        printf("%s %s\n", Builtins::KindNameOf(i),
+               isolate->builtins()->name(i));
       }
 
       bool is_off_heap_safe = true;
@@ -136,15 +172,22 @@ TEST(VerifyBuiltinsOffHeapSafety) {
       // trampoline.
       if (Builtins::IsTooShortForOffHeapTrampoline(i)) is_off_heap_safe = false;
 
-    const bool expected_result = Builtins::IsOffHeapSafe(i);
-    if (is_off_heap_safe != expected_result) {
+      // Relaxed condition only checks whether the off-heap-safe list is
+      // valid, not whether it is complete. This is to avoid constant work
+      // updating the list.
+      bool should_be_off_heap_safe = Builtins::IsOffHeapSafe(i);
+      if (should_be_off_heap_safe && !is_off_heap_safe) {
         found_mismatch = true;
         printf("%s %s expected: %d, is: %d\n", Builtins::KindNameOf(i),
-             isolate->builtins()->name(i), expected_result, is_off_heap_safe);
+               isolate->builtins()->name(i), should_be_off_heap_safe,
+               is_off_heap_safe);
       }
     }
 
     CHECK(!found_mismatch);
+  }
+
+  v8_isolate->Dispose();
 }
 #endif  // V8_EMBEDDED_BUILTINS
 

tools/testrunner/local/variants.py

@@ -19,6 +19,7 @@ ALL_VARIANT_FLAGS = {
   "stress": [["--stress-opt", "--always-opt"]],
   "stress_background_compile": [["--background-compile", "--stress-background-compile"]],
   "stress_incremental_marking":  [["--stress-incremental-marking"]],
+  "stress_off_heap_code":  [["--stress-off-heap-code"]],
   # Trigger stress sampling allocation profiler with sample interval = 2^14
   "stress_sampling": [["--stress-sampling-allocation-profiler=16384"]],
   "trusted": [["--no-untrusted-code-mitigations"]],