Reland "[builtins] Load external references from the external-reference-table"

This is a reland of 9afde91b

Original change's description:
> [builtins] Load external references from the external-reference-table
>
> Off-heap code cannot embed external references. With this CL, we load
> from the external reference table (reached through the root pointer)
> instead.
>
> In a follow-up, the table could be stored within the isolate itself,
> removing one more level of indirection.
>
> Bug: v8:6666
> Change-Id: I4c612ad3d4112ec03c3b389f5bfb9cdc3dc8a671
> Reviewed-on: https://chromium-review.googlesource.com/970468
> Commit-Queue: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#52073}

TBR=mstarzinger@chromium.org

Bug: v8:6666, v8:7580
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I30639fe17ea345119d38a176a29d521c4b1904cb
Reviewed-on: https://chromium-review.googlesource.com/975241
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52141}

src/builtins/constants-table-builder.cc

@@ -46,25 +46,6 @@ uint32_t BuiltinsConstantsTableBuilder::AddObject(Handle<Object> object) {
   }
 }
 
-uint32_t BuiltinsConstantsTableBuilder::AddExternalReference(
-    ExternalReference reference) {
-  // Not yet finalized.
-  DCHECK_EQ(isolate_->heap()->empty_fixed_array(),
-            isolate_->heap()->builtins_constants_table());
-
-  auto maybe_entry = external_reference_map_.find(reference.address());
-  if (maybe_entry == external_reference_map_.end()) {
-    HandleScope handle_scope(isolate_);
-    Handle<Foreign> object =
-        isolate_->factory()->NewForeign(reference.address(), TENURED);
-    uint32_t index = AddObject(object);
-    external_reference_map_.emplace(reference.address(), index);
-    return index;
-  } else {
-    return maybe_entry->second;
-  }
-}
-
 void BuiltinsConstantsTableBuilder::Finalize() {
   HandleScope handle_scope(isolate_);
 

src/builtins/constants-table-builder.h

@@ -5,8 +5,6 @@
 #ifndef V8_BUILTINS_CONSTANTS_TABLE_BUILDER_H_
 #define V8_BUILTINS_CONSTANTS_TABLE_BUILDER_H_
 
-#include <unordered_map>
-
 #include "src/allocation.h"
 #include "src/base/macros.h"
 #include "src/handles.h"
@@ -30,10 +28,6 @@ class BuiltinsConstantsTableBuilder final {
   // object, possibly adding the object to the table. Objects are deduplicated.
   uint32_t AddObject(Handle<Object> object);
 
-  // External references can also be added, and end up as a Foreign object in
-  // the constants table.
-  uint32_t AddExternalReference(ExternalReference reference);
-
   // Should be called after all affected code (e.g. builtins and bytecode
   // handlers) has been generated.
   void Finalize();
@@ -45,10 +39,6 @@ class BuiltinsConstantsTableBuilder final {
   typedef IdentityMap<uint32_t, FreeStoreAllocationPolicy> ConstantsMap;
   ConstantsMap map_;
 
-  // Maps external references to corresponding indices within the constants
-  // list. Note that external references are also in map_ as Foreign objects.
-  std::unordered_map<Address, uint32_t> external_reference_map_;
-
   DISALLOW_COPY_AND_ASSIGN(BuiltinsConstantsTableBuilder)
 };
 

src/compiler/code-assembler.cc

@@ -21,6 +21,7 @@
 #include "src/machine-type.h"
 #include "src/macro-assembler.h"
 #include "src/objects-inl.h"
+#include "src/snapshot/serializer-common.h"
 #include "src/utils.h"
 #include "src/zone/zone.h"
 
@@ -261,37 +262,36 @@ TNode<HeapObject> CodeAssembler::LookupConstant(Handle<HeapObject> object) {
       Load(MachineType::AnyTagged(), builtins_constants_table, offset));
 }
 
-// External references are stored on the builtins constants list, wrapped in
-// Foreign objects.
-// TODO(jgruber,v8:6666): A more efficient solution.
+// External references are stored in the external reference table.
 TNode<ExternalReference> CodeAssembler::LookupExternalReference(
     ExternalReference reference) {
   DCHECK(isolate()->serializer_enabled());
 
-  // Ensure the given object is in the builtins constants table and fetch its
-  // index.
-  BuiltinsConstantsTableBuilder* builder =
-      isolate()->builtins_constants_table_builder();
-  uint32_t index = builder->AddExternalReference(reference);
+  // Encode as an index into the external reference table stored on the isolate.
 
-  // The builtins constants table is loaded through the root register on all
-  // supported platforms. This is checked by the
-  // VerifyBuiltinsIsolateIndependence cctest, which disallows embedded objects
-  // in isolate-independent builtins.
-  DCHECK(isolate()->heap()->RootCanBeTreatedAsConstant(
-      Heap::kBuiltinsConstantsTableRootIndex));
-  TNode<FixedArray> builtins_constants_table = UncheckedCast<FixedArray>(
-      LoadRoot(Heap::kBuiltinsConstantsTableRootIndex));
+  ExternalReferenceEncoder encoder(isolate());
+  ExternalReferenceEncoder::Value v = encoder.Encode(reference.address());
+  CHECK(!v.is_from_api());
+  uint32_t index = v.index();
 
-  // Generate the lookup.
-  const int32_t header_size = FixedArray::kHeaderSize - kHeapObjectTag;
-  TNode<IntPtrT> offset = IntPtrConstant(header_size + kPointerSize * index);
-  TNode<Object> foreign = UncheckedCast<HeapObject>(
-      Load(MachineType::AnyTagged(), builtins_constants_table, offset));
+  // Generate code to load from the external reference table.
+  // TODO(jgruber,v8:6666): If the external reference points within the isolate,
+  // we could return kRootPointer + offset without loading through the table.
+
+  TNode<IntPtrT> roots_ptr = LoadRootsPointer();
+
+  static const int roots_to_isolate_offset =
+#ifdef V8_TARGET_ARCH_X64
+      Heap::roots_to_external_reference_table_offset() - kRootRegisterBias;
+#else
+      Heap::roots_to_external_reference_table_offset();
+#endif
 
+  Node* external_refs_ptr = Load(MachineType::Pointer(), roots_ptr,
+                                 IntPtrConstant(roots_to_isolate_offset));
   return UncheckedCast<ExternalReference>(
-      Load(MachineType::Pointer(), foreign,
-           IntPtrConstant(Foreign::kForeignAddressOffset - kHeapObjectTag)));
+      Load(MachineType::Pointer(), external_refs_ptr,
+           IntPtrConstant(ExternalReferenceTable::OffsetOfEntry(index))));
 }
 #endif  // V8_EMBEDDED_BUILTINS
 

src/external-reference-table.h

@@ -51,6 +51,12 @@ class ExternalReferenceTable {
 
   static const char* ResolveSymbol(void* address);
 
+  static uint32_t OffsetOfEntry(uint32_t i) {
+    // Used in CodeAssembler::LookupExternalReference.
+    STATIC_ASSERT(offsetof(ExternalReferenceEntry, address) == 0);
+    return i * sizeof(ExternalReferenceEntry);
+  }
+
  private:
   struct ExternalReferenceEntry {
     Address address;

src/heap/heap.cc

@@ -5969,6 +5969,9 @@ void Heap::TearDown() {
     }
   }
 
+  delete external_reference_table_;
+  external_reference_table_ = nullptr;
+
   new_space()->RemoveAllocationObserver(idle_scavenge_observer_);
   delete idle_scavenge_observer_;
   idle_scavenge_observer_ = nullptr;

src/heap/heap.h

@@ -43,6 +43,7 @@ class BoilerplateDescription;
 class BytecodeArray;
 class CodeDataContainer;
 class DeoptimizationData;
+class ExternalReferenceTable;
 class HandlerTable;
 class IncrementalMarking;
 class JSArrayBuffer;
@@ -1092,6 +1093,10 @@ class Heap {
   // Generated code can embed this address to get access to the roots.
   Object** roots_array_start() { return roots_; }
 
+  static constexpr int roots_to_external_reference_table_offset() {
+    return kRootsExternalReferenceTableOffset;
+  }
+
   // Sets the stub_cache_ (only used when expanding the dictionary).
   void SetRootCodeStubs(SimpleNumberDictionary* value);
 
@@ -2399,6 +2404,13 @@ class Heap {
 
   Object* roots_[kRootListLength];
 
+  // This table is accessed from builtin code compiled into the snapshot, and
+  // thus its offset from roots_ must remain static. This is verified in
+  // Isolate::Init() using runtime checks.
+  static constexpr int kRootsExternalReferenceTableOffset =
+      kRootListLength * kPointerSize;
+  ExternalReferenceTable* external_reference_table_ = nullptr;
+
   size_t code_range_size_;
   size_t max_semi_space_size_;
   size_t initial_semispace_size_;

src/isolate.cc

@@ -2586,8 +2586,6 @@ void Isolate::GlobalTearDown() {
 
 
 void Isolate::ClearSerializerData() {
-  delete external_reference_table_;
-  external_reference_table_ = nullptr;
   delete external_reference_map_;
   external_reference_map_ = nullptr;
 }
@@ -3097,6 +3095,10 @@ bool Isolate::Init(StartupDeserializer* des) {
   CHECK_EQ(static_cast<int>(
                OFFSET_OF(Isolate, heap_.external_memory_at_last_mark_compact_)),
            Internals::kExternalMemoryAtLastMarkCompactOffset);
+  CHECK_EQ(
+      static_cast<int>(OFFSET_OF(Isolate, heap_.external_reference_table_)),
+      Internals::kIsolateRootsOffset +
+          Heap::kRootsExternalReferenceTableOffset);
 
   {
     HandleScope scope(this);

src/isolate.h

@@ -422,7 +422,6 @@ typedef std::vector<HeapObject*> DebugObjectCache;
   V(Relocatable*, relocatable_top, nullptr)                                   \
   V(DebugObjectCache*, string_stream_debug_object_cache, nullptr)             \
   V(Object*, string_stream_current_security_token, nullptr)                   \
-  V(ExternalReferenceTable*, external_reference_table, nullptr)               \
   V(const intptr_t*, api_external_references, nullptr)                        \
   V(AddressToIndexHashMap*, external_reference_map, nullptr)                  \
   V(HeapObjectToIndexHashMap*, root_index_map, nullptr)                       \
@@ -863,6 +862,14 @@ class Isolate {
   ISOLATE_INIT_LIST(GLOBAL_ACCESSOR)
 #undef GLOBAL_ACCESSOR
 
+  ExternalReferenceTable* external_reference_table() const {
+    return heap_.external_reference_table_;
+  }
+
+  void set_external_reference_table(ExternalReferenceTable* v) {
+    heap_.external_reference_table_ = v;
+  }
+
 #define GLOBAL_ARRAY_ACCESSOR(type, name, length)                       \
   inline type* name() {                                                 \
     DCHECK(OFFSET_OF(Isolate, name##_) == name##_debug_offset_);        \