From 5353114955441883a27697d6c0ff3786fb57e355 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 3 Jun 2025 15:49:41 +0200 Subject: [PATCH 1/4] Improve handling of p/invokes using "foreign" libraries --- src/native/clr/include/constants.hh | 1 + .../clr/include/runtime-base/monodroid-dl.hh | 18 ++++++++----- src/native/clr/include/runtime-base/util.hh | 6 +++++ src/native/clr/runtime-base/android-system.cc | 25 ++++++++++++++----- .../common/include/runtime-base/strings.hh | 10 ++++++++ 5 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/native/clr/include/constants.hh b/src/native/clr/include/constants.hh index e29c2fa5c32..14bd7b01270 100644 --- a/src/native/clr/include/constants.hh +++ b/src/native/clr/include/constants.hh @@ -32,6 +32,7 @@ namespace xamarin::android { #endif static constexpr std::string_view MANGLED_ASSEMBLY_NAME_EXT { ".so" }; static constexpr std::string_view dso_suffix { ".so" }; + static constexpr std::string_view DSO_PREFIX { "lib" }; private: static constexpr std::string_view RUNTIME_CONFIG_BLOB_BASE_NAME { "libarc.bin" }; diff --git a/src/native/clr/include/runtime-base/monodroid-dl.hh b/src/native/clr/include/runtime-base/monodroid-dl.hh index a0e0fe7392b..c6d0ec6fe19 100644 --- a/src/native/clr/include/runtime-base/monodroid-dl.hh +++ b/src/native/clr/include/runtime-base/monodroid-dl.hh @@ -92,19 +92,22 @@ namespace xamarin::android static auto monodroid_dlopen_ignore_component_or_load (std::string_view const& name, int flags) noexcept -> void* { + // We first try to load the DSO using the passed name, it will cause `dlopen` to search our APK (or + // on-filesystem location), if necessary, so it's more efficient than trying to load from any specific + // directories first. unsigned int dl_flags = static_cast(flags); - void * handle = AndroidSystem::load_dso_from_any_directories (name, dl_flags); + void *handle = AndroidSystem::load_dso (name, dl_flags, false /* skip_existing_check */); if (handle != nullptr) { return monodroid_dlopen_log_and_return (handle, name); } - handle = AndroidSystem::load_dso (name, dl_flags, false /* skip_existing_check */); + handle = AndroidSystem::load_dso_from_any_directories (name, dl_flags); return monodroid_dlopen_log_and_return (handle, name); } public: - [[gnu::flatten]] - static auto monodroid_dlopen (std::string_view const& name, int flags, bool prefer_aot_cache) noexcept -> void* + template [[gnu::flatten]] + static auto monodroid_dlopen (std::string_view const& name, int flags) noexcept -> void* { if (name.empty ()) [[unlikely]] { log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET+"sv); @@ -115,7 +118,10 @@ namespace xamarin::android log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash for name '{}' is {:x}", name, name_hash); DSOCacheEntry *dso = nullptr; - if (prefer_aot_cache) { + if constexpr (PREFER_AOT_CACHE) { + // This code isn't currently used by CoreCLR, but it's possible that in the future we will have separate + // .so files for AOT-d assemblies, similar to MonoVM, so let's keep it. + // // If we're asked to look in the AOT DSO cache, do it first. This is because we're likely called from the // MonoVM's dlopen fallback handler and it will not be a request to resolved a p/invoke, but most likely to // find and load an AOT image for a managed assembly. Since there might be naming/hash conflicts in this @@ -183,7 +189,7 @@ namespace xamarin::android // We're called by MonoVM via a callback, we might need to return an AOT DSO. // See: https://github.com/dotnet/android/issues/9081 constexpr bool PREFER_AOT_CACHE = true; - return monodroid_dlopen (name, flags, PREFER_AOT_CACHE); + return monodroid_dlopen (name, flags); } [[gnu::flatten]] diff --git a/src/native/clr/include/runtime-base/util.hh b/src/native/clr/include/runtime-base/util.hh index 95a64903a84..a301b841148 100644 --- a/src/native/clr/include/runtime-base/util.hh +++ b/src/native/clr/include/runtime-base/util.hh @@ -267,6 +267,12 @@ namespace xamarin::android { return path[0] == '/'; } + [[gnu::flatten, gnu::always_inline]] + static auto path_has_directory_components (std::string_view const& path) noexcept -> bool + { + return !path.empty () && path.contains ('/'); + } + private: // TODO: needs some work to accept mixed params of different accepted types template TBuffer, detail::PathComponentString ...TPart> diff --git a/src/native/clr/runtime-base/android-system.cc b/src/native/clr/runtime-base/android-system.cc index f88ee179a14..67bad0598eb 100644 --- a/src/native/clr/runtime-base/android-system.cc +++ b/src/native/clr/runtime-base/android-system.cc @@ -416,21 +416,34 @@ auto AndroidSystem::get_full_dso_path (std::string const& base_dir, std::string_ return false; } + dynamic_local_path_string lib_path { dso_path }; + bool have_so_extension = lib_path.ends_with (Constants::dso_suffix); if (base_dir.empty () || Util::is_path_rooted (dso_path)) { + // Absolute path or no base path, can't do much with it path.assign (dso_path); - return true; // Absolute path or no base path, can't do much with it + if (!have_so_extension) { + path.append (Constants::dso_suffix); + } + + return true; } - path.assign (base_dir) - .append ("/"sv) - .append (dso_path); + path.assign (base_dir).append (Constants::DIR_SEP); + + if (!Util::path_has_directory_components (dso_path) && !lib_path.starts_with (Constants::DSO_PREFIX)) { + path.append (Constants::DSO_PREFIX); + } + path.append (dso_path); + if (!have_so_extension) { + path.append (Constants::dso_suffix); + } return true; } auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void* { - if (path.empty ()) [[unlikely]] { + if (path.empty ()) { return nullptr; } @@ -450,7 +463,7 @@ auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flag return handle; } -template [[gnu::always_inline]] // TODO: replace with a concept +template [[gnu::always_inline]] auto AndroidSystem::load_dso_from_specified_dirs (TContainer directories, std::string_view const& dso_name, unsigned int dl_flags) noexcept -> void* { if (dso_name.empty ()) { diff --git a/src/native/common/include/runtime-base/strings.hh b/src/native/common/include/runtime-base/strings.hh index feef2ef4342..19f52d26fe7 100644 --- a/src/native/common/include/runtime-base/strings.hh +++ b/src/native/common/include/runtime-base/strings.hh @@ -743,6 +743,16 @@ namespace xamarin::android { return starts_with (s.data (), s.length ()); } + [[gnu::always_inline]] + auto ends_with (std::string_view const& s) noexcept -> bool + { + if (empty () || s.length () > buffer.size ()) { + return false; + } + + return memcmp (buffer.get () + (length () - s.length ()), s.data (), s.length ()) == 0; + } + [[gnu::always_inline]] void set_length_after_direct_write (size_t new_length) noexcept { From 3cbd96de3e06eb8fc7de54c958ca4423f3a43517 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 3 Jun 2025 17:36:27 +0200 Subject: [PATCH 2/4] API change --- src/native/clr/include/host/pinvoke-override.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/clr/include/host/pinvoke-override.hh b/src/native/clr/include/host/pinvoke-override.hh index e1b7afebd95..4e7cabf69bb 100644 --- a/src/native/clr/include/host/pinvoke-override.hh +++ b/src/native/clr/include/host/pinvoke-override.hh @@ -79,7 +79,7 @@ namespace xamarin::android { if (lib_handle == nullptr) { // We're being called as part of the p/invoke mechanism, we don't need to look in the AOT cache constexpr bool PREFER_AOT_CACHE = false; - lib_handle = MonodroidDl::monodroid_dlopen (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY, PREFER_AOT_CACHE); + lib_handle = MonodroidDl::monodroid_dlopen (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY); if (lib_handle == nullptr) { log_warn (LOG_ASSEMBLY, "Shared library '{}' not loaded, p/invoke '{}' may fail", optional_string (library_name), optional_string (symbol_name)); return nullptr; From 76fdc6578a8584a849bf49eaaa4a8ca06f3a2b26 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 4 Jun 2025 07:09:41 +0000 Subject: [PATCH 3/4] Update src/native/clr/runtime-base/android-system.cc Co-authored-by: Jonathan Peppers --- src/native/clr/runtime-base/android-system.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/clr/runtime-base/android-system.cc b/src/native/clr/runtime-base/android-system.cc index 67bad0598eb..ef011d2fc9a 100644 --- a/src/native/clr/runtime-base/android-system.cc +++ b/src/native/clr/runtime-base/android-system.cc @@ -443,7 +443,7 @@ auto AndroidSystem::get_full_dso_path (std::string const& base_dir, std::string_ auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flags, bool skip_exists_check) noexcept -> void* { - if (path.empty ()) { + if (path.empty ()) [[unlikely]] { return nullptr; } From 2ef003801d38c72be0eb73972ad8dc1f73bfa498 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 5 Jun 2025 09:42:07 +0200 Subject: [PATCH 4/4] Fix after rebase