Skip to content

Commit 9f8e3d4

Browse files
authored
[native] Better p/invoke handling when loading foreign libraries (#10165)
Improve looking for "foreign" libraries when resolving a p/invoke symbol. Foreign libraries are those which either aren't contained in the application APK or which reside on the device in a system location. In such cases, we must attempt to load the library using its name passed from the managed code (which uses the `DllImport` attribute to name the library). Since `DllImport` doesn't usually use full name of the target library (e.g. instead of `liblog.so` it will use either `liblog` or `log`), we must modify the library name so that the `dlopen(3)` call can find it on the filesystem. We must make sure that the library name follows the `libNAME.so` convention before attempting to load the library from filesystem.. This is an interim fix for us not finding e.g. the `liblog.so` library when the `[DllImport("liblog")]` attribute is used in the managed code. The library resolution and reloading system must be redesigned, which will happen in a separate PR as it is currently not the highest priority task.
1 parent 8ade56f commit 9f8e3d4

File tree

6 files changed

+48
-12
lines changed

6 files changed

+48
-12
lines changed

src/native/clr/include/constants.hh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ namespace xamarin::android {
3232
#endif
3333
static constexpr std::string_view MANGLED_ASSEMBLY_NAME_EXT { ".so" };
3434
static constexpr std::string_view dso_suffix { ".so" };
35+
static constexpr std::string_view DSO_PREFIX { "lib" };
3536

3637
private:
3738
static constexpr std::string_view RUNTIME_CONFIG_BLOB_BASE_NAME { "libarc.bin" };

src/native/clr/include/host/pinvoke-override.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ namespace xamarin::android {
7979
if (lib_handle == nullptr) {
8080
// We're being called as part of the p/invoke mechanism, we don't need to look in the AOT cache
8181
constexpr bool PREFER_AOT_CACHE = false;
82-
lib_handle = MonodroidDl::monodroid_dlopen (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY, PREFER_AOT_CACHE);
82+
lib_handle = MonodroidDl::monodroid_dlopen<PREFER_AOT_CACHE> (library_name, microsoft::java_interop::JAVA_INTEROP_LIB_LOAD_LOCALLY);
8383
if (lib_handle == nullptr) {
8484
log_warn (LOG_ASSEMBLY, "Shared library '{}' not loaded, p/invoke '{}' may fail", optional_string (library_name), optional_string (symbol_name));
8585
return nullptr;

src/native/clr/include/runtime-base/monodroid-dl.hh

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,22 @@ namespace xamarin::android
9292

9393
static auto monodroid_dlopen_ignore_component_or_load (std::string_view const& name, int flags) noexcept -> void*
9494
{
95+
// We first try to load the DSO using the passed name, it will cause `dlopen` to search our APK (or
96+
// on-filesystem location), if necessary, so it's more efficient than trying to load from any specific
97+
// directories first.
9598
unsigned int dl_flags = static_cast<unsigned int>(flags);
96-
void * handle = AndroidSystem::load_dso_from_any_directories (name, dl_flags);
99+
void *handle = AndroidSystem::load_dso (name, dl_flags, false /* skip_existing_check */);
97100
if (handle != nullptr) {
98101
return monodroid_dlopen_log_and_return (handle, name);
99102
}
100103

101-
handle = AndroidSystem::load_dso (name, dl_flags, false /* skip_existing_check */);
104+
handle = AndroidSystem::load_dso_from_any_directories (name, dl_flags);
102105
return monodroid_dlopen_log_and_return (handle, name);
103106
}
104107

105108
public:
106-
[[gnu::flatten]]
107-
static auto monodroid_dlopen (std::string_view const& name, int flags, bool prefer_aot_cache) noexcept -> void*
109+
template<bool PREFER_AOT_CACHE> [[gnu::flatten]]
110+
static auto monodroid_dlopen (std::string_view const& name, int flags) noexcept -> void*
108111
{
109112
if (name.empty ()) [[unlikely]] {
110113
log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET+"sv);
@@ -115,7 +118,10 @@ namespace xamarin::android
115118
log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash for name '{}' is {:x}", name, name_hash);
116119

117120
DSOCacheEntry *dso = nullptr;
118-
if (prefer_aot_cache) {
121+
if constexpr (PREFER_AOT_CACHE) {
122+
// This code isn't currently used by CoreCLR, but it's possible that in the future we will have separate
123+
// .so files for AOT-d assemblies, similar to MonoVM, so let's keep it.
124+
//
119125
// If we're asked to look in the AOT DSO cache, do it first. This is because we're likely called from the
120126
// MonoVM's dlopen fallback handler and it will not be a request to resolved a p/invoke, but most likely to
121127
// 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
183189
// We're called by MonoVM via a callback, we might need to return an AOT DSO.
184190
// See: https://github.com/dotnet/android/issues/9081
185191
constexpr bool PREFER_AOT_CACHE = true;
186-
return monodroid_dlopen (name, flags, PREFER_AOT_CACHE);
192+
return monodroid_dlopen<PREFER_AOT_CACHE> (name, flags);
187193
}
188194

189195
[[gnu::flatten]]

src/native/clr/include/runtime-base/util.hh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,12 @@ namespace xamarin::android {
267267
return path[0] == '/';
268268
}
269269

270+
[[gnu::flatten, gnu::always_inline]]
271+
static auto path_has_directory_components (std::string_view const& path) noexcept -> bool
272+
{
273+
return !path.empty () && path.contains ('/');
274+
}
275+
270276
private:
271277
// TODO: needs some work to accept mixed params of different accepted types
272278
template<size_t MaxStackSpace, detail::PathBuffer<MaxStackSpace> TBuffer, detail::PathComponentString ...TPart>

src/native/clr/runtime-base/android-system.cc

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,14 +416,27 @@ auto AndroidSystem::get_full_dso_path (std::string const& base_dir, std::string_
416416
return false;
417417
}
418418

419+
dynamic_local_path_string lib_path { dso_path };
420+
bool have_so_extension = lib_path.ends_with (Constants::dso_suffix);
419421
if (base_dir.empty () || Util::is_path_rooted (dso_path)) {
422+
// Absolute path or no base path, can't do much with it
420423
path.assign (dso_path);
421-
return true; // Absolute path or no base path, can't do much with it
424+
if (!have_so_extension) {
425+
path.append (Constants::dso_suffix);
426+
}
427+
428+
return true;
422429
}
423430

424-
path.assign (base_dir)
425-
.append ("/"sv)
426-
.append (dso_path);
431+
path.assign (base_dir).append (Constants::DIR_SEP);
432+
433+
if (!Util::path_has_directory_components (dso_path) && !lib_path.starts_with (Constants::DSO_PREFIX)) {
434+
path.append (Constants::DSO_PREFIX);
435+
}
436+
path.append (dso_path);
437+
if (!have_so_extension) {
438+
path.append (Constants::dso_suffix);
439+
}
427440

428441
return true;
429442
}
@@ -450,7 +463,7 @@ auto AndroidSystem::load_dso (std::string_view const& path, unsigned int dl_flag
450463
return handle;
451464
}
452465

453-
template<class TContainer> [[gnu::always_inline]] // TODO: replace with a concept
466+
template<class TContainer> [[gnu::always_inline]]
454467
auto AndroidSystem::load_dso_from_specified_dirs (TContainer directories, std::string_view const& dso_name, unsigned int dl_flags) noexcept -> void*
455468
{
456469
if (dso_name.empty ()) {

src/native/common/include/runtime-base/strings.hh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,16 @@ namespace xamarin::android {
743743
return starts_with (s.data (), s.length ());
744744
}
745745

746+
[[gnu::always_inline]]
747+
auto ends_with (std::string_view const& s) noexcept -> bool
748+
{
749+
if (empty () || s.length () > buffer.size ()) {
750+
return false;
751+
}
752+
753+
return memcmp (buffer.get () + (length () - s.length ()), s.data (), s.length ()) == 0;
754+
}
755+
746756
[[gnu::always_inline]]
747757
void set_length_after_direct_write (size_t new_length) noexcept
748758
{

0 commit comments

Comments
 (0)