[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 <[email protected]>
Commit-Queue: Daco Harkes <[email protected]>

Author: dcharkes

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" ]

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",

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);

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

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<intptr_t>(&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<intptr_t>(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<intptr_t>(FfiResolve));
-}
-
 DEFINE_FFI_NATIVE_ENTRY(FinalizerEntry_SetExternalSize,
                         void,
                         (Dart_Handle entry_handle, intptr_t external_size)) {

runtime/lib/ffi_dynamic_library.cc

@@ -11,11 +11,12 @@
 #include <tchar.h>
 #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 : "<process>", 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 : "<process>", 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<uword>(ResolveSymbol(handle, argSymbolName.ToCString()));
+  char* error = nullptr;
+  const uword pointer = reinterpret_cast<uword>(
+      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<intptr_t>(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<intptr_t>(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<intptr_t>(FfiResolve));
+}
+
+#endif  // defined(USING_SIMULATOR) ||                                         \
+        // (defined(DART_PRECOMPILER) && !defined(TESTING))
 
 }  // namespace dart

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(); }

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) {