From 4679c040c6309b9d12c5ab130591afcaa4eb6394 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 5 Feb 2026 01:29:23 -0800 Subject: [PATCH] [vm] Sync ffi callbacks can enter owned target isolate Adds a new code path for NativeCallable.isolateLocal invocations. If the current thread is not entered into any isolate, but owns the target isolate, then it enters the target isolate, invokes, then exits the isolate. Fixes: https://github.com/dart-lang/sdk/issues/61623 TEST=tests/ffi/function_callbacks_isolate_ownership_test.dart Change-Id: I401f185fadf7d2a55190dafd15387e1c418c67c9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452380 Reviewed-by: Ryan Macnak Commit-Queue: Liam Appelbe --- BUILD.gn | 1 + runtime/bin/ffi_test/ffi_test_functions.cc | 22 ++- .../tests/vm/dart/exported_symbols_test.dart | 1 + runtime/vm/bss_relocs.cc | 3 + runtime/vm/bss_relocs.h | 3 +- runtime/vm/compiler/stub_code_compiler_arm.cc | 35 ++++- .../vm/compiler/stub_code_compiler_arm64.cc | 56 ++++++- .../vm/compiler/stub_code_compiler_ia32.cc | 58 ++++++- .../vm/compiler/stub_code_compiler_riscv.cc | 72 +++++++-- runtime/vm/compiler/stub_code_compiler_x64.cc | 48 +++++- runtime/vm/dart_api_impl.cc | 10 ++ runtime/vm/ffi_callback_metadata.cc | 3 + runtime/vm/ffi_callback_metadata.h | 23 ++- runtime/vm/runtime_entry.cc | 147 +++++++++++------- runtime/vm/runtime_entry.h | 1 + runtime/vm/simulator_arm64.cc | 4 + tests/ffi/ffi.status | 1 + ...tion_callbacks_isolate_ownership_test.dart | 63 ++++++++ 18 files changed, 447 insertions(+), 104 deletions(-) create mode 100644 tests/ffi/function_callbacks_isolate_ownership_test.dart diff --git a/BUILD.gn b/BUILD.gn index 16415b2a893..56c4b9ab1fd 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -266,6 +266,7 @@ if (is_fuchsia) { "tests/ffi/ffi_induce_a_crash_test.dart", "tests/ffi/ffi_native_test.dart", "tests/ffi/finalizer_external_size_accounting_test.dart", + "tests/ffi/function_callbacks_isolate_ownership_test.dart", "tests/ffi/function_callbacks_many_test.dart", "tests/ffi/function_callbacks_structs_by_value_generated_test.dart", "tests/ffi/function_callbacks_structs_by_value_native_callable_generated_test.dart", diff --git a/runtime/bin/ffi_test/ffi_test_functions.cc b/runtime/bin/ffi_test/ffi_test_functions.cc index ef0b978ab97..7f856aa32b0 100644 --- a/runtime/bin/ffi_test/ffi_test_functions.cc +++ b/runtime/bin/ffi_test/ffi_test_functions.cc @@ -26,13 +26,7 @@ #endif #include "bin/ffi_test/ffi_test_fields.h" - -#if defined(_WIN32) -#define DART_EXPORT extern "C" __declspec(dllexport) -#else -#define DART_EXPORT \ - extern "C" __attribute__((visibility("default"))) __attribute((used)) -#endif +#include "include/dart_api.h" namespace dart { @@ -1449,4 +1443,18 @@ DART_EXPORT void TwiddleVec4Components(Vec4 input, Vec4* result) { result->w = input.x; } +DART_EXPORT int32_t CallTwoIntFunctionIsolateOwnership( + void (*Dart_ClearCurrentThreadOwnsIsolate_ForTesting)(), + int32_t (*fn)(int32_t, int32_t), + int32_t a, + int32_t b) { + Dart_Isolate isolate = Dart_CurrentIsolate(); + Dart_SetCurrentThreadOwnsIsolate(); + Dart_ExitIsolate(); + int result = fn(a, b); + Dart_EnterIsolate(isolate); + Dart_ClearCurrentThreadOwnsIsolate_ForTesting(); + return result; +} + } // namespace dart diff --git a/runtime/tests/vm/dart/exported_symbols_test.dart b/runtime/tests/vm/dart/exported_symbols_test.dart index 34ab7fee8c9..bfe08019536 100644 --- a/runtime/tests/vm/dart/exported_symbols_test.dart +++ b/runtime/tests/vm/dart/exported_symbols_test.dart @@ -65,6 +65,7 @@ main() { "Dart_ClassLibrary", "Dart_ClassName", "Dart_Cleanup", + "Dart_ClearCurrentThreadOwnsIsolate_ForTesting", "Dart_CloseNativePort", "Dart_ClosureFunction", "Dart_CompileAll", diff --git a/runtime/vm/bss_relocs.cc b/runtime/vm/bss_relocs.cc index 5af45ef1478..ddd83b72fc6 100644 --- a/runtime/vm/bss_relocs.cc +++ b/runtime/vm/bss_relocs.cc @@ -38,6 +38,9 @@ void BSS::Initialize(Thread* current, uword* bss_start, bool vm) { InitializeBSSEntry(Relocation::DLRT_ExitIsolateGroupBoundIsolate, reinterpret_cast(DLRT_ExitIsolateGroupBoundIsolate), bss_start); + InitializeBSSEntry( + Relocation::DLRT_ExitSyncCallbackTargetIsolate, + reinterpret_cast(DLRT_ExitSyncCallbackTargetIsolate), bss_start); } } // namespace dart diff --git a/runtime/vm/bss_relocs.h b/runtime/vm/bss_relocs.h index 9b380cacab0..ab6162c93c1 100644 --- a/runtime/vm/bss_relocs.h +++ b/runtime/vm/bss_relocs.h @@ -18,7 +18,8 @@ class BSS : public AllStatic { enum class Relocation : intptr_t { DLRT_GetFfiCallbackMetadata, // TODO(https://dartbug.com/52579): Remove. DLRT_ExitTemporaryIsolate, // TODO(https://dartbug.com/52579): Remove. - DLRT_ExitIsolateGroupBoundIsolate, // TODO(https://dartbug.com/52579) + DLRT_ExitIsolateGroupBoundIsolate, // TODO(https://dartbug.com/52579) + DLRT_ExitSyncCallbackTargetIsolate, // TODO(https://dartbug.com/52579) EndOfVmEntries, // We don't have any isolate group specific entries at the moment. diff --git a/runtime/vm/compiler/stub_code_compiler_arm.cc b/runtime/vm/compiler/stub_code_compiler_arm.cc index 42f6f4dc9a5..8a557aae845 100644 --- a/runtime/vm/compiler/stub_code_compiler_arm.cc +++ b/runtime/vm/compiler/stub_code_compiler_arm.cc @@ -396,13 +396,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { Label async_callback; Label sync_isolate_group_bound_callback; + Label sync_callback_isolate_ownership; Label done; - // If GetFfiCallbackMetadata returned a null thread, it means that the async - // callback was invoked after it was deleted. In this case, do nothing. - __ cmp(THR, Operand(0)); - __ b(&done, EQ); - // Check the trampoline type to see how the callback should be invoked. __ cmp( R4, @@ -413,6 +409,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound))); __ b(&sync_isolate_group_bound_callback, EQ); + __ tst(R4, Operand(FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag)); + __ b(&sync_callback_isolate_ownership, NE); + // Sync callback. The entry point contains the target function, so just call // it. DLRT_GetThreadForNativeCallbackTrampoline exited the safepoint, so // re-enter it afterwards. @@ -427,6 +426,32 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { __ b(&done); + __ Bind(&sync_callback_isolate_ownership); + + __ blx(R5); + + // Exit the target isolate. + { + __ EnterFrame(1 << FP, 0); + __ ReserveAlignedFrameSpace(0); + + const RegisterSet return_registers( + (1 << CallingConventions::kReturnReg) | + (1 << CallingConventions::kSecondReturnReg), + 1 << CallingConventions::kReturnFpuReg); + __ PushRegisters(return_registers); + + GenerateLoadFfiCallbackMetadataRuntimeFunction( + FfiCallbackMetadata::kExitSyncCallbackTargetIsolate, R4); + + __ blx(R4); + + __ PopRegisters(return_registers); + __ LeaveFrame(1 << FP); + } + + __ b(&done); + __ Bind(&sync_isolate_group_bound_callback); __ blx(R5); diff --git a/runtime/vm/compiler/stub_code_compiler_arm64.cc b/runtime/vm/compiler/stub_code_compiler_arm64.cc index dd90c2320a2..e2dc10a1d0f 100644 --- a/runtime/vm/compiler/stub_code_compiler_arm64.cc +++ b/runtime/vm/compiler/stub_code_compiler_arm64.cc @@ -573,13 +573,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { Label async_callback; Label sync_isolate_group_bound_callback; + Label sync_callback_isolate_ownership; Label done; - // If GetFfiCallbackMetadata returned a null thread, it means that the async - // callback was invoked after it was deleted. In this case, do nothing. - __ cmp(THR, Operand(0)); - __ b(&done, EQ); - // Check the trampoline type to see how the callback should be invoked. __ cmp( R9, @@ -590,6 +586,10 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound))); __ b(&sync_isolate_group_bound_callback, EQ); + __ tsti(R9, + Immediate(FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag)); + __ b(&sync_callback_isolate_ownership, NOT_ZERO); + // Sync callback. The entry point contains the target function, so just call // it. DLRT_GetThreadForNativeCallbackTrampoline exited the safepoint, so // re-enter it afterwards. @@ -603,6 +603,52 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { __ b(&done); + __ Bind(&sync_callback_isolate_ownership); + + __ blr(R10); + + // Exit the target isolate. + { + __ SetupDartSP(); + __ EnterFrame(0); + __ ReserveAlignedFrameSpace(0); + + const RegisterSet return_registers( + (1 << CallingConventions::kReturnReg) | + (1 << CallingConventions::kSecondReturnReg), + 1 << CallingConventions::kReturnFpuReg); + __ PushRegisters(return_registers); + +#if defined(DART_TARGET_OS_FUCHSIA) + // TODO(https://dartbug.com/52579): Remove. + if (FLAG_precompiled_mode) { + GenerateLoadBSSEntry(BSS::Relocation::DLRT_ExitSyncCallbackTargetIsolate, + R4, R9); + } else { + Label call; + __ ldr(R4, compiler::Address::PC(2 * Instr::kInstrSize)); + __ b(&call); + __ Emit64(reinterpret_cast(&DLRT_ExitSyncCallbackTargetIsolate)); + __ Bind(&call); + } +#else + GenerateLoadFfiCallbackMetadataRuntimeFunction( + FfiCallbackMetadata::kExitSyncCallbackTargetIsolate, R4); +#endif + + __ mov(CSP, SP); + __ blr(R4); + __ mov(SP, CSP); + __ mov(THR, R0); + + __ PopRegisters(return_registers); + + __ LeaveFrame(); + __ RestoreCSP(); + } + + __ b(&done); + __ Bind(&sync_isolate_group_bound_callback); __ blr(R10); diff --git a/runtime/vm/compiler/stub_code_compiler_ia32.cc b/runtime/vm/compiler/stub_code_compiler_ia32.cc index 8c8880dc0c4..c84ed7fea0f 100644 --- a/runtime/vm/compiler/stub_code_compiler_ia32.cc +++ b/runtime/vm/compiler/stub_code_compiler_ia32.cc @@ -292,23 +292,23 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { Label async_callback; Label sync_isolate_group_bound_callback; + Label sync_callback_isolate_ownership; Label done; - // If GetFfiCallbackMetadata returned a null thread, it means that the async - // callback was invoked after it was deleted. In this case, do nothing. - __ cmpl(THR, Immediate(0)); - __ j(EQUAL, &done, Assembler::kFarJump); - // Check the trampoline type to see how the callback should be invoked. __ cmpl(EBX, Immediate(static_cast( FfiCallbackMetadata::TrampolineType::kAsync))); - __ j(EQUAL, &async_callback, Assembler::kNearJump); + __ j(EQUAL, &async_callback); __ cmpl(EBX, Immediate(static_cast( FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound))); __ j(EQUAL, &sync_isolate_group_bound_callback, Assembler::kNearJump); + __ testl(EBX, + Immediate(FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag)); + __ j(NOT_ZERO, &sync_callback_isolate_ownership, Assembler::kNearJump); + // Sync callback. The entry point contains the target function, so just call // it. DLRT_GetThreadForNativeCallbackTrampoline exited the safepoint, so // re-enter it afterwards. @@ -335,6 +335,52 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { __ Bind(&ret_4); __ ret(Immediate(4)); + __ Bind(&sync_callback_isolate_ownership); + + __ call(ECX); + + // Exit the target isolate. + { + __ pushl(CallingConventions::kReturnReg); + __ pushl(CallingConventions::kSecondReturnReg); + __ subl(ESP, Immediate(kFpuRegisterSize)); + __ movups(Address(ESP, 0), CallingConventions::kReturnFpuReg); + + __ EnterFrame(0); + __ ReserveAlignedFrameSpace(0); + + __ movl(EAX, Immediate(reinterpret_cast( + DLRT_ExitSyncCallbackTargetIsolate))); + __ CallCFunction(EAX); + + __ LeaveFrame(); + + __ movups(Address(ESP, 0), CallingConventions::kReturnFpuReg); + __ addl(ESP, Immediate(kFpuRegisterSize)); + __ popl(CallingConventions::kSecondReturnReg); + __ popl(CallingConventions::kReturnReg); + + // Pop the trampoline type into ECX. + __ popl(ECX); + + // Restore callee-saved registers. + __ popl(EBX); + __ popl(THR); + + Label ownership_ret_4; + __ cmpl(ECX, + Immediate( + static_cast(FfiCallbackMetadata::TrampolineType::kSync) | + FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag)); + __ j(NOT_EQUAL, &ownership_ret_4, Assembler::kNearJump); + __ ret(); + + __ Bind(&ownership_ret_4); + __ ret(Immediate(4)); + } + + __ jmp(&done, Assembler::kNearJump); + __ Bind(&sync_isolate_group_bound_callback); __ call(ECX); diff --git a/runtime/vm/compiler/stub_code_compiler_riscv.cc b/runtime/vm/compiler/stub_code_compiler_riscv.cc index c7d42ca03c1..523362f85ef 100644 --- a/runtime/vm/compiler/stub_code_compiler_riscv.cc +++ b/runtime/vm/compiler/stub_code_compiler_riscv.cc @@ -434,12 +434,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { Label something_other_than_sync_callback; Label async_callback; + Label sync_isolate_group_bound_callback; Label done; - // If GetFfiCallbackMetadata returned a null thread, it means that the - // callback was invoked after it was deleted. In this case, do nothing. - __ beqz(THR, &done, Assembler::kFarJump); - // Check the trampoline type to see how the callback should be invoked. COMPILE_ASSERT( @@ -459,15 +456,63 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { __ j(&done, Assembler::kNearJump); __ Bind(&something_other_than_sync_callback); - COMPILE_ASSERT( - static_cast(FfiCallbackMetadata::TrampolineType::kAsync) == 2); - __ subi(T3, T3, 2); - __ beqz(T3, &async_callback, Assembler::kNearJump); + __ li(T4, static_cast(FfiCallbackMetadata::TrampolineType::kAsync)); + __ beq(T3, T4, &async_callback, Assembler::kNearJump); - COMPILE_ASSERT( - static_cast( - FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound) == 3); - // isolate-group-shared callback + __ li(T4, static_cast( + FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound)); + __ beq(T3, T4, &sync_isolate_group_bound_callback, Assembler::kNearJump); + + // Sync callback that entered the target isolate. + __ jalr(T2); + + // Exit the target isolate. + { + __ EnterFrame(0); + __ ReserveAlignedFrameSpace(0); + + const RegisterSet return_registers( + (1 << CallingConventions::kReturnReg) | + (1 << CallingConventions::kSecondReturnReg), + 1 << CallingConventions::kReturnFpuReg); + __ PushRegisters(return_registers); + + Label call; + +#if defined(DART_TARGET_OS_FUCHSIA) + // TODO(https://dartbug.com/52579): Remove. + if (FLAG_precompiled_mode) { + GenerateLoadBSSEntry(BSS::Relocation::DRT_ExitSyncCallbackTargetIsolate, + T1, T2); + } else { + const intptr_t kPCRelativeLoadOffset = 12; + intptr_t start = __ CodeSize(); + __ auipc(T1, 0); + __ lx(T1, Address(T1, kPCRelativeLoadOffset)); + __ j(&call); + + ASSERT_EQUAL(__ CodeSize() - start, kPCRelativeLoadOffset); +#if XLEN == 32 + __ Emit32(reinterpret_cast(&DLRT_ExitSyncCallbackTargetIsolate)); +#else + __ Emit64(reinterpret_cast(&DLRT_ExitSyncCallbackTargetIsolate)); +#endif + } +#else + GenerateLoadFfiCallbackMetadataRuntimeFunction( + FfiCallbackMetadata::kExitSyncCallbackTargetIsolate, T1); +#endif // defined(DART_TARGET_OS_FUCHSIA) + + __ Bind(&call); + __ jalr(T1); + + __ PopRegisters(return_registers); + + __ LeaveFrame(); + __ j(&done, Assembler::kNearJump); + } + + __ Bind(&sync_isolate_group_bound_callback); __ jalr(T2); // Exit isolate group bound isolate. @@ -513,10 +558,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { __ PopRegisters(return_registers); __ LeaveFrame(); + __ j(&done, Assembler::kNearJump); } - __ j(&done, Assembler::kNearJump); - __ Bind(&async_callback); // Async callback. The entrypoint marshals the arguments into a message and diff --git a/runtime/vm/compiler/stub_code_compiler_x64.cc b/runtime/vm/compiler/stub_code_compiler_x64.cc index 0d6e14cfc21..75b2e1afbec 100644 --- a/runtime/vm/compiler/stub_code_compiler_x64.cc +++ b/runtime/vm/compiler/stub_code_compiler_x64.cc @@ -609,13 +609,9 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { Label async_callback; Label sync_isolate_group_bound_callback; + Label sync_callback_isolate_ownership; Label done; - // If GetFfiCallbackMetadata returned a null thread, it means that the - // callback was invoked after it was deleted. In this case, do nothing. - __ cmpq(THR, Immediate(0)); - __ j(EQUAL, &done); - // Check the trampoline type to see how the callback should be invoked. __ cmpq(RAX, Immediate(static_cast( FfiCallbackMetadata::TrampolineType::kAsync))); @@ -626,6 +622,10 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { FfiCallbackMetadata::TrampolineType::kSyncIsolateGroupBound))); __ j(EQUAL, &sync_isolate_group_bound_callback, Assembler::kNearJump); + __ testq(RAX, + Immediate(FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag)); + __ j(NOT_ZERO, &sync_callback_isolate_ownership, Assembler::kNearJump); + // Sync callback. The entry point contains the target function, so just call // it. DLRT_GetThreadForNativeCallbackTrampoline exited the safepoint, so // re-enter it afterwards. @@ -637,6 +637,44 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() { // Takes care to not clobber *any* registers (besides TMP). __ EnterFullSafepoint(); + __ jmp(&done); + + __ Bind(&sync_callback_isolate_ownership); + + __ call(TMP); + + // Exit the target isolate. + { + const RegisterSet return_registers( + (1 << CallingConventions::kReturnReg) | + (1 << CallingConventions::kSecondReturnReg), + 1 << CallingConventions::kReturnFpuReg); + __ PushRegisters(return_registers); + +#if defined(DART_TARGET_OS_FUCHSIA) + // TODO(https://dartbug.com/52579): Remove. + if (FLAG_precompiled_mode) { + GenerateLoadBSSEntry(BSS::Relocation::DLRT_ExitSyncCallbackTargetIsolate, + RAX, TMP); + } else { + __ movq(RAX, Immediate(reinterpret_cast( + DLRT_ExitSyncCallbackTargetIsolate))); + } +#else + GenerateLoadFfiCallbackMetadataRuntimeFunction( + FfiCallbackMetadata::kExitSyncCallbackTargetIsolate, RAX); +#endif // defined(DART_TARGET_OS_FUCHSIA) + + __ EnterFrame(0); + __ ReserveAlignedFrameSpace(0); + + __ CallCFunction(RAX); + + __ LeaveFrame(); + + __ PopRegisters(return_registers); + } + __ jmp(&done, Assembler::kNearJump); __ Bind(&sync_isolate_group_bound_callback); diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index b710c734504..24c0eb33718 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -2232,6 +2232,16 @@ DART_EXPORT void Dart_SetCurrentThreadOwnsIsolate() { } } +DART_EXPORT void Dart_ClearCurrentThreadOwnsIsolate_ForTesting() { + Isolate* isolate = Isolate::Current(); + CHECK_ISOLATE(isolate); + if (!isolate->SetOwnerThread(OSThread::GetCurrentThreadId(), + OSThread::kInvalidThreadId)) { + FATAL("Tried to clear ownership of isolate %s, but we don't own it\n", + isolate->name()); + } +} + DART_EXPORT bool Dart_GetCurrentThreadOwnsIsolate(Dart_Port port) { return PortMap::IsOwnedByCurrentThread(port); } diff --git a/runtime/vm/ffi_callback_metadata.cc b/runtime/vm/ffi_callback_metadata.cc index 403376770c4..bba57d227d6 100644 --- a/runtime/vm/ffi_callback_metadata.cc +++ b/runtime/vm/ffi_callback_metadata.cc @@ -235,6 +235,9 @@ void FfiCallbackMetadata::EnsureFreeListNotEmptyLocked() { FillRuntimeFunction( new_page, kExitIsolateGroupBoundIsolate, reinterpret_cast(DLRT_ExitIsolateGroupBoundIsolate)); + FillRuntimeFunction( + new_page, kExitSyncCallbackTargetIsolate, + reinterpret_cast(DLRT_ExitSyncCallbackTargetIsolate)); // Add all the trampolines to the free list. const intptr_t trampolines_per_page = NumCallbackTrampolinesPerPage(); diff --git a/runtime/vm/ffi_callback_metadata.h b/runtime/vm/ffi_callback_metadata.h index 1502e6c5c3c..86bbc560874 100644 --- a/runtime/vm/ffi_callback_metadata.h +++ b/runtime/vm/ffi_callback_metadata.h @@ -50,10 +50,21 @@ class FfiCallbackMetadata { kSyncIsolateGroupBoundStackDelta4 = 4, // Only used by TARGET_ARCH_IA32 }; + // There are 2 supported invocation flows for kSync callbacks. The normal flow + // is when the current thread is already entered into the target isolate. The + // other flow is when the current thread is not entered into any isolate, but + // it owns the target isolate. In the latter case, GetFfiCallbackMetadata + // enters the target isolate. It also ORs this flag onto out_trampoline_type + // so that the invocation stub knows to exit the isolate again after calling + // the target callback. So this flag must not collide with TrampolineType + // values, and on 32-bit arm it needs to fit in a uint8. + static constexpr uword kSyncCallbackIsolateOwnershipFlag = 1 << 7; + enum RuntimeFunctions { kGetFfiCallbackMetadata, kExitTemporaryIsolate, kExitIsolateGroupBoundIsolate, + kExitSyncCallbackTargetIsolate, kNumRuntimeFunctions, }; @@ -317,27 +328,27 @@ class FfiCallbackMetadata { #if defined(TARGET_ARCH_X64) static constexpr intptr_t kNativeCallbackTrampolineSize = 12; - static constexpr intptr_t kNativeCallbackSharedStubSize = 376; + static constexpr intptr_t kNativeCallbackSharedStubSize = 393; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2; #elif defined(TARGET_ARCH_IA32) static constexpr intptr_t kNativeCallbackTrampolineSize = 10; - static constexpr intptr_t kNativeCallbackSharedStubSize = 193; + static constexpr intptr_t kNativeCallbackSharedStubSize = 241; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 4; #elif defined(TARGET_ARCH_ARM) static constexpr intptr_t kNativeCallbackTrampolineSize = 8; - static constexpr intptr_t kNativeCallbackSharedStubSize = 328; + static constexpr intptr_t kNativeCallbackSharedStubSize = 400; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 4; #elif defined(TARGET_ARCH_ARM64) static constexpr intptr_t kNativeCallbackTrampolineSize = 8; - static constexpr intptr_t kNativeCallbackSharedStubSize = 428; + static constexpr intptr_t kNativeCallbackSharedStubSize = 480; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2; #elif defined(TARGET_ARCH_RISCV32) static constexpr intptr_t kNativeCallbackTrampolineSize = 8; - static constexpr intptr_t kNativeCallbackSharedStubSize = 302; + static constexpr intptr_t kNativeCallbackSharedStubSize = 358; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2; #elif defined(TARGET_ARCH_RISCV64) static constexpr intptr_t kNativeCallbackTrampolineSize = 8; - static constexpr intptr_t kNativeCallbackSharedStubSize = 302; + static constexpr intptr_t kNativeCallbackSharedStubSize = 358; static constexpr intptr_t kNativeCallbackTrampolineStackDelta = 2; #else #error What architecture? diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc index 2f8513c3373..2a66364c4bc 100644 --- a/runtime/vm/runtime_entry.cc +++ b/runtime/vm/runtime_entry.cc @@ -5018,70 +5018,88 @@ Thread* HandleAsyncFfiCallback(FfiCallbackMetadata::Metadata metadata, return temp_thread; } -Thread* HandleSyncFfiCallback(FfiCallbackMetadata::Metadata metadata, - uword* out_entry_point, - uword* out_trampoline_type) { +Thread* HandleIsolateGroupBoundSyncFfiCallback( + FfiCallbackMetadata::Metadata metadata, + uword* out_entry_point, + uword* out_trampoline_type) { Thread* current_thread = Thread::Current(); - if (metadata.is_isolate_group_bound()) { - *out_entry_point = metadata.target_entry_point(); - *out_trampoline_type = static_cast(metadata.trampoline_type()); - } else { - Isolate* target_isolate = metadata.target_isolate(); - *out_entry_point = metadata.target_entry_point(); - *out_trampoline_type = static_cast(metadata.trampoline_type()); - if (current_thread == nullptr) { - FATAL("Cannot invoke native callback outside an isolate."); - } - if (current_thread->no_callback_scope_depth() != 0) { - FATAL("Cannot invoke native callback when API callbacks are prohibited."); - } - if (current_thread->is_unwind_in_progress()) { - FATAL("Cannot invoke native callback while unwind error propagates."); - } - if (!current_thread->IsDartMutatorThread()) { - FATAL("Native callbacks must be invoked on the mutator thread."); - } - if (current_thread->isolate() != target_isolate) { - FATAL("Cannot invoke native callback from a different isolate."); - } - if (current_thread->execution_state() != Thread::kThreadInNative) { - FATAL("Cannot invoke native callback from a leaf call."); - } - } + *out_entry_point = metadata.target_entry_point(); + *out_trampoline_type = static_cast(metadata.trampoline_type()); if (current_thread != nullptr) { current_thread->ExitSafepointFromNative(); current_thread->set_execution_state(Thread::kThreadInVM); } - if (metadata.is_isolate_group_bound()) { - Isolate* current_isolate = - current_thread != nullptr ? current_thread->isolate() : nullptr; + Isolate* current_isolate = + current_thread != nullptr ? current_thread->isolate() : nullptr; - if (current_thread != nullptr) { - Thread::ExitIsolate(/*isolate_shutdown=*/false); - } - Thread::EnterIsolateGroupAsMutator(metadata.target_isolate_group(), - /*bypass_safepoint=*/false); - auto new_thread = Thread::Current(); - new_thread->set_execution_state(Thread::kThreadInVM); - // We need to go back to current thread after we come back from - // the callback. - new_thread->set_unboxed_int64_runtime_arg( - reinterpret_cast(current_thread)); - new_thread->set_unboxed_int64_runtime_second_arg( - reinterpret_cast(current_isolate)); - current_thread = new_thread; + if (current_thread != nullptr) { + Thread::ExitIsolate(/*isolate_shutdown=*/false); } + Thread::EnterIsolateGroupAsMutator(metadata.target_isolate_group(), + /*bypass_safepoint=*/false); + auto new_thread = Thread::Current(); + new_thread->set_execution_state(Thread::kThreadInVM); + // We need to go back to current thread after we come back from + // the callback. + new_thread->set_unboxed_int64_runtime_arg( + reinterpret_cast(current_thread)); + new_thread->set_unboxed_int64_runtime_second_arg( + reinterpret_cast(current_isolate)); + current_thread = new_thread; current_thread->set_unboxed_int64_runtime_arg(metadata.context()); - TRACE_RUNTIME_CALL("GetFfiCallbackMetadata thread %p", current_thread); - TRACE_RUNTIME_CALL("GetFfiCallbackMetadata entry_point %p", - (void*)*out_entry_point); - TRACE_RUNTIME_CALL("GetFfiCallbackMetadata trampoline_type %p", - (void*)*out_trampoline_type); + return current_thread; +} + +void FfiCallbackThreadChecks(Thread* thread, Isolate* target_isolate) { + if (thread->no_callback_scope_depth() != 0) { + FATAL("Cannot invoke native callback when API callbacks are prohibited."); + } + if (thread->is_unwind_in_progress()) { + FATAL("Cannot invoke native callback while unwind error propagates."); + } + if (!thread->IsDartMutatorThread()) { + FATAL("Native callbacks must be invoked on the mutator thread."); + } + if (thread->isolate() != target_isolate) { + FATAL("Cannot invoke native callback from a different isolate."); + } +} + +Thread* HandleIsolateBoundSyncFfiCallback( + FfiCallbackMetadata::Metadata metadata, + uword* out_entry_point, + uword* out_trampoline_type) { + Thread* current_thread = Thread::Current(); + + *out_entry_point = metadata.target_entry_point(); + *out_trampoline_type = static_cast(metadata.trampoline_type()); + + Isolate* target_isolate = metadata.target_isolate(); + if (current_thread == nullptr) { + if (!PortMap::IsOwnedByCurrentThread(target_isolate->main_port())) { + FATAL("Cannot invoke native callback outside an isolate."); + } + Thread::EnterIsolate(target_isolate); + current_thread = Thread::Current(); + *out_trampoline_type |= + FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag; + FfiCallbackThreadChecks(current_thread, target_isolate); + } else { + FfiCallbackThreadChecks(current_thread, target_isolate); + if (current_thread->execution_state() != Thread::kThreadInNative) { + FATAL("Cannot invoke native callback from a leaf call."); + } + current_thread->ExitSafepointFromNative(); + } + + current_thread->set_execution_state(Thread::kThreadInVM); + current_thread->set_unboxed_int64_runtime_arg(metadata.context()); + return current_thread; } } // namespace @@ -5122,14 +5140,25 @@ extern "C" Thread* DLRT_GetFfiCallbackMetadata( FATAL("Callback invoked after it has been deleted."); } + Thread* thread = nullptr; if (metadata.trampoline_type() == FfiCallbackMetadata::TrampolineType::kAsync) { - return HandleAsyncFfiCallback(metadata, out_entry_point, - out_trampoline_type); + thread = + HandleAsyncFfiCallback(metadata, out_entry_point, out_trampoline_type); + } else if (metadata.is_isolate_group_bound()) { + thread = HandleIsolateGroupBoundSyncFfiCallback(metadata, out_entry_point, + out_trampoline_type); } else { - return HandleSyncFfiCallback(metadata, out_entry_point, - out_trampoline_type); + thread = HandleIsolateBoundSyncFfiCallback(metadata, out_entry_point, + out_trampoline_type); } + + TRACE_RUNTIME_CALL("GetFfiCallbackMetadata thread %p", thread); + TRACE_RUNTIME_CALL("GetFfiCallbackMetadata entry_point %p", + (void*)*out_entry_point); + TRACE_RUNTIME_CALL("GetFfiCallbackMetadata trampoline_type %p", + (void*)*out_trampoline_type); + return thread; } extern "C" void DLRT_ExitIsolateGroupBoundIsolate() { @@ -5147,6 +5176,14 @@ extern "C" void DLRT_ExitIsolateGroupBoundIsolate() { } } +extern "C" void DLRT_ExitSyncCallbackTargetIsolate() { + TRACE_RUNTIME_CALL("ExitSyncCallbackTargetIsolate%s", ""); + Thread* thread = Thread::Current(); + ASSERT(thread != nullptr); + thread->set_execution_state(Thread::kThreadInVM); + Thread::ExitIsolate(/*isolate_shutdown=*/false); +} + extern "C" void DLRT_ExitTemporaryIsolate() { TRACE_RUNTIME_CALL("ExitTemporaryIsolate%s", ""); Thread* thread = Thread::Current(); diff --git a/runtime/vm/runtime_entry.h b/runtime/vm/runtime_entry.h index c54adba84d4..d15a018c6f1 100644 --- a/runtime/vm/runtime_entry.h +++ b/runtime/vm/runtime_entry.h @@ -111,6 +111,7 @@ extern "C" Thread* DLRT_GetFfiCallbackMetadata(uword trampoline, uword* out_callback_kind); extern "C" void DLRT_ExitTemporaryIsolate(); extern "C" void DLRT_ExitIsolateGroupBoundIsolate(); +extern "C" void DLRT_ExitSyncCallbackTargetIsolate(); const char* DeoptReasonToCString(ICData::DeoptReasonId deopt_reason); diff --git a/runtime/vm/simulator_arm64.cc b/runtime/vm/simulator_arm64.cc index ba84fedcaa4..8359065a112 100644 --- a/runtime/vm/simulator_arm64.cc +++ b/runtime/vm/simulator_arm64.cc @@ -1915,6 +1915,10 @@ void Simulator::DoRedirectedFfiCallback(Thread* thread, if (trampoline_type == static_cast(FfiCallbackMetadata::TrampolineType::kAsync)) { DLRT_ExitTemporaryIsolate(); + } else if ((trampoline_type & + FfiCallbackMetadata::kSyncCallbackIsolateOwnershipFlag) != 0) { + thread->set_execution_state(Thread::kThreadInVM); + Thread::ExitIsolate(/*isolate_shutdown=*/false); } else { thread->EnterSafepointToNative(); } diff --git a/tests/ffi/ffi.status b/tests/ffi/ffi.status index 9e024429e38..020c7bd48f6 100644 --- a/tests/ffi/ffi.status +++ b/tests/ffi/ffi.status @@ -56,6 +56,7 @@ vmspecific_native_finalizer_isolate_groups_test: Skip # SpawnUri not available o async_void_function_callbacks_test/*: Skip # Test harness doesn't support multitest with Fuchsia exceptional_return_const_test/*: Skip # Test harness doesn't support multitest with Fuchsia ffi_induce_a_crash_test/*: Skip # Test harness doesn't support multitest with Fuchsia +function_callbacks_isolate_ownership_test: SkipByDesign # Needs access to Dart executable function_callbacks_leaf_test: SkipByDesign # Needs access to Dart executable function_callbacks_many_test/*: Skip # Test harness doesn't support multitest with Fuchsia function_callbacks_structs_by_value_generated_test/*: Skip # Test harness doesn't support multitest with Fuchsia diff --git a/tests/ffi/function_callbacks_isolate_ownership_test.dart b/tests/ffi/function_callbacks_isolate_ownership_test.dart new file mode 100644 index 00000000000..b5f5e6cbbf9 --- /dev/null +++ b/tests/ffi/function_callbacks_isolate_ownership_test.dart @@ -0,0 +1,63 @@ +// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Dart test program for testing dart:ffi async callbacks. +// +// VMOptions= +// VMOptions=--use-slow-path +// VMOptions=--use-slow-path --stacktrace-every=100 +// VMOptions=--dwarf_stack_traces --no-retain_function_objects --no-retain_code_objects +// VMOptions=--test_il_serialization +// VMOptions=--profiler --profile_vm=true +// VMOptions=--profiler --profile_vm=false +// SharedObjects=ffi_test_functions + +import 'dart:ffi'; + +import 'dart:io'; + +import 'package:expect/expect.dart'; + +import 'dylib_utils.dart'; + +main() { + testNativeCallableHelloWorld(); + + print('All tests completed :)'); +} + +int simpleFunction(int a, int b) { + return a + b; +} + +Future testNativeCallableHelloWorld() async { + final callback = NativeCallable.isolateLocal( + simpleFunction, + exceptionalReturn: 0, + ); + + final result = callTwoIntFunctionIsolateOwnership( + clearCurrentThreadOwnsIsolatePointer, + callback.nativeFunction, + 123, + 1000, + ); + + Expect.equals(1123, result); + callback.close(); +} + +final ffiTestFunctions = dlopenPlatformSpecific('ffi_test_functions'); + +typedef FnRunnerNativeType = Int32 Function(Pointer, Pointer, Int32, Int32); +typedef FnRunnerType = int Function(Pointer, Pointer, int, int); +final FnRunnerType callTwoIntFunctionIsolateOwnership = ffiTestFunctions + .lookupFunction( + 'CallTwoIntFunctionIsolateOwnership', + ); + +final Pointer clearCurrentThreadOwnsIsolatePointer = DynamicLibrary.process() + .lookup>( + 'Dart_ClearCurrentThreadOwnsIsolate_ForTesting', + );