[snapshot] Don't restrict off-heap targets to builtin hosts

There's no reason to restrict off-heap targets to builtin host Code
objects during serialization. They can also occur e.g. in irregexp
code created by embedded scripts.

Drive-by: unify the list of reloc modes that have a
target_address_address.

Bug: v8:8572,v8:6666
Change-Id: I26dce735463b79677a7b7dcfdb604c5234b5f10b
Reviewed-on: https://chromium-review.googlesource.com/c/1371568Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58194}

src/arm/assembler-arm-inl.h

@@ -76,9 +76,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
   if (Assembler::IsMovW(Memory<int32_t>(pc_))) {
     return pc_;
   } else {

src/arm64/assembler-arm64-inl.h

@@ -656,9 +656,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
   Instruction* instr = reinterpret_cast<Instruction*>(pc_);
   // Read the address of the word containing the target_address in an
   // instruction stream.

src/ia32/assembler-ia32-inl.h

@@ -75,9 +75,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsWasmStubCall(rmode_) || IsEmbeddedObject(rmode_) ||
-         IsExternalReference(rmode_) || IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
   return pc_;
 }
 

src/mips/assembler-mips-inl.h

@@ -83,9 +83,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
   // Read the address of the word containing the target_address in an
   // instruction stream.
   // The only architecture-independent user of this function is the serializer.

src/mips64/assembler-mips64-inl.h

@@ -79,9 +79,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
   // Read the address of the word containing the target_address in an
   // instruction stream.
   // The only architecture-independent user of this function is the serializer.

src/ppc/assembler-ppc-inl.h

@@ -90,9 +90,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
 
   if (FLAG_enable_embedded_constant_pool &&
       Assembler::IsConstantPoolLoadStart(pc_)) {

src/reloc-info.cc

@@ -380,6 +380,23 @@ void RelocInfo::set_target_address(Address target,
   }
 }
 
+bool RelocInfo::HasTargetAddressAddress() const {
+  // TODO(jgruber): Investigate whether WASM_CALL is still appropriate on
+  // non-intel platforms now that wasm code is no longer on the heap.
+#if defined(V8_TARGET_ARCH_IA32) || defined(V8_TARGET_ARCH_X64)
+  static constexpr int kTargetAddressAddressModeMask =
+      ModeMask(CODE_TARGET) | ModeMask(EMBEDDED_OBJECT) |
+      ModeMask(EXTERNAL_REFERENCE) | ModeMask(OFF_HEAP_TARGET) |
+      ModeMask(RUNTIME_ENTRY) | ModeMask(WASM_CALL) | ModeMask(WASM_STUB_CALL);
+#else
+  static constexpr int kTargetAddressAddressModeMask =
+      ModeMask(CODE_TARGET) | ModeMask(EMBEDDED_OBJECT) |
+      ModeMask(EXTERNAL_REFERENCE) | ModeMask(OFF_HEAP_TARGET) |
+      ModeMask(RUNTIME_ENTRY) | ModeMask(WASM_CALL);
+#endif
+  return (ModeMask(rmode_) & kTargetAddressAddressModeMask) != 0;
+}
+
 bool RelocInfo::RequiresRelocationAfterCodegen(const CodeDesc& desc) {
   RelocIterator it(desc, RelocInfo::PostCodegenRelocationMask());
   return !it.done();

src/reloc-info.h

@@ -260,6 +260,7 @@ class RelocInfo {
   // output before the next target.  Architecture-independent code shouldn't
   // dereference the pointer it gets back from this.
   V8_INLINE Address target_address_address();
+  bool HasTargetAddressAddress() const;
 
   // This indicates how much space a target takes up when deserializing a code
   // stream.  For most architectures this is just the size of a pointer.  For

src/s390/assembler-s390-inl.h

@@ -97,9 +97,7 @@ Address RelocInfo::target_address() {
 }
 
 Address RelocInfo::target_address_address() {
-  DCHECK(IsCodeTarget(rmode_) || IsRuntimeEntry(rmode_) || IsWasmCall(rmode_) ||
-         IsEmbeddedObject(rmode_) || IsExternalReference(rmode_) ||
-         IsOffHeapTarget(rmode_));
+  DCHECK(HasTargetAddressAddress());
 
   // Read the address of the word containing the target_address in an
   // instruction stream.

src/snapshot/serializer.cc

@@ -806,45 +806,35 @@ void Serializer::ObjectSerializer::VisitRuntimeEntry(Code host,
 void Serializer::ObjectSerializer::VisitOffHeapTarget(Code host,
                                                       RelocInfo* rinfo) {
   DCHECK(FLAG_embedded_builtins);
-  {
-    STATIC_ASSERT(EmbeddedData::kTableSize == Builtins::builtin_count);
-    CHECK(Builtins::IsIsolateIndependentBuiltin(host));
-    Address addr = rinfo->target_off_heap_target();
-    CHECK_NE(kNullAddress, addr);
-    CHECK(!InstructionStream::TryLookupCode(serializer_->isolate(), addr)
-               .is_null());
-  }
+  STATIC_ASSERT(EmbeddedData::kTableSize == Builtins::builtin_count);
+
+  Address addr = rinfo->target_off_heap_target();
+  CHECK_NE(kNullAddress, addr);
+
+  Code target = InstructionStream::TryLookupCode(serializer_->isolate(), addr);
+  CHECK(Builtins::IsIsolateIndependentBuiltin(target));
 
   int skip = SkipTo(rinfo->target_address_address());
   sink_->Put(kOffHeapTarget, "OffHeapTarget");
   sink_->PutInt(skip, "SkipB4OffHeapTarget");
-  sink_->PutInt(host->builtin_index(), "builtin index");
+  sink_->PutInt(target->builtin_index(), "builtin index");
   bytes_processed_so_far_ += rinfo->target_address_size();
 }
 
 namespace {
+
 class CompareRelocInfo {
  public:
   bool operator()(RelocInfo x, RelocInfo y) {
     // Everything that does not use target_address_address will compare equal.
     Address x_num = 0;
     Address y_num = 0;
-    if (HasTargetAddressAddress(x.rmode())) {
-      x_num = x.target_address_address();
-    }
-    if (HasTargetAddressAddress(y.rmode())) {
-      y_num = y.target_address_address();
-    }
+    if (x.HasTargetAddressAddress()) x_num = x.target_address_address();
+    if (y.HasTargetAddressAddress()) y_num = y.target_address_address();
     return x_num > y_num;
   }
-
- private:
-  static bool HasTargetAddressAddress(RelocInfo::Mode mode) {
-    return RelocInfo::IsEmbeddedObject(mode) || RelocInfo::IsCodeTarget(mode) ||
-           RelocInfo::IsExternalReference(mode) ||
-           RelocInfo::IsRuntimeEntry(mode);
-  }
 };
+
 }  // namespace
 
 void Serializer::ObjectSerializer::VisitRelocInfo(RelocIterator* it) {

src/wasm/wasm-serialization.cc

@@ -133,6 +133,7 @@ void WriteVersion(Writer* writer) {
 // Other platforms simply require accessing the target address.
 void SetWasmCalleeTag(RelocInfo* rinfo, uint32_t tag) {
 #if V8_TARGET_ARCH_X64 || V8_TARGET_ARCH_IA32
+  DCHECK(rinfo->HasTargetAddressAddress());
   *(reinterpret_cast<uint32_t*>(rinfo->target_address_address())) = tag;
 #elif V8_TARGET_ARCH_ARM64
   Instruction* instr = reinterpret_cast<Instruction*>(rinfo->pc());

test/cctest/test-serialize.cc

@@ -818,6 +818,49 @@ UNINITIALIZED_TEST(CustomSnapshotDataBlobStringNotInternalized) {
   FreeCurrentEmbeddedBlob();
 }
 
+UNINITIALIZED_TEST(CustomSnapshotDataBlobWithIrregexpCode) {
+  DisableAlwaysOpt();
+  const char* source =
+      "var re = /\\/\\*[^*]*\\*+([^/*][^*]*\\*+)*\\//;\n"
+      "function f() { return '/* a comment */'.search(re); }\n"
+      "function g() { return 'not a comment'.search(re); }\n"
+      "function h() { return '// this is a comment'.search(re); }\n"
+      "f(); f(); g(); g();";
+
+  v8::StartupData data1 = CreateSnapshotDataBlob(source);
+
+  v8::Isolate::CreateParams params1;
+  params1.snapshot_blob = &data1;
+  params1.array_buffer_allocator = CcTest::array_buffer_allocator();
+
+  // Test-appropriate equivalent of v8::Isolate::New.
+  v8::Isolate* isolate1 = TestSerializer::NewIsolate(params1);
+  {
+    v8::Isolate::Scope i_scope(isolate1);
+    v8::HandleScope h_scope(isolate1);
+    v8::Local<v8::Context> context = v8::Context::New(isolate1);
+    v8::Context::Scope c_scope(context);
+    {
+      v8::Maybe<int32_t> result =
+          CompileRun("f()")->Int32Value(isolate1->GetCurrentContext());
+      CHECK_EQ(0, result.FromJust());
+    }
+    {
+      v8::Maybe<int32_t> result =
+          CompileRun("g()")->Int32Value(isolate1->GetCurrentContext());
+      CHECK_EQ(-1, result.FromJust());
+    }
+    {
+      v8::Maybe<int32_t> result =
+          CompileRun("h()")->Int32Value(isolate1->GetCurrentContext());
+      CHECK_EQ(-1, result.FromJust());
+    }
+  }
+  isolate1->Dispose();
+  delete[] data1.data;  // We can dispose of the snapshot blob now.
+  FreeCurrentEmbeddedBlob();
+}
+
 UNINITIALIZED_TEST(SnapshotChecksum) {
   DisableAlwaysOpt();
   const char* source1 = "function f() { return 42; }";