[monodroid] Properly process satellite assemblies (#7823)

Fixes: #7819

Our native runtime uses a cache of pointers to loaded managed assembly
images (essentially an array of the native `struct MonoImage` pointers)
which is pre-allocated at build time and placed in the native
`libxamarin-app.so` library.

While generating the cache, we also generate hashes for a number of
assembly name permutations (currently two per assembly: with and
without the extension).  Only unique assembly names are considered when
generating the cache (it's possible to have duplicate names, because we
package more than one copy of some assemblies - those which are
architecture specific).

This algorithm had a bug which made it ignore culture prefix in
satellite assembly names (e.g. `en/MyAssembly.resources.dll`);
instead of several entries for each culture, we generated only two
entries (e.g. `MyAssembly.resources.dll` and `MyAssembly.resources`)
but we still counted each culture-prefixed assembly and stored that
number in `libxamarin-app.so` to be used at runtime to calculate number
of entries in the cache.

This made the array storing cached `MonoImage*` pointers to be smaller
than the number of actual assemblies in the APK times 2 and in some
cases we failed to look up pointer to some images and, as the result,
passed a `NULL` pointer to MonoVM which then caused a segmentation fault
trying to dereference the pointer:

	F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x000000000000001c
	F DEBUG   : Cause: null pointer dereference
	…
	F DEBUG   :       #00 pc 00000000000bdad8  /data/app/~~7gCkYmQcKwTyS9NmxfgKxA==/com.companyname.bubby-PJIkJ0Lv0RiQhEVLeWi4wg==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (mono_class_get_checked+24) (BuildId: 80b786675a56824331f363bf29a2b54f454cf006)

Update the `<BuildApk/>` task to stop ignoring the culture prefix for
satellite assemblies in order to avoid the situation.  Additionally,
since the previous assumption that MonoVM will validate all pointers
passed to its APIs turned out to be unwarranted, we now check more
carefully for `NULL` pointers when trying to obtain a native function
pointer from the MonoVM runtime.

Add a test for the issue, based on the
`MissingSatelliteAssemblyInLibrary()` packaging test which, when it
fails, will result in a `SIGABRT`:

	D monodroid-assembly: assembly_store_open_from_bundles: looking for bundled name: 'System.Private.CoreLib' (hash 0x6b0ff375198b9c17)
	F monodroid-assembly: Invalid assembly index 19, exceeds the maximum index of 11
	F libc    : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 17957 (semblyinlibrary), pid 17957 (semblyinlibrary)

Enhance the packaging test `MissingSatelliteAssemblyInLibrary()` by
adding more languages, to see if they're all packaged correctly.

src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs

@@ -265,7 +265,9 @@ void AddEnvironment ()
 			HashSet<string> archAssemblyNames = null;
 			HashSet<string> uniqueAssemblyNames = new HashSet<string> (StringComparer.OrdinalIgnoreCase);
 			Action<ITaskItem> updateAssemblyCount = (ITaskItem assembly) => {
-				string assemblyName = Path.GetFileName (assembly.ItemSpec);
+				// We need to use the 'RelativePath' metadata, if found, because it will give us the correct path for satellite assemblies - with the culture in the path.
+				string? relativePath = assembly.GetMetadata ("RelativePath");
+				string assemblyName = String.IsNullOrEmpty (relativePath) ? Path.GetFileName (assembly.ItemSpec) : relativePath;
 				if (!uniqueAssemblyNames.Contains (assemblyName)) {
 					uniqueAssemblyNames.Add (assemblyName);
 				}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs

@@ -807,12 +807,18 @@ public void MissingSatelliteAssemblyInLibrary ()
 					new BuildItem ("EmbeddedResource", "Foo.resx") {
 						TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancel</value></data>")
 					},
-					new BuildItem ("EmbeddedResource", "Foo.es.resx") {
-						TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancelar</value></data>")
-					}
 				}
 			};
 
+			var languages = new string[] {"es", "de", "fr", "he", "it", "pl", "pt", "ru", "sl" };
+			foreach (string lang in languages) {
+				lib.OtherBuildItems.Add (
+					new BuildItem ("EmbeddedResource", $"Foo.{lang}.resx") {
+						TextContent = () => InlineData.ResxWithContents ($"<data name=\"CancelButton\"><value>{lang}</value></data>")
+					}
+				);
+			}
+
 			var app = new XamarinAndroidApplicationProject {
 				IsRelease = true,
 			};
@@ -829,7 +835,10 @@ public void MissingSatelliteAssemblyInLibrary ()
 				var apk = Path.Combine (Root, appBuilder.ProjectDirectory,
 					app.OutputPath, $"{app.PackageName}-Signed.apk");
 				var helper = new ArchiveAssemblyHelper (apk);
-				Assert.IsTrue (helper.Exists ($"assemblies/es/{lib.ProjectName}.resources.dll"), "Apk should contain satellite assemblies!");
+
+				foreach (string lang in languages) {
+					Assert.IsTrue (helper.Exists ($"assemblies/{lang}/{lib.ProjectName}.resources.dll"), $"Apk should contain satellite assembly for language '{lang}'!");
+				}
 			}
 		}
 

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

@@ -816,7 +816,8 @@ void WriteHashes<T> () where T: struct
 				uint index = 0;
 
 				foreach (string name in uniqueAssemblyNames) {
-					string clippedName = Path.GetFileNameWithoutExtension (name);
+					// We must make sure we keep the possible culture prefix, which will be treated as "directory" path here
+					string clippedName = Path.Combine (Path.GetDirectoryName (name) ?? String.Empty, Path.GetFileNameWithoutExtension (name));
 					ulong hashFull = HashName (name, is64Bit);
 					ulong hashClipped = HashName (clippedName, is64Bit);
 

src/monodroid/jni/xamarin-android-app-context.cc

@@ -13,7 +13,7 @@ MonodroidRuntime::get_method_name (uint32_t mono_image_index, uint32_t method_to
 {
 	uint64_t id = (static_cast<uint64_t>(mono_image_index) << 32) | method_token;
 
-	log_debug (LOG_ASSEMBLY, "Looking for name of method with id 0x%llx, in mono image at index %u", id, mono_image_index);
+	log_debug (LOG_ASSEMBLY, "MM: looking for name of method with id 0x%llx, in mono image at index %u", id, mono_image_index);
 	size_t i = 0;
 	while (mm_method_names[i].id != 0) {
 		if (mm_method_names[i].id == id) {
@@ -58,19 +58,15 @@ MonodroidRuntime::get_function_pointer (uint32_t mono_image_index, uint32_t clas
 	// We need to do that, as Mono APIs cannot be invoked from threads that aren't attached to the runtime.
 	mono_thread_attach (mono_get_root_domain ());
 
-	// We don't check for valid return values from image loader, class and method lookup because if any
-	// of them fails to find the requested entity, they will return `null`.  In consequence, we can pass
-	// these pointers without checking all the way to `mono_method_get_unmanaged_callers_only_ftnptr`, after
-	// which call we check for errors.  This saves some time (not much, but definitely more than zero)
 	MonoImage *image = MonoImageLoader::get_from_index (mono_image_index);
 	MarshalMethodsManagedClass &klass = marshal_methods_class_cache[class_index];
 	if (klass.klass == nullptr) {
-		klass.klass = mono_class_get (image, klass.token);
+		klass.klass = image != nullptr ? mono_class_get (image, klass.token) : nullptr;
 	}
 
-	MonoMethod *method = mono_get_method (image, method_token, klass.klass);
+	MonoMethod *method = klass.klass != nullptr ? mono_get_method (image, method_token, klass.klass) : nullptr;
 	MonoError error;
-	void *ret = mono_method_get_unmanaged_callers_only_ftnptr (method, &error);
+	void *ret = method != nullptr ? mono_method_get_unmanaged_callers_only_ftnptr (method, &error) : nullptr;
 
 	if (XA_LIKELY (ret != nullptr)) {
 		if constexpr (NeedsLocking) {

tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs

@@ -25,9 +25,56 @@ public void Teardown ()
 				Directory.Delete (builder.ProjectDirectory, recursive: true);
 
 			builder?.Dispose ();
+			builder = null;
 			proj = null;
 		}
 
+		[Test]
+		public void NativeAssemblyCacheWithSatelliteAssemblies ()
+		{
+			var path = Path.Combine ("temp", TestName);
+			var lib = new XamarinAndroidLibraryProject {
+				ProjectName = "Localization",
+				OtherBuildItems = {
+					new BuildItem ("EmbeddedResource", "Foo.resx") {
+						TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancel</value></data>")
+					},
+				}
+			};
+
+			var languages = new string[] {"es", "de", "fr", "he", "it", "pl", "pt", "ru", "sl" };
+			foreach (string lang in languages) {
+				lib.OtherBuildItems.Add (
+					new BuildItem ("EmbeddedResource", $"Foo.{lang}.resx") {
+						TextContent = () => InlineData.ResxWithContents ($"<data name=\"CancelButton\"><value>{lang}</value></data>")
+					}
+				);
+			}
+
+			proj = new XamarinAndroidApplicationProject {
+				IsRelease = true,
+			};
+			proj.References.Add (new BuildItem.ProjectReference ($"..\\{lib.ProjectName}\\{lib.ProjectName}.csproj", lib.ProjectName, lib.ProjectGuid));
+			proj.SetAndroidSupportedAbis ("armeabi-v7a", "arm64-v8a", "x86", "x86_64");
+
+			using (var libBuilder = CreateDllBuilder (Path.Combine (path, lib.ProjectName))) {
+				builder = CreateApkBuilder (Path.Combine (path, proj.ProjectName));
+				Assert.IsTrue (libBuilder.Build (lib), "Library Build should have succeeded.");
+				Assert.IsTrue (builder.Install (proj), "Install should have succeeded.");
+
+				var apk = Path.Combine (Root, builder.ProjectDirectory, proj.OutputPath, $"{proj.PackageName}-Signed.apk");
+				var helper = new ArchiveAssemblyHelper (apk);
+
+				foreach (string lang in languages) {
+					Assert.IsTrue (helper.Exists ($"assemblies/{lang}/{lib.ProjectName}.resources.dll"), $"Apk should contain satellite assembly for language '{lang}'!");
+				}
+
+				Assert.True (builder.RunTarget (proj, "_Run"), "Project should have run.");
+				Assert.True (WaitForActivityToStart (proj.PackageName, "MainActivity",
+				                                     Path.Combine (Root, builder.ProjectDirectory, "logcat.log"), 30), "Activity should have started.");
+			}
+		}
+
 		[Test]
 		public void GlobalLayoutEvent_ShouldRegisterAndFire_OnActivityLaunch ([Values (false, true)] bool isRelease)
 		{