[vm/ffi] Simplify some aspects of ffi callback metadata implementation

Make the free and allocated lists simply linked lists of `Metadata*`
(instead of `Metadata*` pointing to Trampoline, which - via some
logic - can be translated into next `Metadata*`).

Make the layout of the virtual address space mapping and
offsets `constexpr` functions instead of computing them at
runtime & caching in fields.

Use `uword` to represent `Trampoline` entrypoint (as we generally
use `uword` for `Code.EntryPoint()` / ...)

TEST=ci

Change-Id: If4ffa11712acc46c9295b609caff7576d2354fe4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305983
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>

runtime/lib/ffi.cc

@@ -40,8 +40,7 @@ DEFINE_NATIVE_ENTRY(Ffi_asFunctionInternal, 2, 2) {
 
 DEFINE_NATIVE_ENTRY(Ffi_pointerFromFunction, 1, 1) {
   const auto& function = Function::CheckedHandle(zone, arguments->NativeArg0());
-  void* pointer = isolate->CreateSyncFfiCallback(zone, function);
-  return Pointer::New(reinterpret_cast<uword>(pointer));
+  return Pointer::New(isolate->CreateSyncFfiCallback(zone, function));
 }
 
 DEFINE_NATIVE_ENTRY(DartNativeApiFunctionPointer, 0, 1) {

runtime/vm/ffi_callback_metadata.cc

@@ -19,42 +19,18 @@ void FfiCallbackMetadata::EnsureStubPageLocked() {
     return;
   }
 
-  // Keep in sync with GenerateLoadFfiCallbackMetadataRuntimeFunction.
-
-  // The FfiCallbackTrampoline stub is designed to take up 1 page of memory. At
-  // the moment it's not aligned though, so we need to do some alignment math
-  // here. So when we duplicate it below, we're wasting some memory because the
-  // stub probably straddles 2 aligned pages. It would be better to align the
-  // stub inside the stub code compiler, but we don't have a way of doing that
-  // at the moment.
-  // TODO(https://dartbug.com/52498): Align the stub.
-
-  // |      page       |      page     |                pages               |
-  // [ alignment ][ stub ][ alignment ][ functions ][ metadata ][ alignment ]
   ASSERT_LESS_OR_EQUAL(VirtualMemory::PageSize(), kPageSize);
+
   const Code& trampoline_code = StubCode::FfiCallbackTrampoline();
-
   const uword code_start = trampoline_code.EntryPoint();
-  const uword page_start = Utils::RoundDown(code_start, kPageSize);
-  const uword code_end_aligned = page_start + 2 * kPageSize;
-  ASSERT_LESS_OR_EQUAL(code_start + trampoline_code.Size(), code_end_aligned);
+  const uword page_start = code_start & kPageMask;
+  offset_of_first_trampoline_in_page_ = code_start - page_start;
 
-  const uword functions_start = code_end_aligned;
-  const uword functions_size =
-      kNumRuntimeFunctions * compiler::target::kWordSize;
-
-  const uword metadata_start = functions_start + functions_size;
-  const uword metadata_size =
-      NumCallbackTrampolinesPerPage() * sizeof(Metadata);
-  const uword metadata_end = metadata_start + metadata_size;
-  const uword page_end = Utils::RoundUp(metadata_end, kPageSize);
+  ASSERT_LESS_OR_EQUAL((code_start - page_start) + trampoline_code.Size(),
+                       RXMappingSize());
 
   stub_page_ = VirtualMemory::ForImagePage(reinterpret_cast<void*>(page_start),
-                                           code_end_aligned - page_start);
-  offset_of_first_trampoline_in_page_ = code_start - page_start;
-  offset_of_first_runtime_function_in_page_ = functions_start - page_start;
-  offset_of_first_metadata_in_page_ = metadata_start - page_start;
-  size_of_trampoline_page_ = page_end - page_start;
+                                           RXMappingSize());
 
 #if defined(DART_TARGET_OS_FUCHSIA)
   // On Fuchsia we can't currently duplicate pages, so use the first page of
@@ -62,9 +38,13 @@ void FfiCallbackMetadata::EnsureStubPageLocked() {
   // page.
   // TODO(https://dartbug.com/52579): Remove.
   fuchsia_metadata_page_ = VirtualMemory::AllocateAligned(
-      size_of_trampoline_page_, kPageSize, /*is_executable=*/false,
+      MappingSize(), MappingAlignment(), /*is_executable=*/false,
       /*is_compressed=*/false, "FfiCallbackMetadata::TrampolinePage");
-  AddAllTrampolinesToFreeListLocked(page_start);
+  Metadata* metadata = reinterpret_cast<Metadata*>(
+      fuchsia_metadata_page_->start() + MetadataOffset());
+  for (intptr_t i = 0; i < NumCallbackTrampolinesPerPage(); ++i) {
+    AddToFreeListLocked(&metadata[i]);
+  }
 #endif  // defined(DART_TARGET_OS_FUCHSIA)
 }
 
@@ -100,9 +80,8 @@ FfiCallbackMetadata* FfiCallbackMetadata::Instance() {
 void FfiCallbackMetadata::FillRuntimeFunction(VirtualMemory* page,
                                               uword index,
                                               void* function) {
-  uword offset = offset_of_first_runtime_function_in_page_ +
-                 index * compiler::target::kWordSize;
-  void** slot = reinterpret_cast<void**>(page->start() + offset);
+  void** slot =
+      reinterpret_cast<void**>(page->start() + RuntimeFunctionOffset(index));
   *slot = function;
 }
 
@@ -111,7 +90,7 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
   return nullptr;
 #else
   VirtualMemory* new_page = VirtualMemory::AllocateAligned(
-      size_of_trampoline_page_, kPageSize, /*is_executable=*/false,
+      MappingSize(), MappingAlignment(), /*is_executable=*/false,
       /*is_compressed=*/false, "FfiCallbackMetadata::TrampolinePage");
   if (new_page == nullptr) {
     return nullptr;
@@ -126,17 +105,6 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
 #endif  // defined(DART_TARGET_OS_FUCHSIA)
 }
 
-void FfiCallbackMetadata::AddAllTrampolinesToFreeListLocked(uword page_start) {
-  // Assumes lock_ is already locked for writing.
-  const intptr_t trampolines_per_page = NumCallbackTrampolinesPerPage();
-  for (intptr_t i = 0; i < trampolines_per_page; ++i) {
-    const Trampoline trampoline = reinterpret_cast<Trampoline>(
-        page_start + offset_of_first_trampoline_in_page_ +
-        i * kNativeCallbackTrampolineSize);
-    AddToFreeListLocked(trampoline, LookupEntryLocked(trampoline));
-  }
-}
-
 void FfiCallbackMetadata::EnsureFreeListNotEmptyLocked() {
   EnsureStubPageLocked();
 
@@ -155,50 +123,49 @@ void FfiCallbackMetadata::EnsureFreeListNotEmptyLocked() {
   FillRuntimeFunction(new_page, kGetFfiCallbackMetadata,
                       reinterpret_cast<void*>(DLRT_GetFfiCallbackMetadata));
 
-  AddAllTrampolinesToFreeListLocked(new_page->start());
+  // Add all the trampolines to the free list.
+  const intptr_t trampolines_per_page = NumCallbackTrampolinesPerPage();
+  Metadata* metadata =
+      reinterpret_cast<Metadata*>(new_page->start() + MetadataOffset());
+  for (intptr_t i = 0; i < trampolines_per_page; ++i) {
+    AddToFreeListLocked(&metadata[i]);
+  }
 }
 
-FfiCallbackMetadata::Trampoline
-FfiCallbackMetadata::AllocateTrampolineLocked() {
+FfiCallbackMetadata::Metadata* FfiCallbackMetadata::AllocateTrampolineLocked() {
   // Assumes lock_ is already locked for writing.
   EnsureFreeListNotEmptyLocked();
   ASSERT(free_list_head_ != nullptr);
-  const Trampoline trampoline = free_list_head_;
-  auto* entry = LookupEntryLocked(trampoline);
+  Metadata* entry = free_list_head_;
   free_list_head_ = entry->free_list_next_;
   if (free_list_head_ == nullptr) {
-    ASSERT(free_list_tail_ == trampoline);
+    ASSERT(free_list_tail_ == entry);
     free_list_tail_ = nullptr;
   }
-  return trampoline;
+  return entry;
 }
 
-void FfiCallbackMetadata::AddToFreeListLocked(Trampoline trampoline,
-                                              Metadata* entry) {
+void FfiCallbackMetadata::AddToFreeListLocked(Metadata* entry) {
   // Assumes lock_ is already locked for writing.
   if (free_list_tail_ == nullptr) {
     ASSERT(free_list_head_ == nullptr);
-    free_list_head_ = free_list_tail_ = trampoline;
+    free_list_head_ = free_list_tail_ = entry;
   } else {
-    ASSERT(free_list_head_ != nullptr);
-    auto* tail = LookupEntryLocked(free_list_tail_);
-    ASSERT(!tail->IsLive());
-    ASSERT(tail->free_list_next_ == nullptr);
-    tail->free_list_next_ = trampoline;
-    free_list_tail_ = trampoline;
+    ASSERT(free_list_head_ != nullptr && free_list_tail_ != nullptr);
+    ASSERT(!free_list_tail_->IsLive());
+    free_list_tail_->free_list_next_ = entry;
+    free_list_tail_ = entry;
   }
   entry->target_isolate_ = nullptr;
   entry->free_list_next_ = nullptr;
 }
 
-void FfiCallbackMetadata::DeleteSyncTrampolines(Trampoline* sync_list_head) {
+void FfiCallbackMetadata::DeleteSyncTrampolines(Metadata** sync_list_head) {
   WriteRwLocker locker(Thread::Current(), &lock_);
-  for (Trampoline trampoline = *sync_list_head; trampoline != nullptr;) {
-    auto* entry = LookupEntryLocked(trampoline);
-    ASSERT(entry != nullptr);
-    const Trampoline next_trampoline = entry->sync_list_next();
-    AddToFreeListLocked(trampoline, entry);
-    trampoline = next_trampoline;
+  for (Metadata* entry = *sync_list_head; entry != nullptr;) {
+    Metadata* next = entry->sync_list_next();
+    AddToFreeListLocked(entry);
+    entry = next;
   }
   *sync_list_head = nullptr;
 }
@@ -207,14 +174,13 @@ FfiCallbackMetadata::Trampoline FfiCallbackMetadata::CreateFfiCallback(
     Isolate* isolate,
     Zone* zone,
     const Function& function,
-    Trampoline* sync_list_head) {
+    Metadata** sync_list_head) {
   const auto& code =
       Code::Handle(zone, FLAG_precompiled_mode ? function.CurrentCode()
                                                : function.EnsureHasCode());
   ASSERT(!code.IsNull());
 
   const uword target_entry_point = code.EntryPoint();
-  const Trampoline sync_list_next = *sync_list_head;
   TrampolineType trampoline_type = TrampolineType::kSync;
 
 #if defined(TARGET_ARCH_IA32)
@@ -230,50 +196,59 @@ FfiCallbackMetadata::Trampoline FfiCallbackMetadata::CreateFfiCallback(
 #endif
 
   WriteRwLocker locker(Thread::Current(), &lock_);
-  const Trampoline trampoline = AllocateTrampolineLocked();
-  *sync_list_head = trampoline;
-  *LookupEntryLocked(trampoline) =
+  Metadata* entry = AllocateTrampolineLocked();
+  Metadata* sync_list_next = *sync_list_head;
+  *sync_list_head = entry;
+  *entry =
       Metadata(isolate, target_entry_point, sync_list_next, trampoline_type);
 
-  return trampoline;
+  return TrampolineOfMetadata(entry);
 }
 
 FfiCallbackMetadata::Trampoline FfiCallbackMetadata::CreateSyncFfiCallback(
     Isolate* isolate,
     Zone* zone,
     const Function& function,
-    Trampoline* sync_list_head) {
+    Metadata** sync_list_head) {
   return CreateFfiCallback(isolate, zone, function, sync_list_head);
 }
 
-FfiCallbackMetadata::Metadata* FfiCallbackMetadata::LookupEntryLocked(
+FfiCallbackMetadata::Trampoline FfiCallbackMetadata::TrampolineOfMetadata(
+    Metadata* metadata) const {
+  const uword start = MappingStart(reinterpret_cast<uword>(metadata));
+  Metadata* metadatas = reinterpret_cast<Metadata*>(start + MetadataOffset());
+  const uword index = (metadata - metadatas);
+#if defined(DART_TARGET_OS_FUCHSIA)
+  return StubCode::FfiCallbackTrampoline().EntryPoint() +
+         index * kNativeCallbackTrampolineSize;
+#else
+  return start + offset_of_first_trampoline_in_page_ +
+         index * kNativeCallbackTrampolineSize;
+#endif
+}
+
+FfiCallbackMetadata::Metadata* FfiCallbackMetadata::MetadataOfTrampoline(
     Trampoline trampoline) const {
-  // Assumes lock_ is already locked for reading or writing.
-  const uword location = reinterpret_cast<uword>(trampoline);
-
-  // The location that the trampoline would be if the code page was aligned.
-  const uword aligned_location = location - offset_of_first_trampoline_in_page_;
-
-  // Since the code page isn't aligned, the trampoline may actually be in the
-  // following page. So round down the aligned_location, not the raw location.
-  const uword page_start = Utils::RoundDown(aligned_location, kPageSize);
-
-  const uword offset = aligned_location - page_start;
-  ASSERT_EQUAL(offset % kNativeCallbackTrampolineSize, 0);
-
-  const intptr_t index = offset / kNativeCallbackTrampolineSize;
-  ASSERT(index < NumCallbackTrampolinesPerPage());
-
 #if defined(DART_TARGET_OS_FUCHSIA)
   // On Fuchsia the metadata page is separate to the trampoline page.
   // TODO(https://dartbug.com/52579): Remove.
-  const uword metadata_table =
-      fuchsia_metadata_page_->start() + offset_of_first_metadata_in_page_;
+  const uword page_start = Utils::RoundDown(
+      trampoline - offset_of_first_trampoline_in_page_, kPageSize);
+  const uword index =
+      (trampoline - offset_of_first_trampoline_in_page_ - page_start) /
+      kNativeCallbackTrampolineSize;
+  ASSERT(index < NumCallbackTrampolinesPerPage());
+  Metadata* metadata_table = reinterpret_cast<Metadata*>(
+      fuchsia_metadata_page_->start() + MetadataOffset());
+  return metadata_table + index;
 #else
-  const uword metadata_table = page_start + offset_of_first_metadata_in_page_;
-#endif  // defined(DART_TARGET_OS_FUCHSIA)
-
-  return reinterpret_cast<Metadata*>(metadata_table) + index;
+  const uword start = MappingStart(trampoline);
+  Metadata* metadatas = reinterpret_cast<Metadata*>(start + MetadataOffset());
+  const uword index =
+      (trampoline - start - offset_of_first_trampoline_in_page_) /
+      kNativeCallbackTrampolineSize;
+  return &metadatas[index];
+#endif
 }
 
 FfiCallbackMetadata::Metadata FfiCallbackMetadata::LookupMetadataForTrampoline(
@@ -281,7 +256,7 @@ FfiCallbackMetadata::Metadata FfiCallbackMetadata::LookupMetadataForTrampoline(
   // Note: The locker's thread may be null because this method is explicitly
   // designed to be usable outside of a VM thread.
   ReadRwLocker locker(Thread::Current(), &lock_);
-  return *LookupEntryLocked(trampoline);
+  return *MetadataOfTrampoline(trampoline);
 }
 
 FfiCallbackMetadata* FfiCallbackMetadata::singleton_ = nullptr;

runtime/vm/ffi_callback_metadata.h

@@ -30,7 +30,10 @@ namespace dart {
 // unify the FFI callback implementation across JIT and AOT, even on iOS.
 class FfiCallbackMetadata {
  public:
-  using Trampoline = void*;
+  class Metadata;
+
+  // The address of the allocated trampoline.
+  using Trampoline = uword;
 
   enum class TrampolineType : uint8_t {
     kSync = 0,
@@ -55,10 +58,10 @@ class FfiCallbackMetadata {
   Trampoline CreateSyncFfiCallback(Isolate* isolate,
                                    Zone* zone,
                                    const Function& function,
-                                   Trampoline* sync_list_head);
+                                   Metadata** sync_list_head);
 
   // Deletes all the sync trampolines in the list.
-  void DeleteSyncTrampolines(Trampoline* sync_list_head);
+  void DeleteSyncTrampolines(Metadata** sync_list_head);
 
   // FFI callback metadata for any sync or async trampoline.
   class Metadata {
@@ -71,13 +74,13 @@ class FfiCallbackMetadata {
         // safe because Instructions objects are never moved by the GC.
         uword target_entry_point_;
 
-        Trampoline sync_list_next_;
+        Metadata* sync_list_next_;
 
         TrampolineType trampoline_type_;
       };
 
       // !IsLive()
-      Trampoline free_list_next_;
+      Metadata* free_list_next_;
     };
 
     Metadata()
@@ -86,7 +89,7 @@ class FfiCallbackMetadata {
           trampoline_type_(TrampolineType::kSync) {}
     Metadata(Isolate* target_isolate,
              uword target_entry_point,
-             Trampoline sync_list_next,
+             Metadata* sync_list_next,
              TrampolineType trampoline_type)
         : target_isolate_(target_isolate),
           target_entry_point_(target_entry_point),
@@ -122,7 +125,7 @@ class FfiCallbackMetadata {
 
     // To efficiently delete all the sync callbacks for a isolate, they are
     // stored in a singly-linked list. This is the next link in that list.
-    Trampoline sync_list_next() const {
+    Metadata* sync_list_next() {
       ASSERT(IsLive());
       return sync_list_next_;
     }
@@ -140,7 +143,7 @@ class FfiCallbackMetadata {
   Metadata LookupMetadataForTrampoline(Trampoline trampoline) const;
 
   // The number of trampolines that can be stored on a single page.
-  static intptr_t NumCallbackTrampolinesPerPage() {
+  static constexpr intptr_t NumCallbackTrampolinesPerPage() {
     return (kPageSize - kNativeCallbackSharedStubSize) /
            kNativeCallbackTrampolineSize;
   }
@@ -163,10 +166,35 @@ class FfiCallbackMetadata {
 #endif
   static constexpr intptr_t kPageMask = ~(kPageSize - 1);
 
-  // Offset from the start of the trampoline code page to a specific slot in the
-  // runtime function table.
-  static uword RuntimeFunctionOffset(uword function_index) {
-    return 2 * kPageSize + function_index * compiler::target::kWordSize;
+  // Each time we allocate new virtual memory for trampolines we allocate an
+  // [RX][RW] area:
+  //
+  //   * [RX] 2 pages fully containing [StubCode::FfiCallbackTrampoline()]
+  //   * [RW] pages sufficient to hold
+  //      - `kNumRuntimeFunctions` x [uword] function pointers
+  //      - `NumCallbackTrampolinesPerPage()` x [Metadata] objects
+  static constexpr intptr_t RXMappingSize() { return 2 * kPageSize; }
+  static constexpr intptr_t RWMappingSize() {
+    return Utils::RoundUp(
+        kNumRuntimeFunctions * compiler::target::kWordSize +
+            sizeof(Metadata) * NumCallbackTrampolinesPerPage(),
+        kPageSize);
+  }
+  static constexpr intptr_t MappingSize() {
+    return RXMappingSize() + RWMappingSize();
+  }
+  static constexpr intptr_t MappingAlignment() {
+    return Utils::RoundUpToPowerOfTwo(MappingSize());
+  }
+  static constexpr intptr_t MappingStart(uword address) {
+    const uword mask = MappingAlignment() - 1;
+    return address & ~mask;
+  }
+  static constexpr uword RuntimeFunctionOffset(uword function_index) {
+    return RXMappingSize() + function_index * compiler::target::kWordSize;
+  }
+  static constexpr intptr_t MetadataOffset() {
+    return RuntimeFunctionOffset(kNumRuntimeFunctions);
   }
 
 #if defined(TARGET_ARCH_X64)
@@ -201,18 +229,18 @@ class FfiCallbackMetadata {
   FfiCallbackMetadata();
   ~FfiCallbackMetadata();
   void EnsureStubPageLocked();
-  void AddAllTrampolinesToFreeListLocked(uword page_start);
-  void AddToFreeListLocked(Trampoline trampoline, Metadata* entry);
+  void AddToFreeListLocked(Metadata* entry);
   void FillRuntimeFunction(VirtualMemory* page, uword index, void* function);
   VirtualMemory* AllocateTrampolinePage();
   void EnsureFreeListNotEmptyLocked();
-  Trampoline AllocateTrampolineLocked();
+  Metadata* AllocateTrampolineLocked();
   Trampoline TryAllocateFromFreeListLocked();
   Trampoline CreateFfiCallback(Isolate* isolate,
                                Zone* zone,
                                const Function& function,
-                               Trampoline* sync_list_head);
-  Metadata* LookupEntryLocked(Trampoline trampoline) const;
+                               Metadata** sync_list_head);
+  Metadata* MetadataOfTrampoline(Trampoline trampoline) const;
+  Trampoline TrampolineOfMetadata(Metadata* metadata) const;
 
   static FfiCallbackMetadata* singleton_;
 
@@ -220,11 +248,8 @@ class FfiCallbackMetadata {
   VirtualMemory* stub_page_ = nullptr;
   MallocGrowableArray<VirtualMemory*> trampoline_pages_;
   uword offset_of_first_trampoline_in_page_ = 0;
-  uword offset_of_first_runtime_function_in_page_ = 0;
-  uword offset_of_first_metadata_in_page_ = 0;
-  uword size_of_trampoline_page_ = 0;
-  Trampoline free_list_head_ = nullptr;
-  Trampoline free_list_tail_ = nullptr;
+  Metadata* free_list_head_ = nullptr;
+  Metadata* free_list_tail_ = nullptr;
 
 #if defined(DART_TARGET_OS_FUCHSIA)
   // TODO(https://dartbug.com/52579): Remove.

runtime/vm/ffi_callback_metadata_test.cc

@@ -77,8 +77,8 @@ class FakeMessageHandler : public MessageHandler {
 
 VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateSyncFfiCallback) {
   auto* fcm = FfiCallbackMetadata::Instance();
-  FfiCallbackMetadata::Trampoline tramp1 = nullptr;
-  FfiCallbackMetadata::Trampoline tramp2 = nullptr;
+  FfiCallbackMetadata::Trampoline tramp1 = 0;
+  FfiCallbackMetadata::Trampoline tramp2 = 0;
 
   {
     TestIsolateScope isolate_scope;
@@ -95,33 +95,32 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateSyncFfiCallback) {
     const auto& code = Code::Handle(func.EnsureHasCode());
     EXPECT(!code.IsNull());
 
+    EXPECT_EQ(isolate->ffi_callback_sync_list_head(), nullptr);
     tramp1 = isolate->CreateSyncFfiCallback(zone, func);
-    EXPECT_NE(tramp1, nullptr);
+    FfiCallbackMetadata::Metadata* m1 = isolate->ffi_callback_sync_list_head();
 
-    FfiCallbackMetadata::Metadata m1 = fcm->LookupMetadataForTrampoline(tramp1);
-    EXPECT(m1.IsLive());
-    EXPECT_EQ(m1.target_isolate(), isolate);
-    EXPECT_EQ(m1.target_entry_point(), code.EntryPoint());
-    EXPECT_EQ(static_cast<uint8_t>(m1.trampoline_type()),
-              static_cast<uint8_t>(FfiCallbackMetadata::TrampolineType::kSync));
+    EXPECT_NE(tramp1, 0u);
+    EXPECT_NE(m1, nullptr);
+    EXPECT_EQ(m1->sync_list_next(), nullptr);
 
-    EXPECT_EQ(isolate->ffi_callback_sync_list_head(), tramp1);
-    EXPECT_EQ(m1.sync_list_next(), nullptr);
+    EXPECT(m1->IsLive());
+    EXPECT_EQ(m1->target_isolate(), isolate);
+    EXPECT_EQ(m1->target_entry_point(), code.EntryPoint());
+    EXPECT(m1->trampoline_type() == FfiCallbackMetadata::TrampolineType::kSync);
 
     tramp2 = isolate->CreateSyncFfiCallback(zone, func);
-    EXPECT_NE(tramp2, nullptr);
-    EXPECT_NE(tramp2, tramp1);
+    FfiCallbackMetadata::Metadata* m2 = isolate->ffi_callback_sync_list_head();
 
-    FfiCallbackMetadata::Metadata m2 = fcm->LookupMetadataForTrampoline(tramp2);
-    EXPECT(m2.IsLive());
-    EXPECT_EQ(m2.target_isolate(), isolate);
-    EXPECT_EQ(m2.target_entry_point(), code.EntryPoint());
-    EXPECT_EQ(static_cast<uint8_t>(m2.trampoline_type()),
-              static_cast<uint8_t>(FfiCallbackMetadata::TrampolineType::kSync));
+    EXPECT_NE(tramp2, 0u);
+    EXPECT_NE(m2, nullptr);
+    EXPECT_NE(m2, m1);
+    EXPECT_EQ(m2->sync_list_next(), m1);
+    EXPECT_EQ(m2->sync_list_next()->sync_list_next(), nullptr);
 
-    EXPECT_EQ(isolate->ffi_callback_sync_list_head(), tramp2);
-    EXPECT_EQ(m2.sync_list_next(), tramp1);
-    EXPECT_EQ(m1.sync_list_next(), nullptr);
+    EXPECT(m2->IsLive());
+    EXPECT_EQ(m2->target_isolate(), isolate);
+    EXPECT_EQ(m2->target_entry_point(), code.EntryPoint());
+    EXPECT(m2->trampoline_type() == FfiCallbackMetadata::TrampolineType::kSync);
   }
 
   {
@@ -147,7 +146,7 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_DeleteSyncTrampolines) {
 
   auto* fcm = FfiCallbackMetadata::Instance();
   std::unordered_set<FfiCallbackMetadata::Trampoline> sync_tramps;
-  FfiCallbackMetadata::Trampoline sync_list_head = nullptr;
+  FfiCallbackMetadata::Metadata* sync_list_head = nullptr;
 
   const auto& sync_func = Function::Handle(CreateTestFunction());
   const auto& sync_code = Code::Handle(sync_func.EnsureHasCode());
@@ -170,13 +169,12 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_DeleteSyncTrampolines) {
 
   // Verify the list of callbacks.
   uword sync_list_length = 0;
-  for (FfiCallbackMetadata::Trampoline tramp = sync_list_head; tramp != 0;) {
+  for (FfiCallbackMetadata::Metadata* metadata = sync_list_head;
+       metadata != 0;) {
     ++sync_list_length;
-    auto metadata = fcm->LookupMetadataForTrampoline(tramp);
-    EXPECT(metadata.IsLive());
-    EXPECT_EQ(metadata.target_isolate(), isolate);
-    EXPECT_EQ(sync_tramps.count(tramp), 1u);
-    tramp = metadata.sync_list_next();
+    EXPECT(metadata->IsLive());
+    EXPECT_EQ(metadata->target_isolate(), isolate);
+    metadata = metadata->sync_list_next();
   }
   EXPECT_EQ(sync_list_length, sync_tramps.size());
 
@@ -202,7 +200,7 @@ static void RunBigRandomMultithreadedTest(uint64_t seed) {
   auto* fcm = FfiCallbackMetadata::Instance();
   Random random(seed);
   std::unordered_set<FfiCallbackMetadata::Trampoline> sync_tramps;
-  FfiCallbackMetadata::Trampoline sync_list_head = nullptr;
+  FfiCallbackMetadata::Metadata* sync_list_head = nullptr;
 
   const auto& sync_func = Function::Handle(CreateTestFunction());
   const auto& sync_code = Code::Handle(sync_func.EnsureHasCode());
@@ -244,13 +242,12 @@ static void RunBigRandomMultithreadedTest(uint64_t seed) {
 
     // Verify the isolate's list of sync callbacks.
     uword sync_list_length = 0;
-    for (FfiCallbackMetadata::Trampoline tramp = sync_list_head; tramp != 0;) {
+    for (FfiCallbackMetadata::Metadata* metadata = sync_list_head;
+         metadata != 0;) {
       ++sync_list_length;
-      auto metadata = fcm->LookupMetadataForTrampoline(tramp);
-      EXPECT(metadata.IsLive());
-      EXPECT_EQ(metadata.target_isolate(), isolate);
-      EXPECT_EQ(sync_tramps.count(tramp), 1u);
-      tramp = metadata.sync_list_next();
+      EXPECT(metadata->IsLive());
+      EXPECT_EQ(metadata->target_isolate(), isolate);
+      metadata = metadata->sync_list_next();
     }
     EXPECT_EQ(sync_list_length, sync_tramps.size());
   }

runtime/vm/isolate.cc

@@ -3519,7 +3519,9 @@ void Isolate::WaitForOutstandingSpawns() {
   }
 }
 
-void* Isolate::CreateSyncFfiCallback(Zone* zone, const Function& function) {
+FfiCallbackMetadata::Trampoline Isolate::CreateSyncFfiCallback(
+    Zone* zone,
+    const Function& function) {
   return FfiCallbackMetadata::Instance()->CreateSyncFfiCallback(
       this, zone, function, &ffi_callback_sync_list_head_);
 }

runtime/vm/isolate.h

@@ -20,6 +20,7 @@
 #include "vm/class_table.h"
 #include "vm/dispatch_table.h"
 #include "vm/exceptions.h"
+#include "vm/ffi_callback_metadata.h"
 #include "vm/field_table.h"
 #include "vm/fixed_cache.h"
 #include "vm/growable_array.h"
@@ -1242,10 +1243,14 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
     deopt_context_ = value;
   }
 
-  void* CreateSyncFfiCallback(Zone* zone, const Function& function);
+  FfiCallbackMetadata::Trampoline CreateSyncFfiCallback(
+      Zone* zone,
+      const Function& function);
 
   // Visible for testing.
-  void* ffi_callback_sync_list_head() { return ffi_callback_sync_list_head_; }
+  FfiCallbackMetadata::Metadata* ffi_callback_sync_list_head() {
+    return ffi_callback_sync_list_head_;
+  }
 
   intptr_t BlockClassFinalization() {
     ASSERT(defer_finalization_count_ >= 0);
@@ -1637,7 +1642,7 @@ class Isolate : public BaseIsolate, public IntrusiveDListEntry<Isolate> {
   MessageHandler* message_handler_ = nullptr;
   intptr_t defer_finalization_count_ = 0;
   DeoptContext* deopt_context_ = nullptr;
-  void* ffi_callback_sync_list_head_ = nullptr;
+  FfiCallbackMetadata::Metadata* ffi_callback_sync_list_head_ = nullptr;
 
   GrowableObjectArrayPtr tag_table_;
 

runtime/vm/runtime_entry.cc

@@ -3852,11 +3852,13 @@ DEFINE_RAW_LEAF_RUNTIME_ENTRY(ExitSafepointIgnoreUnwindInProgress,
 // This is called by a native callback trampoline
 // (see StubCodeCompiler::GenerateFfiCallbackTrampolineStub). Not registered as
 // a runtime entry because we can't use Thread to look it up.
-extern "C" Thread* DLRT_GetFfiCallbackMetadata(void* trampoline,
-                                               uword* out_entry_point,
-                                               uword* out_trampoline_type) {
+extern "C" Thread* DLRT_GetFfiCallbackMetadata(
+    FfiCallbackMetadata::Trampoline trampoline,
+    uword* out_entry_point,
+    uword* out_trampoline_type) {
   CHECK_STACK_ALIGNMENT;
-  TRACE_RUNTIME_CALL("GetFfiCallbackMetadata %p", trampoline);
+  TRACE_RUNTIME_CALL("GetFfiCallbackMetadata %p",
+                     reinterpret_cast<void*>(trampoline));
 
   auto metadata =
       FfiCallbackMetadata::Instance()->LookupMetadataForTrampoline(trampoline);

runtime/vm/runtime_entry.h

@@ -158,7 +158,7 @@ RUNTIME_ENTRY_LIST(DECLARE_RUNTIME_ENTRY)
 LEAF_RUNTIME_ENTRY_LIST(DECLARE_LEAF_RUNTIME_ENTRY)
 
 // Expected to be called inside a safepoint.
-extern "C" Thread* DLRT_GetFfiCallbackMetadata(void* trampoline,
+extern "C" Thread* DLRT_GetFfiCallbackMetadata(uword trampoline,
                                                uword* out_entry_point,
                                                uword* out_callback_kind);