From f524ec74cef2c6be31c2bf626c11ec9c52f7ce75 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Tue, 25 Oct 2022 10:57:07 +0000 Subject: [PATCH] [vm/ffi] `FfiNative` process lookup This CL makes `FfiNative`s use `DynamicLibrary.process()` lookup if resolving with the resolver set by `Dart_SetFfiNativeResolver` fails. Also moves the implementation over from ffi.cc to ffi_dynamic_library.cc so the implementation can be shared with `DynamicLibrary.process()`. Moves the implementation behind non-simulator and non-precompiler. However, the implementation is tested in vm/cc tests which are in precompiler mode. So enables the implementation if TESTED is defined. This CL massages the build files so that TESTED is properly defined when compiling the runtime for the vm/cc tests, and links the ole32 symbols on windows for vm/cc tests. (And some unrelated small cleanup changes here and there.) TEST=tests/ffi/native_assets/process_test.dart Change-Id: I25395d381db1d9b4b7a5759171a798a1140a6140 Cq-Include-Trybots: luci.dart.try:vm-kernel-win-debug-x64c-try,vm-kernel-win-debug-x64-try,vm-kernel-nnbd-win-debug-x64-try,vm-kernel-precomp-win-debug-x64c-try,dart-sdk-win-try,vm-kernel-win-release-x64-try,vm-kernel-win-release-ia32-try,vm-kernel-precomp-win-product-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/264982 Reviewed-by: Martin Kustermann Commit-Queue: Daco Harkes --- runtime/BUILD.gn | 4 + runtime/bin/BUILD.gn | 5 +- runtime/bin/dartutils.h | 4 + runtime/configs.gni | 10 ++ runtime/lib/ffi.cc | 40 ----- runtime/lib/ffi_dynamic_library.cc | 169 +++++++++++++++++----- runtime/vm/dart_api_impl.h | 2 +- runtime/vm/dart_api_impl_test.cc | 11 +- runtime/vm/heap/heap_sources.gni | 2 - tests/ffi/native_assets/process_test.dart | 91 ++++++++++++ 10 files changed, 253 insertions(+), 85 deletions(-) create mode 100644 tests/ffi/native_assets/process_test.dart diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index c62a2b03cd6..13bfd0a724b 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -256,6 +256,10 @@ config("dart_config") { } } +config("dart_testing_config") { + defines = [ "TESTING" ] +} + config("dart_shared_lib") { if (dart_lib_export_symbols) { defines = [ "DART_SHARED_LIB" ] diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn index 4bd01e4ea5c..f597fa5b0c3 100644 --- a/runtime/bin/BUILD.gn +++ b/runtime/bin/BUILD.gn @@ -956,7 +956,7 @@ executable("run_vm_tests") { ":dart_kernel_platform_cc", ":dart_snapshot_cc", ":standalone_dart_io", - "..:libdart_precompiler", + "..:libdart_precompiler_testing", "//third_party/boringssl", # for secure_socket_utils_test "//third_party/zlib", ] @@ -1015,6 +1015,9 @@ executable("run_vm_tests") { if (is_win) { libs = [ + # ole32.dll contains CoTaskMemAlloc. Here so that package:ffi can look + # CoTaskMemAlloc up with `DynamicLibrary.process()`. + "ole32.lib", "iphlpapi.lib", "psapi.lib", "ws2_32.lib", diff --git a/runtime/bin/dartutils.h b/runtime/bin/dartutils.h index 694ea9ee88d..be2519eab98 100644 --- a/runtime/bin/dartutils.h +++ b/runtime/bin/dartutils.h @@ -152,6 +152,10 @@ class DartUtils { static bool IsDartBuiltinLibURL(const char* url_name); static bool IsHttpSchemeURL(const char* url_name); static const char* RemoveScheme(const char* url); + + // Retuns directory name including the last path separtor. + // + // The return value must be `free`d by the caller. static char* DirName(const char* url); static void* MapExecutable(const char* name, intptr_t* file_len); static void* OpenFile(const char* name, bool write); diff --git a/runtime/configs.gni b/runtime/configs.gni index 94ee5d3fd23..3388db00fcd 100644 --- a/runtime/configs.gni +++ b/runtime/configs.gni @@ -46,6 +46,9 @@ _precompiler_base = [ "$_dart_runtime:dart_precompiler_config" ] _precompiler_config = _base_config + _precompiler_base + _maybe_product +_precompiler_testing_config = + _precompiler_config + [ "$_dart_runtime:dart_testing_config" ] + _precompiler_product_config = _base_config + _precompiler_base + _product _precompiler_fuchsia_config = @@ -96,6 +99,13 @@ _all_configs = [ compiler = true is_product = false }, + { + suffix = "_precompiler_testing" + configs = _precompiler_testing_config + snapshot = false + compiler = true + is_product = false + }, { suffix = "_precompiler_product" configs = _precompiler_product_config diff --git a/runtime/lib/ffi.cc b/runtime/lib/ffi.cc index 16f4a4bb48f..803f2cc61b9 100644 --- a/runtime/lib/ffi.cc +++ b/runtime/lib/ffi.cc @@ -12,7 +12,6 @@ #include "vm/class_finalizer.h" #include "vm/class_id.h" #include "vm/compiler/ffi/native_type.h" -#include "vm/dart_api_impl.h" #include "vm/exceptions.h" #include "vm/flags.h" #include "vm/heap/gc_shared.h" @@ -206,45 +205,6 @@ DEFINE_NATIVE_ENTRY(DartApiDLInitializeData, 0, 0) { return Integer::New(reinterpret_cast(&dart_api_data)); } -// FFI native C function pointer resolver. -static intptr_t FfiResolve(Dart_Handle lib_url, - Dart_Handle name, - uintptr_t args_n) { - DARTSCOPE(Thread::Current()); - - const String& lib_url_str = Api::UnwrapStringHandle(T->zone(), lib_url); - const String& function_name = Api::UnwrapStringHandle(T->zone(), name); - - // Find the corresponding library's native function resolver (if set). - const Library& lib = Library::Handle(Library::LookupLibrary(T, lib_url_str)); - if (lib.IsNull()) { - const String& error = String::Handle(String::NewFormatted( - "Unknown library: '%s'.", lib_url_str.ToCString())); - Exceptions::ThrowArgumentError(error); - } - auto resolver = lib.ffi_native_resolver(); - if (resolver == nullptr) { - const String& error = String::Handle(String::NewFormatted( - "Library has no handler: '%s'.", lib_url_str.ToCString())); - Exceptions::ThrowArgumentError(error); - } - - auto* f = resolver(function_name.ToCString(), args_n); - if (f == nullptr) { - const String& error = String::Handle(String::NewFormatted( - "Couldn't resolve function: '%s'.", function_name.ToCString())); - Exceptions::ThrowArgumentError(error); - } - - return reinterpret_cast(f); -} - -// Bootstrap to get the FFI Native resolver through a `native` call. -DEFINE_NATIVE_ENTRY(Ffi_GetFfiNativeResolver, 1, 0) { - GET_NATIVE_TYPE_ARGUMENT(type_arg, arguments->NativeTypeArgAt(0)); - return Pointer::New(type_arg, reinterpret_cast(FfiResolve)); -} - DEFINE_FFI_NATIVE_ENTRY(FinalizerEntry_SetExternalSize, void, (Dart_Handle entry_handle, intptr_t external_size)) { diff --git a/runtime/lib/ffi_dynamic_library.cc b/runtime/lib/ffi_dynamic_library.cc index 528c6bfd15a..7bd6523c09b 100644 --- a/runtime/lib/ffi_dynamic_library.cc +++ b/runtime/lib/ffi_dynamic_library.cc @@ -11,11 +11,12 @@ #include #endif -#include "include/dart_api.h" #include "vm/bootstrap_natives.h" +#include "vm/dart_api_impl.h" #include "vm/exceptions.h" #include "vm/globals.h" #include "vm/native_entry.h" +#include "vm/object_store.h" #if defined(DART_HOST_OS_LINUX) || defined(DART_HOST_OS_MACOS) || \ defined(DART_HOST_OS_ANDROID) || defined(DART_HOST_OS_FUCHSIA) @@ -24,11 +25,15 @@ namespace dart { -#if defined(USING_SIMULATOR) || defined(DART_PRECOMPILER) +#if defined(USING_SIMULATOR) || (defined(DART_PRECOMPILER) && !defined(TESTING)) DART_NORETURN static void SimulatorUnsupported() { +#if defined(USING_SIMULATOR) Exceptions::ThrowUnsupportedError( "Not supported on simulated architectures."); +#else + Exceptions::ThrowUnsupportedError("Not supported in precompiler."); +#endif } DEFINE_NATIVE_ENTRY(Ffi_dl_open, 0, 1) { @@ -50,17 +55,26 @@ DEFINE_NATIVE_ENTRY(Ffi_dl_providesSymbol, 0, 2) { SimulatorUnsupported(); } -#else // defined(USING_SIMULATOR) || defined(DART_PRECOMPILER) +DEFINE_NATIVE_ENTRY(Ffi_GetFfiNativeResolver, 1, 0) { + SimulatorUnsupported(); +} -static void* LoadDynamicLibrary(const char* library_file) { - char* error = nullptr; - void* handle = Utils::LoadDynamicLibrary(library_file, &error); - if (error != nullptr) { - const String& msg = String::Handle(String::NewFormatted( - "Failed to load dynamic library '%s': %s", - library_file != nullptr ? library_file : "", error)); - free(error); - Exceptions::ThrowArgumentError(msg); +#else // defined(USING_SIMULATOR) || \ + // (defined(DART_PRECOMPILER) && !defined(TESTING)) + +// If an error occurs populates |error| (if provided) with an error message +// (caller must free this message when it is no longer needed). +static void* LoadDynamicLibrary(const char* library_file, + char** error = nullptr) { + char* utils_error = nullptr; + void* handle = Utils::LoadDynamicLibrary(library_file, &utils_error); + if (utils_error != nullptr) { + if (error != nullptr) { + *error = OS::SCreate( + /*use malloc*/ nullptr, "Failed to load dynamic library '%s': %s", + library_file != nullptr ? library_file : "", utils_error); + } + free(utils_error); } return handle; } @@ -71,6 +85,8 @@ const nullptr_t kWindowsDynamicLibraryProcessPtr = nullptr; void* co_task_mem_alloced = nullptr; +// If an error occurs populates |error| with an error message +// (caller must free this message when it is no longer needed). void* LookupSymbolInProcess(const char* symbol, char** error) { // Force loading ole32.dll. if (co_task_mem_alloced == nullptr) { @@ -101,31 +117,22 @@ void* LookupSymbolInProcess(const char* symbol, char** error) { CloseHandle(current_process); *error = OS::SCreate( - nullptr, + nullptr, // Use `malloc`. "None of the loaded modules contained the requested symbol '%s'.", symbol); return nullptr; } #endif -static void* ResolveSymbol(void* handle, const char* symbol) { - char* error = nullptr; -#if !defined(DART_HOST_OS_WINDOWS) - void* const result = - Utils::ResolveSymbolInDynamicLibrary(handle, symbol, &error); -#else - void* const result = - handle == kWindowsDynamicLibraryProcessPtr - ? LookupSymbolInProcess(symbol, &error) - : Utils::ResolveSymbolInDynamicLibrary(handle, symbol, &error); -#endif - if (error != nullptr) { - const String& msg = String::Handle(String::NewFormatted( - "Failed to lookup symbol '%s': %s", symbol, error)); - free(error); - Exceptions::ThrowArgumentError(msg); +// If an error occurs populates |error| with an error message +// (caller must free this message when it is no longer needed). +static void* ResolveSymbol(void* handle, const char* symbol, char** error) { +#if defined(DART_HOST_OS_WINDOWS) + if (handle == kWindowsDynamicLibraryProcessPtr) { + return LookupSymbolInProcess(symbol, error); } - return result; +#endif + return Utils::ResolveSymbolInDynamicLibrary(handle, symbol, error); } static bool SymbolExists(void* handle, const char* symbol) { @@ -149,8 +156,13 @@ static bool SymbolExists(void* handle, const char* symbol) { DEFINE_NATIVE_ENTRY(Ffi_dl_open, 0, 1) { GET_NON_NULL_NATIVE_ARGUMENT(String, lib_path, arguments->NativeArgAt(0)); - void* handle = LoadDynamicLibrary(lib_path.ToCString()); - + char* error = nullptr; + void* handle = LoadDynamicLibrary(lib_path.ToCString(), &error); + if (error != nullptr) { + const String& msg = String::Handle(String::New(error)); + free(error); + Exceptions::ThrowArgumentError(msg); + } return DynamicLibrary::New(handle); } @@ -176,8 +188,15 @@ DEFINE_NATIVE_ENTRY(Ffi_dl_lookup, 1, 2) { void* handle = dlib.GetHandle(); - const uword pointer = - reinterpret_cast(ResolveSymbol(handle, argSymbolName.ToCString())); + char* error = nullptr; + const uword pointer = reinterpret_cast( + ResolveSymbol(handle, argSymbolName.ToCString(), &error)); + if (error != nullptr) { + const String& msg = String::Handle(String::NewFormatted( + "Failed to lookup symbol '%s': %s", argSymbolName.ToCString(), error)); + free(error); + Exceptions::ThrowArgumentError(msg); + } return Pointer::New(type_arg, pointer); } @@ -197,6 +216,86 @@ DEFINE_NATIVE_ENTRY(Ffi_dl_providesSymbol, 0, 2) { return Bool::Get(SymbolExists(handle, argSymbolName.ToCString())).ptr(); } -#endif // defined(USING_SIMULATOR) +// nullptr if no native resolver is installed. +static Dart_FfiNativeResolver GetFfiNativeResolver(Thread* const thread, + const String& lib_url_str) { + const Library& lib = + Library::Handle(Library::LookupLibrary(thread, lib_url_str)); + if (lib.IsNull()) { + // It is not an error to not have a native resolver installed. + return nullptr; + } + return lib.ffi_native_resolver(); +} + +// If an error occurs populates |error| with an error message +// (caller must free this message when it is no longer needed). +static void* FfiResolveWithFfiNativeResolver(Thread* const thread, + Dart_FfiNativeResolver resolver, + const String& symbol, + intptr_t args_n, + char** error) { + auto* result = resolver(symbol.ToCString(), args_n); + if (result == nullptr) { + *error = OS::SCreate(/*use malloc*/ nullptr, + "Couldn't resolve function: '%s'", symbol.ToCString()); + } + return result; +} + +// Frees |error|. +static void ThrowFfiResolveError(const String& symbol, + const String& asset, + char* error) { + const String& error_message = String::Handle(String::NewFormatted( + "Couldn't resolve native function '%s' in '%s' : %s.\n", + symbol.ToCString(), asset.ToCString(), error)); + free(error); + Exceptions::ThrowArgumentError(error_message); +} + +// FFI native C function pointer resolver. +static intptr_t FfiResolve(Dart_Handle asset_handle, + Dart_Handle symbol_handle, + uintptr_t args_n) { + auto* const thread = Thread::Current(); + DARTSCOPE(thread); + auto* const zone = thread->zone(); + const String& asset = Api::UnwrapStringHandle(zone, asset_handle); + const String& symbol = Api::UnwrapStringHandle(zone, symbol_handle); + char* error = nullptr; + + // Resolver resolution. + auto resolver = GetFfiNativeResolver(thread, asset); + if (resolver != nullptr) { + void* ffi_native_result = FfiResolveWithFfiNativeResolver( + thread, resolver, symbol, args_n, &error); + if (error != nullptr) { + ThrowFfiResolveError(symbol, asset, error); + } + return reinterpret_cast(ffi_native_result); + } + + // Resolution in current process. +#if !defined(DART_HOST_OS_WINDOWS) + void* const result = Utils::ResolveSymbolInDynamicLibrary( + RTLD_DEFAULT, symbol.ToCString(), &error); +#else + void* const result = LookupSymbolInProcess(symbol.ToCString(), &error); +#endif + if (error != nullptr) { + ThrowFfiResolveError(symbol, asset, error); + } + return reinterpret_cast(result); +} + +// Bootstrap to get the FFI Native resolver through a `native` call. +DEFINE_NATIVE_ENTRY(Ffi_GetFfiNativeResolver, 1, 0) { + GET_NATIVE_TYPE_ARGUMENT(type_arg, arguments->NativeTypeArgAt(0)); + return Pointer::New(type_arg, reinterpret_cast(FfiResolve)); +} + +#endif // defined(USING_SIMULATOR) || \ + // (defined(DART_PRECOMPILER) && !defined(TESTING)) } // namespace dart diff --git a/runtime/vm/dart_api_impl.h b/runtime/vm/dart_api_impl.h index d38bf5833b0..0163fdc3660 100644 --- a/runtime/vm/dart_api_impl.h +++ b/runtime/vm/dart_api_impl.h @@ -176,7 +176,7 @@ class Api : AllStatic { // Casts the internal IsolateGroup* type to the external Dart_IsolateGroup // type. - static Dart_IsolateGroup CastIsolateGroup(IsolateGroup* isolate); + static Dart_IsolateGroup CastIsolateGroup(IsolateGroup* isolate_group); // Gets the handle used to designate successful return. static Dart_Handle Success() { return Api::True(); } diff --git a/runtime/vm/dart_api_impl_test.cc b/runtime/vm/dart_api_impl_test.cc index 0d0acbd50e2..421afb00ae9 100644 --- a/runtime/vm/dart_api_impl_test.cc +++ b/runtime/vm/dart_api_impl_test.cc @@ -10392,9 +10392,10 @@ TEST_CASE(Dart_SetFfiNativeResolver_MissingResolver) { EXPECT_VALID(lib); Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, nullptr); - EXPECT_ERROR( - result, - "Invalid argument(s): Library has no handler: 'file:///test-lib'."); + + // With no resolver, we resolve in process. Expect to not find the symbol + // in processes. Error message depends on architecture. + EXPECT_ERROR(result, "Invalid argument(s):"); } static void* NopResolver(const char* name, uintptr_t args_n) { @@ -10415,9 +10416,7 @@ TEST_CASE(Dart_SetFfiNativeResolver_DoesNotResolve) { EXPECT_VALID(result); result = Dart_Invoke(lib, NewString("main"), 0, nullptr); - EXPECT_ERROR( - result, - "Invalid argument(s): Couldn't resolve function: 'DoesNotResolve'"); + EXPECT_ERROR(result, "Couldn't resolve function: 'DoesNotResolve'"); } TEST_CASE(DartAPI_UserTags) { diff --git a/runtime/vm/heap/heap_sources.gni b/runtime/vm/heap/heap_sources.gni index 39a2ec93c67..1c93fa4a6e7 100644 --- a/runtime/vm/heap/heap_sources.gni +++ b/runtime/vm/heap/heap_sources.gni @@ -2,8 +2,6 @@ # 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. -# This file contains all sources (vm and tests) for the compiler pipeline. -# Unit test files need to have a "_test" suffix appended to the name. heap_sources = [ "become.cc", "become.h", diff --git a/tests/ffi/native_assets/process_test.dart b/tests/ffi/native_assets/process_test.dart new file mode 100644 index 00000000000..2fcae54f9b4 --- /dev/null +++ b/tests/ffi/native_assets/process_test.dart @@ -0,0 +1,91 @@ +// Copyright (c) 2022, 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. + +// Assumes we have no native asset mapping, so we look up the symbols in the +// process. + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:expect/expect.dart'; + +void main() { + testSuccess(); + testFailure(); + print('done'); +} + +void testSuccess() { + final p1 = malloc(); + Expect.notEquals(nullptr, p1); + malloc.free(p1); +} + +@FfiNative('malloc') +external Pointer posixMalloc(int size); + +@FfiNative('calloc') +external Pointer posixCalloc(int num, int size); + +@FfiNative('free') +external void posixFree(Pointer pointer); + +@FfiNative('CoTaskMemAlloc') +external Pointer winCoTaskMemAlloc(int cb); + +@FfiNative('CoTaskMemFree') +external void winCoTaskMemFree(Pointer pv); + +class _MallocAllocator implements Allocator { + const _MallocAllocator(); + + @override + Pointer allocate(int byteCount, {int? alignment}) { + Pointer result; + if (Platform.isWindows) { + result = winCoTaskMemAlloc(byteCount).cast(); + } else { + result = posixMalloc(byteCount).cast(); + } + if (result.address == 0) { + throw ArgumentError('Could not allocate $byteCount bytes.'); + } + return result; + } + + @override + void free(Pointer pointer) { + if (Platform.isWindows) { + winCoTaskMemFree(pointer); + } else { + posixFree(pointer); + } + } +} + +const Allocator malloc = _MallocAllocator(); + +void testFailure() { + Expect.throws(() { + symbolIsNotDefined(); + }); + try { + symbolIsNotDefined(); + } on ArgumentError catch (e) { + if (Platform.isWindows) { + Expect.contains( + 'None of the loaded modules contained the requested symbol', + e.message); + } else if (Platform.isMacOS || Platform.isIOS) { + Expect.contains('symbol_is_not_defined_29903211', e.message); + Expect.contains('symbol not found', e.message); + } else { + Expect.contains( + 'undefined symbol: symbol_is_not_defined_29903211', e.message); + } + } +} + +@FfiNative('symbol_is_not_defined_29903211') +external void symbolIsNotDefined();