Skip to content

[native] Better p/invoke handling when loading foreign libraries #10165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 5, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/native/clr/include/constants.hh
Original file line number Diff line number Diff line change
@@ -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" };
2 changes: 1 addition & 1 deletion src/native/clr/include/host/pinvoke-override.hh
Original file line number Diff line number Diff line change
@@ -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<PREFER_AOT_CACHE> (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;
18 changes: 12 additions & 6 deletions src/native/clr/include/runtime-base/monodroid-dl.hh
Original file line number Diff line number Diff line change
@@ -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<unsigned int>(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<bool PREFER_AOT_CACHE> [[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<PREFER_AOT_CACHE> (name, flags);
}

[[gnu::flatten]]
6 changes: 6 additions & 0 deletions src/native/clr/include/runtime-base/util.hh
Original file line number Diff line number Diff line change
@@ -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<size_t MaxStackSpace, detail::PathBuffer<MaxStackSpace> TBuffer, detail::PathComponentString ...TPart>
23 changes: 18 additions & 5 deletions src/native/clr/runtime-base/android-system.cc
Original file line number Diff line number Diff line change
@@ -416,14 +416,27 @@ 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;
}
@@ -450,7 +463,7 @@ auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flag
return handle;
}

template<class TContainer> [[gnu::always_inline]] // TODO: replace with a concept
template<class TContainer> [[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 ()) {
10 changes: 10 additions & 0 deletions src/native/common/include/runtime-base/strings.hh
Original file line number Diff line number Diff line change
@@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for me to understand, length() is always inlined, so no problem with calling it multiple times in an expression?

[[gnu::always_inline]]
auto length () const noexcept -> size_t

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's fully inlined each time it's called (which can also the be further optimized by common sub-expression elimination and other techniques, depending on context)

}

[[gnu::always_inline]]
void set_length_after_direct_write (size_t new_length) noexcept
{
Loading
Oops, something went wrong.