[vm] Per-group callback thunks.

With no VM isolate, the lifetime of the template stub will be the lifetime of the group instead of the VM.

TEST=ci
Change-Id: I4ed2f5e5c6e8f4c92c430898dc38355a227d4057
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500620
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>

runtime/vm/dart.cc

@@ -364,7 +364,6 @@ char* Dart::DartInit(const Dart_InitializeParams* params) {
   StoreBuffer::Init();
   MarkingStack::Init();
   TargetCPUFeatures::Init();
-  FfiCallbackMetadata::Init();
 
 #if defined(DART_INCLUDE_SIMULATOR)
   Simulator::Init();
@@ -776,7 +775,6 @@ char* Dart::Cleanup() {
   ICData::Cleanup();
   ArgumentsDescriptor::Cleanup();
   OffsetsTable::Cleanup();
-  FfiCallbackMetadata::Cleanup();
   TargetCPUFeatures::Cleanup();
   MarkingStack::Cleanup();
   StoreBuffer::Cleanup();

runtime/vm/ffi_callback_metadata.cc

@@ -27,9 +27,6 @@ namespace dart {
 extern "C" void SimulatorFfiCallbackTrampoline();
 extern "C" void SimulatorFfiCallbackTrampolineEnd();
 #endif
-#if defined(DART_HOST_OS_FUCHSIA)
-static zx_handle_t rx_vmo = ZX_HANDLE_INVALID;
-#endif
 
 FfiCallbackMetadata::FfiCallbackMetadata() {}
 
@@ -37,7 +34,7 @@ void FfiCallbackMetadata::EnsureStubPageLocked() {
   ASSERT(lock_.IsOwnedByCurrentThread());
 
 #if defined(DART_HOST_OS_FUCHSIA)
-  if (rx_vmo == ZX_HANDLE_INVALID) {
+  if (rx_vmo_ == ZX_HANDLE_INVALID) {
     int fd = -1;
     const char* path = "pkg/lib/ffi_callback_stub.bin";
     zx_status_t status =
@@ -49,7 +46,7 @@ void FfiCallbackMetadata::EnsureStubPageLocked() {
       FATAL("fdio_open3_fd(%s) failed: %s\n", path,
             zx_status_get_string(status));
     }
-    status = fdio_get_vmo_exec(fd, &rx_vmo);
+    status = fdio_get_vmo_exec(fd, &rx_vmo_);
     if (status != ZX_OK) {
       FATAL("fdio_get_vmo_exec failed %s\n", zx_status_get_string(status));
     }
@@ -100,27 +97,10 @@ FfiCallbackMetadata::~FfiCallbackMetadata() {
     delete trampoline_pages_[i];
   }
 #if defined(DART_HOST_OS_FUCHSIA)
-  zx_handle_close(rx_vmo);
-  rx_vmo = ZX_HANDLE_INVALID;
+  zx_handle_close(rx_vmo_);
 #endif
 }
 
-void FfiCallbackMetadata::Init() {
-  ASSERT(singleton_ == nullptr);
-  singleton_ = new FfiCallbackMetadata();
-}
-
-void FfiCallbackMetadata::Cleanup() {
-  ASSERT(singleton_ != nullptr);
-  delete singleton_;
-  singleton_ = nullptr;
-}
-
-FfiCallbackMetadata* FfiCallbackMetadata::Instance() {
-  ASSERT(singleton_ != nullptr);
-  return singleton_;
-}
-
 namespace {
 uword RXAreaStart(VirtualMemory* page) {
   return page->start() + page->OffsetToExecutableAlias();
@@ -135,15 +115,23 @@ void FfiCallbackMetadata::FillRuntimeFunction(VirtualMemory* page,
   *slot = function;
 }
 
+FfiCallbackMetadata* FfiCallbackMetadata::Instance(Trampoline trampoline) {
+  const uword start = MappingStart(trampoline);
+  return *reinterpret_cast<FfiCallbackMetadata**>(
+      start + RuntimeFunctionOffset(kGroupFfiCallbackMetadata));
+}
+
 VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
 #if defined(DART_HOST_OS_FUCHSIA)
   zx_handle_t vmar = ZX_HANDLE_INVALID;
   zx_vaddr_t addr = 0;
-  ASSERT(MappingAlignment() <= 64 * KB);
+  zx_vm_option_t align_flag = Utils::ShiftForPowerOfTwo(MappingAlignment())
+                              << ZX_VM_ALIGN_BASE;
+  ASSERT((ZX_VM_ALIGN_1KB <= align_flag) && (align_flag <= ZX_VM_ALIGN_4GB));
   zx_status_t status = zx_vmar_allocate(
       zx_vmar_root_self(),
       ZX_VM_CAN_MAP_SPECIFIC | ZX_VM_CAN_MAP_READ | ZX_VM_CAN_MAP_WRITE |
-          ZX_VM_CAN_MAP_EXECUTE | ZX_VM_ALIGN_64KB,
+          ZX_VM_CAN_MAP_EXECUTE | align_flag,
       0, MappingSize(), &vmar, &addr);
   if (status != ZX_OK) {
     FATAL("zx_vmar_allocate failed: %s", zx_status_get_string(status));
@@ -152,16 +140,18 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
   zx_handle_t rw_vmo = ZX_HANDLE_INVALID;
   status = zx_vmo_create(RWMappingSize(), 0, &rw_vmo);
   if (status != ZX_OK) {
-    FATAL("zx_vmar_allocate failed: %s", zx_status_get_string(status));
+    FATAL("zx_vmo_create failed: %s", zx_status_get_string(status));
   }
+  const char* name = "dart-ffi-callback-bss";
+  zx_object_set_property(rw_vmo, ZX_PROP_NAME, name, strlen(name));
 
   zx_vaddr_t rx_addr = 0;
   status =
       zx_vmar_map(vmar, ZX_VM_SPECIFIC | ZX_VM_PERM_READ | ZX_VM_PERM_EXECUTE,
-                  /*vmar_offset=*/0, rx_vmo,
+                  /*vmar_offset=*/0, rx_vmo_,
                   /*vmo_offset=*/0, RXMappingSize(), &rx_addr);
   if (status != ZX_OK) {
-    FATAL("zx_vmar_allocate failed: %s", zx_status_get_string(status));
+    FATAL("zx_vmar_map failed: %s", zx_status_get_string(status));
   }
 
   zx_vaddr_t rw_addr = 0;
@@ -170,7 +160,7 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
                   /*vmar_offset=*/RXMappingSize(), rw_vmo,
                   /*vmo_offset=*/0, RWMappingSize(), &rw_addr);
   if (status != ZX_OK) {
-    FATAL("zx_vmar_allocate failed: %s", zx_status_get_string(status));
+    FATAL("zx_vmar_map failed: %s", zx_status_get_string(status));
   }
 
   zx_handle_close(rw_vmo);
@@ -274,6 +264,7 @@ void FfiCallbackMetadata::EnsureFreeListNotEmptyLocked() {
   FillRuntimeFunction(new_page, kDoRedirectedFfiCallback,
                       reinterpret_cast<void*>(DoRedirectedFfiCallback));
 #endif
+  FillRuntimeFunction(new_page, kGroupFfiCallbackMetadata, this);
 
   // Add all the trampolines to the free list.
   const intptr_t trampolines_per_page = NumCallbackTrampolinesPerPage();
@@ -498,8 +489,6 @@ FfiCallbackMetadata::LookupMetadataForTrampolineUnlocked(
   return *MetadataEntryOfTrampoline(trampoline)->metadata();
 }
 
-FfiCallbackMetadata* FfiCallbackMetadata::singleton_ = nullptr;
-
 ApiState* FfiCallbackMetadata::Metadata::api_state() const {
   return (is_isolate_group_bound() ? target_isolate_group_
                                    : target_isolate_->group())

runtime/vm/ffi_callback_metadata.h

@@ -4,6 +4,12 @@
 #ifndef RUNTIME_VM_FFI_CALLBACK_METADATA_H_
 #define RUNTIME_VM_FFI_CALLBACK_METADATA_H_
 
+#include "platform/globals.h"
+
+#if defined(DART_HOST_OS_FUCHSIA)
+#include <zircon/types.h>
+#endif
+
 #include "platform/growable_array.h"
 #include "platform/utils.h"
 #include "vm/hash_map.h"
@@ -63,14 +69,15 @@ class FfiCallbackMetadata {
   enum RuntimeFunctions {
     kGetFfiCallbackMetadata,
     kDoRedirectedFfiCallback,
+    kGroupFfiCallbackMetadata,
     kNumRuntimeFunctions,
   };
 
-  static void Init();
-  static void Cleanup();
+  FfiCallbackMetadata();
+  ~FfiCallbackMetadata();
 
   // Returns the FfiCallbackMetadata singleton.
-  static FfiCallbackMetadata* Instance();
+  static FfiCallbackMetadata* Instance(Trampoline trampoline);
 
   // Creates an async callback trampoline for the given function and associates
   // it with the send_port.
@@ -360,8 +367,6 @@ class FfiCallbackMetadata {
   Trampoline TrampolineOfMetadataEntry(MetadataEntry* metadata) const;
 
  private:
-  FfiCallbackMetadata();
-  ~FfiCallbackMetadata();
   void EnsureStubPageLocked();
   void AddToFreeListLocked(MetadataEntry* entry);
   void DeleteCallbackLocked(MetadataEntry* entry);
@@ -393,6 +398,9 @@ class FfiCallbackMetadata {
   uword offset_of_first_trampoline_in_page_ = 0;
   MetadataEntry* free_list_head_ = nullptr;
   MetadataEntry* free_list_tail_ = nullptr;
+#if defined(DART_HOST_OS_FUCHSIA)
+  zx_handle_t rx_vmo_ = ZX_HANDLE_INVALID;
+#endif
 
   DISALLOW_COPY_AND_ASSIGN(FfiCallbackMetadata);
 };

runtime/vm/ffi_callback_metadata_test.cc

@@ -76,7 +76,6 @@ class FakeMessageHandler : public MessageHandler {
 };
 
 VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateSyncFfiCallback) {
-  auto* fcm = FfiCallbackMetadata::Instance();
   FfiCallbackMetadata::Trampoline tramp1 = 0;
   FfiCallbackMetadata::Trampoline tramp2 = 0;
 
@@ -85,6 +84,7 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateSyncFfiCallback) {
     Thread* thread = Thread::Current();
     Isolate* isolate = thread->isolate();
     ASSERT(isolate == isolate_scope.isolate());
+    auto* fcm = isolate->group()->callback_metadata();
     TransitionNativeToVM transition(thread);
     StackZone stack_zone(thread);
 
@@ -156,21 +156,9 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateSyncFfiCallback) {
       EXPECT_EQ(e2->list_next(), nullptr);
     }
   }
-
-  {
-    // Isolate has shut down, so all callbacks should be deleted.
-    FfiCallbackMetadata::Metadata m1 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp1);
-    EXPECT(!m1.IsLive());
-
-    FfiCallbackMetadata::Metadata m2 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp2);
-    EXPECT(!m2.IsLive());
-  }
 }
 
 VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateAsyncFfiCallback) {
-  auto* fcm = FfiCallbackMetadata::Instance();
   FfiCallbackMetadata::Trampoline tramp1 = 0;
   FfiCallbackMetadata::Trampoline tramp2 = 0;
 
@@ -179,6 +167,7 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateAsyncFfiCallback) {
     Thread* thread = Thread::Current();
     Isolate* isolate = thread->isolate();
     ASSERT(thread->isolate() == isolate_scope.isolate());
+    auto* fcm = isolate->group()->callback_metadata();
     TransitionNativeToVM transition(thread);
     StackZone stack_zone(thread);
 
@@ -252,21 +241,9 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateAsyncFfiCallback) {
       EXPECT_EQ(e1->list_next(), nullptr);
     }
   }
-
-  {
-    // Isolate has shut down, so all callbacks should be deleted.
-    FfiCallbackMetadata::Metadata m1 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp1);
-    EXPECT(!m1.IsLive());
-
-    FfiCallbackMetadata::Metadata m2 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp2);
-    EXPECT(!m2.IsLive());
-  }
 }
 
 VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateIsolateLocalFfiCallback) {
-  auto* fcm = FfiCallbackMetadata::Instance();
   FfiCallbackMetadata::Trampoline tramp1 = 0;
   FfiCallbackMetadata::Trampoline tramp2 = 0;
 
@@ -275,6 +252,7 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateIsolateLocalFfiCallback) {
     Thread* thread = Thread::Current();
     Isolate* isolate = thread->isolate();
     ASSERT(thread->isolate() == isolate_scope.isolate());
+    auto* fcm = isolate->group()->callback_metadata();
     TransitionNativeToVM transition(thread);
     StackZone stack_zone(thread);
 
@@ -357,23 +335,12 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_CreateIsolateLocalFfiCallback) {
       EXPECT_EQ(e1->list_next(), nullptr);
     }
   }
-
-  {
-    // Isolate has shut down, so all callbacks should be deleted.
-    FfiCallbackMetadata::Metadata m1 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp1);
-    EXPECT(!m1.IsLive());
-
-    FfiCallbackMetadata::Metadata m2 =
-        fcm->LookupMetadataForTrampolineUnlocked(tramp2);
-    EXPECT(!m2.IsLive());
-  }
 }
 
 ISOLATE_UNIT_TEST_CASE(FfiCallbackMetadata_TrampolineRecycling) {
   Isolate* isolate = thread->isolate();
   auto* zone = thread->zone();
-  auto* fcm = FfiCallbackMetadata::Instance();
+  auto* fcm = IsolateGroup::Current()->callback_metadata();
 
   const Function& func =
       Function::Handle(CreateTestFunction(FfiCallbackKind::kAsyncCallback));
@@ -437,7 +404,7 @@ VM_UNIT_TEST_CASE(FfiCallbackMetadata_DeleteTrampolines) {
   TransitionNativeToVM transition(thread);
   StackZone stack_zone(thread);
 
-  auto* fcm = FfiCallbackMetadata::Instance();
+  auto* fcm = IsolateGroup::Current()->callback_metadata();
   std::unordered_set<FfiCallbackMetadata::Trampoline> tramps;
   FfiCallbackMetadata::MetadataEntry* list_head = nullptr;
 
@@ -515,7 +482,7 @@ static void RunBigRandomMultithreadedTest(uint64_t seed) {
     Dart_Port port;
   };
 
-  auto* fcm = FfiCallbackMetadata::Instance();
+  auto* fcm = IsolateGroup::Current()->callback_metadata();
   Random random(seed);
   std::vector<TrampolineWithPort> tramps;
   std::unordered_set<FfiCallbackMetadata::Trampoline> tramp_set;

runtime/vm/isolate.cc

@@ -336,6 +336,7 @@ IsolateGroup::IsolateGroup(std::shared_ptr<IsolateGroupSource> source,
 #if !defined(DART_PRECOMPILED_RUNTIME)
       background_compiler_(new BackgroundCompiler(this)),
 #endif
+      callback_metadata_(new FfiCallbackMetadata()),
       symbols_mutex_(),
       type_canonicalization_mutex_(),
       type_arguments_canonicalization_mutex_(),
@@ -2545,8 +2546,7 @@ void Isolate::LowLevelShutdown() {
   // Clean up any synchronous FFI callbacks registered with this isolate. Skip
   // if this isolate never registered any.
   if (ffi_callback_list_head_ != nullptr) {
-    FfiCallbackMetadata::Instance()->DeleteAllCallbacks(
-        &ffi_callback_list_head_);
+    group()->callback_metadata()->DeleteAllCallbacks(&ffi_callback_list_head_);
   }
 
 #if !defined(PRODUCT)
@@ -2710,7 +2710,7 @@ void Isolate::LowLevelCleanup(Isolate* isolate) {
       // Clean up any synchronous FFI callbacks registered with this
       // isolate group. Skip if this isolate group never registered any.
       if (isolate_group->ffi_callback_list_head_ != nullptr) {
-        FfiCallbackMetadata::Instance()->DeleteAllCallbacks(
+        isolate_group->callback_metadata()->DeleteAllCallbacks(
             &isolate_group->ffi_callback_list_head_);
       }
 
@@ -3810,7 +3810,7 @@ FfiCallbackMetadata::Trampoline Isolate::CreateAsyncFfiCallback(
     Zone* zone,
     const Function& send_function,
     Dart_Port send_port) {
-  return FfiCallbackMetadata::Instance()->CreateAsyncFfiCallback(
+  return group()->callback_metadata()->CreateAsyncFfiCallback(
       this, zone, send_function, send_port, &ffi_callback_list_head_);
 }
 
@@ -3822,7 +3822,7 @@ FfiCallbackMetadata::Trampoline Isolate::CreateIsolateLocalFfiCallback(
   if (keep_isolate_alive) {
     UpdateNativeCallableKeepIsolateAliveCounter(1);
   }
-  return FfiCallbackMetadata::Instance()->CreateLocalFfiCallback(
+  return group()->callback_metadata()->CreateLocalFfiCallback(
       this, /*isolate_group=*/nullptr, zone, trampoline, target,
       &ffi_callback_list_head_);
 }
@@ -3831,14 +3831,13 @@ FfiCallbackMetadata::Trampoline
 IsolateGroup::CreateIsolateGroupBoundFfiCallback(Zone* zone,
                                                  const Function& trampoline,
                                                  const Closure& target) {
-  return FfiCallbackMetadata::Instance()->CreateLocalFfiCallback(
+  return callback_metadata()->CreateLocalFfiCallback(
       /*isolate=*/nullptr, this, zone, trampoline, target,
       &ffi_callback_list_head_);
 }
 
 void IsolateGroup::DeleteFfiCallback(FfiCallbackMetadata::Trampoline callback) {
-  FfiCallbackMetadata::Instance()->DeleteCallback(callback,
-                                                  &ffi_callback_list_head_);
+  callback_metadata()->DeleteCallback(callback, &ffi_callback_list_head_);
 }
 
 bool Isolate::HasLivePorts() {
@@ -3891,8 +3890,8 @@ void Isolate::CloseReceivePort(const ReceivePort& receive_port) {
 }
 
 void Isolate::DeleteFfiCallback(FfiCallbackMetadata::Trampoline callback) {
-  FfiCallbackMetadata::Instance()->DeleteCallback(callback,
-                                                  &ffi_callback_list_head_);
+  group()->callback_metadata()->DeleteCallback(callback,
+                                               &ffi_callback_list_head_);
 }
 
 void Isolate::UpdateNativeCallableKeepIsolateAliveCounter(intptr_t delta) {

runtime/vm/isolate.h

@@ -343,6 +343,9 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
 
   Heap* heap() const { return heap_.get(); }
   Roots* roots() const { return roots_.get(); }
+  FfiCallbackMetadata* callback_metadata() const {
+    return callback_metadata_.get();
+  }
 
   BackgroundCompiler* background_compiler() const {
 #if defined(DART_PRECOMPILED_RUNTIME)
@@ -965,6 +968,8 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
 
   NOT_IN_PRECOMPILED(std::unique_ptr<BackgroundCompiler> background_compiler_);
 
+  std::unique_ptr<FfiCallbackMetadata> callback_metadata_;
+
   Mutex symbols_mutex_;
   Mutex type_canonicalization_mutex_;
   Mutex type_arguments_canonicalization_mutex_;

runtime/vm/runtime_entry.cc

@@ -5137,9 +5137,8 @@ extern "C" Thread* DLRT_GetFfiCallbackMetadata(
   // have a use-after-free scenario here and therefore undefined behavior.
   // We make some best effort to `FATAL()` in obvious cases of undefined
   // behavior, but not all cases will be caught.
-  auto metadata =
-      FfiCallbackMetadata::Instance()->LookupMetadataForTrampolineUnlocked(
-          trampoline);
+  auto metadata = FfiCallbackMetadata::Instance(trampoline)
+                      ->LookupMetadataForTrampolineUnlocked(trampoline);
 
   if (!metadata.IsLive()) {
     FATAL("Callback invoked after it has been deleted.");