From 219bf34064e875bc4b23bc22abbc8a72719d4df8 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Wed, 7 Jun 2023 10:51:31 +0000 Subject: [PATCH] [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 Reviewed-by: Daco Harkes --- runtime/lib/ffi.cc | 3 +- runtime/vm/ffi_callback_metadata.cc | 177 ++++++++++------------- runtime/vm/ffi_callback_metadata.h | 69 ++++++--- runtime/vm/ffi_callback_metadata_test.cc | 69 +++++---- runtime/vm/isolate.cc | 4 +- runtime/vm/isolate.h | 11 +- runtime/vm/runtime_entry.cc | 10 +- runtime/vm/runtime_entry.h | 2 +- 8 files changed, 175 insertions(+), 170 deletions(-) diff --git a/runtime/lib/ffi.cc b/runtime/lib/ffi.cc index 91ef7ffd36c..472471fc950 100644 --- a/runtime/lib/ffi.cc +++ b/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(pointer)); + return Pointer::New(isolate->CreateSyncFfiCallback(zone, function)); } DEFINE_NATIVE_ENTRY(DartNativeApiFunctionPointer, 0, 1) { diff --git a/runtime/vm/ffi_callback_metadata.cc b/runtime/vm/ffi_callback_metadata.cc index 95188244c56..3a89e0bd6f8 100644 --- a/runtime/vm/ffi_callback_metadata.cc +++ b/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(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( + 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(page->start() + offset); + void** slot = + reinterpret_cast(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( - 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(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(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(metadata)); + Metadata* metadatas = reinterpret_cast(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(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( + 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_table) + index; + const uword start = MappingStart(trampoline); + Metadata* metadatas = reinterpret_cast(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; diff --git a/runtime/vm/ffi_callback_metadata.h b/runtime/vm/ffi_callback_metadata.h index a6f84ff771a..7f11ebab542 100644 --- a/runtime/vm/ffi_callback_metadata.h +++ b/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 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. diff --git a/runtime/vm/ffi_callback_metadata_test.cc b/runtime/vm/ffi_callback_metadata_test.cc index 728a8a98506..dad3b9c2e22 100644 --- a/runtime/vm/ffi_callback_metadata_test.cc +++ b/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(m1.trampoline_type()), - static_cast(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(m2.trampoline_type()), - static_cast(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 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 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()); } diff --git a/runtime/vm/isolate.cc b/runtime/vm/isolate.cc index 29902db65e1..5e050006204 100644 --- a/runtime/vm/isolate.cc +++ b/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_); } diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 84a49def1da..6b0ab411760 100644 --- a/runtime/vm/isolate.h +++ b/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 { 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 { 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_; diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 76fc1b11e47..fcc40d97822 100644 --- a/runtime/vm/runtime_entry.cc +++ b/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(trampoline)); auto metadata = FfiCallbackMetadata::Instance()->LookupMetadataForTrampoline(trampoline); diff --git a/runtime/vm/runtime_entry.h b/runtime/vm/runtime_entry.h index 2a0bb068cc7..50837709a6a 100644 --- a/runtime/vm/runtime_entry.h +++ b/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);