[native] Split up "regular" and AOT DSO caches (#9117)

Fixes: #9081

Context: dotnet/runtime#104397
Context: dotnet/runtime#106026
Context: #9081 (comment)
Context: c227042

("Compatibility is fun")

Consider a P/Invoke method declaration:

	[DllImport("libSkiaSharp")]
	static extern void gr_backendrendertarget_delete(IntPtr rendertarget);

Historically, when attempting to resolve this method, Mono would try
loading the following native libraries:

  * `libSkiaSharp`          (original name)
  * `libSkiaSharp.so`       (add `.so` suffix)
  * `liblibSkiaSharp`       (add `lib` prefix)
  * `liblibSkiaSharp.so`    (`lib` prefix + `.so` suffix)

.NET for Android would further permute these names, *removing* the
`lib` prefix, for attempted compatibility in case there is a P/Invoke
into `"SkiaSharp"`.

The unfortunate occasional result would be an *ambiguity*: when told
to resolve "SkiaSharp", what should we return?  The information for
`libSkiaSharp.so`, or for the *AOT'd image* of the assembly
`SkiaSharp.dll`, by way of `libaot-SkiaSharp.dll.so`?

        %struct.DSOCacheEntry {
                i64 u0x12e73d483788709d, ; from name: SkiaSharp.so
                i64 u0x3cb282562b838c95, ; uint64_t real_name_hash
                i1 false, ; bool ignore
                ptr @.DSOCacheEntry.23_name, ; name: libaot-SkiaSharp.dll.so
                ptr null; void* handle
        }, ; 71
        %struct.DSOCacheEntry {
                i64 u0x12e73d483788709d, ; from name: SkiaSharp.so
                i64 u0x43db119dcc3147fa, ; uint64_t real_name_hash
                i1 false, ; bool ignore
                ptr @.DSOCacheEntry.7_name, ; name: libSkiaSharp.so
                ptr null; void* handle
        }, ; 72

If we return the wrong thing, then the app may crash or otherwise
behave incorrectly.

Fix this by:

  * Splitting up the DSO cache into AOT-related `.so` files and
    everything else.
  * Updating `PinvokeOverride::load_library_symbol()` so that the AOT
    files are *not* consulted when resolving P/Invoke libraries.
  * Updating `MonodroidDl::monodroid_dlopen()` -- which is called by
    MonoVM via `mono_dl_fallback_register()` -- so that the AOT files
    *are* consulted *first* when resolving AOT images.

When dotnet/runtime#104397 is fixed, it will make the AOT side of the
split more efficient as we won't have to permute the shared library
name as many times as now.

Author: grendello

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs

@@ -58,6 +58,7 @@ public sealed class ApplicationConfig
 			public uint   bundled_assembly_name_width;
 			public uint   number_of_assembly_store_files;
 			public uint   number_of_dso_cache_entries;
+			public uint   number_of_aot_cache_entries;
 			public uint   android_runtime_jnienv_class_token;
 			public uint   jnienv_initialize_method_token;
 			public uint   jnienv_registerjninatives_method_token;
@@ -67,7 +68,7 @@ public sealed class ApplicationConfig
 			public string android_package_name = String.Empty;
 		}
 
-		const uint ApplicationConfigFieldCount = 26;
+		const uint ApplicationConfigFieldCount = 27;
 
 		const string ApplicationConfigSymbolName = "application_config";
 		const string AppEnvironmentVariablesSymbolName = "app_environment_variables";
@@ -301,37 +302,42 @@ static ApplicationConfig ReadApplicationConfig (EnvironmentFile envFile)
 						ret.number_of_dso_cache_entries = ConvertFieldToUInt32 ("number_of_dso_cache_entries", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 19: // android_runtime_jnienv_class_token: uint32_t / .word | .long
+					case 19: // number_of_aot_cache_entries: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
-						ret.number_of_dso_cache_entries = ConvertFieldToUInt32 ("android_runtime_jnienv_class_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
+						ret.number_of_aot_cache_entries = ConvertFieldToUInt32 ("number_of_aot_cache_entries", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 20: // jnienv_initialize_method_token: uint32_t / .word | .long
+					case 20: // android_runtime_jnienv_class_token: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
-						ret.number_of_dso_cache_entries = ConvertFieldToUInt32 ("jnienv_initialize_method_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
+						ret.android_runtime_jnienv_class_token = ConvertFieldToUInt32 ("android_runtime_jnienv_class_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 21: // jnienv_registerjninatives_method_token: uint32_t / .word | .long
+					case 21: // jnienv_initialize_method_token: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
-						ret.number_of_dso_cache_entries = ConvertFieldToUInt32 ("jnienv_registerjninatives_method_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
+						ret.jnienv_initialize_method_token = ConvertFieldToUInt32 ("jnienv_initialize_method_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 22: // jni_remapping_replacement_type_count: uint32_t / .word | .long
+					case 22: // jnienv_registerjninatives_method_token: uint32_t / .word | .long
+						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
+						ret.jnienv_registerjninatives_method_token = ConvertFieldToUInt32 ("jnienv_registerjninatives_method_token", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
+						break;
+
+					case 23: // jni_remapping_replacement_type_count: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
 						ret.jni_remapping_replacement_type_count = ConvertFieldToUInt32 ("jni_remapping_replacement_type_count", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 23: // jni_remapping_replacement_method_index_entry_count: uint32_t / .word | .long
+					case 24: // jni_remapping_replacement_method_index_entry_count: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
 						ret.jni_remapping_replacement_method_index_entry_count = ConvertFieldToUInt32 ("jni_remapping_replacement_method_index_entry_count", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 24: // mono_components_mask: uint32_t / .word | .long
+					case 25: // mono_components_mask: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
 						ret.mono_components_mask = ConvertFieldToUInt32 ("mono_components_mask", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
 						break;
 
-					case 25: // android_package_name: string / [pointer type]
+					case 26: // android_package_name: string / [pointer type]
 						Assert.IsTrue (expectedPointerTypes.Contains (field [0]), $"Unexpected pointer field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
 						pointers.Add (field [1].Trim ());
 						break;

src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfig.cs

@@ -42,6 +42,7 @@ sealed class ApplicationConfig
 		public uint   number_of_assemblies_in_apk;
 		public uint   bundled_assembly_name_width;
 		public uint   number_of_dso_cache_entries;
+		public uint   number_of_aot_cache_entries;
 		public uint   number_of_shared_libraries;
 
 		[NativeAssembler (NumberFormat = LLVMIR.LlvmIrVariableNumberFormat.Hexadecimal)]

src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs

@@ -155,6 +155,7 @@ sealed class XamarinAndroidBundledAssembly
 		SortedDictionary <string, string>? systemProperties;
 		StructureInstance? application_config;
 		List<StructureInstance<DSOCacheEntry>>? dsoCache;
+		List<StructureInstance<DSOCacheEntry>>? aotDsoCache;
 		List<StructureInstance<XamarinAndroidBundledAssembly>>? xamarinAndroidBundledAssemblies;
 
 		StructureInfo? applicationConfigStructureInfo;
@@ -218,7 +219,7 @@ protected override void Construct (LlvmIrModule module)
 			};
 			module.Add (sysProps, stringGroupName: "sysprop", stringGroupComment: " System properties name:value pairs");
 
-			dsoCache = InitDSOCache ();
+			(dsoCache, aotDsoCache) = InitDSOCache ();
 			var app_cfg = new ApplicationConfig {
 				uses_mono_llvm = UsesMonoLLVM,
 				uses_mono_aot = UsesMonoAOT,
@@ -239,6 +240,7 @@ protected override void Construct (LlvmIrModule module)
 				number_of_shared_libraries = (uint)NativeLibraries.Count,
 				bundled_assembly_name_width = (uint)BundledAssemblyNameWidth,
 				number_of_dso_cache_entries = (uint)dsoCache.Count,
+				number_of_aot_cache_entries = (uint)aotDsoCache.Count,
 				android_runtime_jnienv_class_token = (uint)AndroidRuntimeJNIEnvToken,
 				jnienv_initialize_method_token = (uint)JNIEnvInitializeToken,
 				jnienv_registerjninatives_method_token = (uint)JNIEnvRegisterJniNativesToken,
@@ -256,6 +258,12 @@ protected override void Construct (LlvmIrModule module)
 			};
 			module.Add (dso_cache);
 
+			var aot_dso_cache = new LlvmIrGlobalVariable (aotDsoCache, "aot_dso_cache", LlvmIrVariableOptions.GlobalWritable) {
+				Comment = " AOT DSO cache entries",
+				BeforeWriteCallback = HashAndSortDSOCache,
+			};
+			module.Add (aot_dso_cache);
+
 			var dso_apk_entries = new LlvmIrGlobalVariable (typeof(List<StructureInstance<DSOApkEntry>>), "dso_apk_entries") {
 				ArrayItemCount = (ulong)NativeLibraries.Count,
 				Options = LlvmIrVariableOptions.GlobalWritable,
@@ -337,7 +345,7 @@ void HashAndSortDSOCache (LlvmIrVariable variable, LlvmIrModuleTarget target, ob
 			cache.Sort ((StructureInstance<DSOCacheEntry> a, StructureInstance<DSOCacheEntry> b) => a.Instance.hash.CompareTo (b.Instance.hash));
 		}
 
-		List<StructureInstance<DSOCacheEntry>> InitDSOCache ()
+		(List<StructureInstance<DSOCacheEntry>> dsoCache, List<StructureInstance<DSOCacheEntry>> aotDsoCache) InitDSOCache ()
 		{
 			var dsos = new List<(string name, string nameLabel, bool ignore)> ();
 			var nameCache = new HashSet<string> (StringComparer.OrdinalIgnoreCase);
@@ -357,6 +365,7 @@ List<StructureInstance<DSOCacheEntry>> InitDSOCache ()
 			}
 
 			var dsoCache = new List<StructureInstance<DSOCacheEntry>> ();
+			var aotDsoCache = new List<StructureInstance<DSOCacheEntry>> ();
 			var nameMutations = new List<string> ();
 
 			for (int i = 0; i < dsos.Count; i++) {
@@ -372,11 +381,16 @@ List<StructureInstance<DSOCacheEntry>> InitDSOCache ()
 						name = name,
 					};
 
-					dsoCache.Add (new StructureInstance<DSOCacheEntry> (dsoCacheEntryStructureInfo, entry));
+					var item = new StructureInstance<DSOCacheEntry> (dsoCacheEntryStructureInfo, entry);
+					if (name.StartsWith ("libaot-", StringComparison.OrdinalIgnoreCase)) {
+						aotDsoCache.Add (item);
+					} else {
+						dsoCache.Add (item);
+					}
 				}
 			}
 
-			return dsoCache;
+			return (dsoCache, aotDsoCache);
 
 			void AddNameMutations (string name)
 			{

src/native/monodroid/CMakeLists.txt

@@ -60,7 +60,6 @@ set(XAMARIN_MONODROID_SOURCES
   monodroid-tracing.cc
   monovm-properties.cc
   osbridge.cc
-  pinvoke-override-api.cc
   runtime-util.cc
   timezones.cc
   xamarin_getifaddrs.cc

src/native/monodroid/pinvoke-override-api.cc

@@ -19,7 +19,9 @@ namespace xamarin::android {
 		void *lib_handle = dso_handle == nullptr ? nullptr : *dso_handle;
 
 		if (lib_handle == nullptr) {
-			lib_handle = internal::MonodroidDl::monodroid_dlopen (library_name, MONO_DL_LOCAL, nullptr, 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 = internal::MonodroidDl::monodroid_dlopen (library_name, MONO_DL_LOCAL, nullptr, PREFER_AOT_CACHE);
 			if (lib_handle == nullptr) {
 				log_warn (LOG_ASSEMBLY, "Shared library '%s' not loaded, p/invoke '%s' may fail", library_name, symbol_name);
 				return nullptr;

src/native/pinvoke-override/pinvoke-override-api-impl.hh

@@ -19,6 +19,15 @@ namespace xamarin::android::internal
 {
 	class MonodroidDl
 	{
+		enum class CacheKind
+		{
+			// Access AOT cache
+			AOT,
+
+			// Access DSO cache
+			DSO,
+		};
+
 		static inline xamarin::android::mutex   dso_handle_write_lock;
 
 		static unsigned int convert_dl_flags (int flags) noexcept
@@ -29,18 +38,48 @@ namespace xamarin::android::internal
 			return lflags;
 		}
 
-		static DSOCacheEntry* find_dso_cache_entry (hash_t hash) noexcept
+		template<CacheKind WhichCache>
+		[[gnu::always_inline, gnu::flatten]]
+		static DSOCacheEntry* find_dso_cache_entry_common (hash_t hash) noexcept
 		{
+			static_assert (WhichCache == CacheKind::AOT || WhichCache == CacheKind::DSO, "Unknown cache type specified");
+
+			DSOCacheEntry *arr;
+			size_t arr_size;
+
+			if constexpr (WhichCache == CacheKind::AOT) {
+				log_debug (LOG_ASSEMBLY, "Looking for hash 0x%x in AOT cache", hash);
+				arr = aot_dso_cache;
+				arr_size = application_config.number_of_aot_cache_entries;
+			} else if constexpr (WhichCache == CacheKind::DSO) {
+				log_debug (LOG_ASSEMBLY, "Looking for hash 0x%x in DSO cache", hash);
+				arr = dso_cache;
+				arr_size = application_config.number_of_dso_cache_entries;
+			}
+
 			auto equal = [](DSOCacheEntry const& entry, hash_t key) -> bool { return entry.hash == key; };
 			auto less_than = [](DSOCacheEntry const& entry, hash_t key) -> bool { return entry.hash < key; };
-			ssize_t idx = Search::binary_search<DSOCacheEntry, equal, less_than> (hash, dso_cache, application_config.number_of_dso_cache_entries);
+			ssize_t idx = Search::binary_search<DSOCacheEntry, equal, less_than> (hash, arr, arr_size);
+
 			if (idx >= 0) {
-				return &dso_cache[idx];
+				return &arr[idx];
 			}
 
 			return nullptr;
 		}
 
+		[[gnu::always_inline, gnu::flatten]]
+		static DSOCacheEntry* find_only_aot_cache_entry (hash_t hash) noexcept
+		{
+			return find_dso_cache_entry_common<CacheKind::AOT> (hash);
+		}
+
+		[[gnu::always_inline, gnu::flatten]]
+		static DSOCacheEntry* find_only_dso_cache_entry (hash_t hash) noexcept
+		{
+			return find_dso_cache_entry_common<CacheKind::DSO> (hash);
+		}
+
 		static void* monodroid_dlopen_log_and_return (void *handle, char **err, const char *full_name, bool free_memory)
 		{
 			if (handle == nullptr && err != nullptr) {
@@ -103,7 +142,7 @@ namespace xamarin::android::internal
 
 	public:
 		[[gnu::flatten]]
-		static void* monodroid_dlopen (const char *name, int flags, char **err, [[maybe_unused]] void *user_data) noexcept
+		static void* monodroid_dlopen (const char *name, int flags, char **err, bool prefer_aot_cache) noexcept
 		{
 			if (name == nullptr) {
 				log_warn (LOG_ASSEMBLY, "monodroid_dlopen got a null name. This is not supported in NET+");
@@ -112,7 +151,22 @@ namespace xamarin::android::internal
 
 			hash_t name_hash = xxhash::hash (name, strlen (name));
 			log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash for name '%s' is 0x%zx", name, name_hash);
-			DSOCacheEntry *dso = find_dso_cache_entry (name_hash);
+
+			DSOCacheEntry *dso = nullptr;
+			if (prefer_aot_cache) {
+				// 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
+				// scenario, we look at the AOT cache first.
+				//
+				// See: https://github.com/dotnet/android/issues/9081
+				dso = find_only_aot_cache_entry (name_hash);
+			}
+
+			if (dso == nullptr) {
+				dso = find_only_dso_cache_entry (name_hash);
+			}
+
 			log_debug (LOG_ASSEMBLY, "monodroid_dlopen: hash match %sfound, DSO name is '%s'", dso == nullptr ? "not " : "", dso == nullptr ? "<unknown>" : dso->name);
 
 			if (dso == nullptr) {
@@ -161,6 +215,15 @@ namespace xamarin::android::internal
 			return monodroid_dlopen_log_and_return (dso->handle, err, name, false /* name_needs_free */);
 		}
 
+		[[gnu::flatten]]
+		static void* monodroid_dlopen (const char *name, int flags, char **err, [[maybe_unused]] void *user_data) noexcept
+		{
+			// 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, err, PREFER_AOT_CACHE);
+		}
+
 		[[gnu::flatten]]
 		static void* monodroid_dlsym (void *handle, const char *name, char **err, [[maybe_unused]] void *user_data)
 		{

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

@@ -145,6 +145,24 @@ DSOCacheEntry dso_cache[] = {
 	},
 };
 
+DSOCacheEntry aot_dso_cache[] = {
+	{
+		.hash = xamarin::android::xxhash::hash (fake_dso_name, sizeof(fake_dso_name) - 1),
+		.real_name_hash = xamarin::android::xxhash::hash (fake_dso_name, sizeof(fake_dso_name) - 1),
+		.ignore = true,
+		.name = fake_dso_name,
+		.handle = nullptr,
+	},
+
+	{
+		.hash = xamarin::android::xxhash::hash (fake_dso_name2, sizeof(fake_dso_name2) - 1),
+		.real_name_hash = xamarin::android::xxhash::hash (fake_dso_name2, sizeof(fake_dso_name2) - 1),
+		.ignore = true,
+		.name = fake_dso_name2,
+		.handle = nullptr,
+	},
+};
+
 DSOApkEntry dso_apk_entries[2] {};
 
 //

src/native/xamarin-app-stub/application_dso_stub.cc

@@ -227,6 +227,9 @@ enum class MonoComponent : uint32_t
 	Tracing   = 0x04,
 };
 
+// Keep in strict sync with:
+//   src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfig.cs
+//   src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs
 struct ApplicationConfig
 {
 	bool uses_mono_llvm;
@@ -247,6 +250,7 @@ struct ApplicationConfig
 	uint32_t number_of_assemblies_in_apk;
 	uint32_t bundled_assembly_name_width;
 	uint32_t number_of_dso_cache_entries;
+	uint32_t number_of_aot_cache_entries;
 	uint32_t number_of_shared_libraries;
 	uint32_t android_runtime_jnienv_class_token;
 	uint32_t jnienv_initialize_method_token;
@@ -336,6 +340,7 @@ MONO_API MONO_API_EXPORT AssemblyStoreSingleAssemblyRuntimeData assembly_store_b
 MONO_API MONO_API_EXPORT AssemblyStoreRuntimeData assembly_store;
 
 MONO_API MONO_API_EXPORT DSOCacheEntry dso_cache[];
+MONO_API MONO_API_EXPORT DSOCacheEntry aot_dso_cache[];
 MONO_API MONO_API_EXPORT DSOApkEntry dso_apk_entries[];
 
 //