Make APK and shared library alignment configurable (#9046)

grendello · web-flow · commit ddb215b762d9 · 2024-06-20T19:28:17.000Z
Context: https://github.com/dotnet/android/wiki/Android-support-for-devices-with-16k-pages Context: #9020 Starting sometime next year Google will require all packages submitted to the Play Store to be aligned to 16 bytes, in order to support Android devices which use 16kb pages instead of the currently supported 4kb ones. The requirement also applies to native shared libaries (`.so`), which need to be linked so that their loaded sections are aligned to 16kb as well. This commit implements changes which make the alignment configurable, while still defaulting to 4kb alignment. The #9020 PR will enable (when NDK r27 is released) building of our runtime shared libraries with 16kb alignment. While strictly speaking NDK r27 is not necessary to enable 16kb alignment for the runtime (we could just modify our `CMakeLists.txt` script to pass the appropriate flag), I prefer to rely on the "official" support once NDK r27 is out. Linking of application shared libraries, however, doesn't use the NDK at all, and in this case we must pass the appropriate flag to the linker explicitly. This commit also prepares our runtime to verify alignment correctly when changing between 4k and 16k.
diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets
@@ -45,6 +45,15 @@
 
     <!-- Mono components -->
     <AndroidEnableProfiler Condition=" '$(AndroidEnableProfiler)' == ''">false</AndroidEnableProfiler>
+
+    <!--
+        Android package (apt/aab) alignment, expressed as the page size in kilobytes.  Two values are supported: 4 and 16.
+        Sometime next year the default value should be changed to 16 since it's going to be a Google Play store requirement for
+        application submissions.
+
+        When changing this default, change the value of `DefaultZipAlignment` in `src/Xamarin.Android.Build.Tasks/Tasks/AndroidZipAlign.cs` as well
+    -->
+    <_AndroidZipAlignment Condition=" '$(_AndroidZipAlignment)' == '' ">4</_AndroidZipAlignment>
   </PropertyGroup>
 
   <!--  User-facing configuration-specific defaults -->
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/AndroidZipAlign.cs b/src/Xamarin.Android.Build.Tasks/Tasks/AndroidZipAlign.cs
@@ -7,6 +7,10 @@ namespace Xamarin.Android.Tasks
 {
 	public class AndroidZipAlign : AndroidRunToolTask
 	{
+		// Sometime next year the default value should be changed to 16 since it's going to be a Google Play store requirement for
+		// application submissions
+		internal const int DefaultZipAlignment = 4;
+
 		public override string TaskPrefix => "AZA";
 
 		[Required]
@@ -15,7 +19,7 @@ public class AndroidZipAlign : AndroidRunToolTask
 		[Required]
 		public ITaskItem DestinationDirectory { get; set; }
 
-		int alignment = 4;
+		int alignment = DefaultZipAlignment;
 		public int Alignment {
 			get {return alignment;}
 			set {alignment = value;}
@@ -53,4 +57,3 @@ protected override void LogEventsFromTextOutput (string singleLine, MessageImpor
 		}
 	}
 }
-
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs
@@ -80,6 +80,7 @@ public class GeneratePackageManagerJava : AndroidTask
 		public string AndroidSequencePointsMode { get; set; }
 		public bool EnableSGenConcurrent { get; set; }
 		public string? CustomBundleConfigFile { get; set; }
+		public int ZipAlignmentPages { get; set; } = AndroidZipAlign.DefaultZipAlignment;
 
 		[Output]
 		public string BuildId { get; set; }
@@ -334,6 +335,7 @@ void AddEnvironment ()
 
 			bool haveRuntimeConfigBlob = !String.IsNullOrEmpty (RuntimeConfigBinFilePath) && File.Exists (RuntimeConfigBinFilePath);
 			var jniRemappingNativeCodeInfo = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal<GenerateJniRemappingNativeCode.JniRemappingNativeCodeInfo> (ProjectSpecificTaskObjectKey (GenerateJniRemappingNativeCode.JniRemappingNativeCodeInfoKey), RegisteredTaskObjectLifetime.Build);
+			uint zipAlignmentMask = MonoAndroidHelper.ZipAlignmentToMask (ZipAlignmentPages);
 			var appConfigAsmGen = new ApplicationConfigNativeAssemblyGenerator (environmentVariables, systemProperties, Log) {
 				UsesMonoAOT = usesMonoAOT,
 				UsesMonoLLVM = EnableLLVM,
@@ -357,6 +359,7 @@ void AddEnvironment ()
 				JNIEnvRegisterJniNativesToken = jnienv_registerjninatives_method_token,
 				JniRemappingReplacementTypeCount = jniRemappingNativeCodeInfo == null ? 0 : jniRemappingNativeCodeInfo.ReplacementTypeCount,
 				JniRemappingReplacementMethodIndexEntryCount = jniRemappingNativeCodeInfo == null ? 0 : jniRemappingNativeCodeInfo.ReplacementMethodIndexEntryCount,
+				ZipAlignmentMask = zipAlignmentMask,
 				MarshalMethodsEnabled = EnableMarshalMethods,
 				IgnoreSplitConfigs = ShouldIgnoreSplitConfigs (),
 			};
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkApplicationSharedLibraries.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkApplicationSharedLibraries.cs
@@ -44,6 +44,8 @@ sealed class InputFiles
 		[Required]
 		public string AndroidBinUtilsDirectory { get; set; }
 
+		public int ZipAlignmentPages { get; set; } = AndroidZipAlign.DefaultZipAlignment;
+
 		public override System.Threading.Tasks.Task RunTaskAsync ()
 		{
 			return this.WhenAll (GetLinkerConfigs (), RunLinker);
@@ -129,7 +131,6 @@ IEnumerable<Config> GetLinkerConfigs ()
 				"-soname libxamarin-app.so " +
 				"-z relro " +
 				"-z noexecstack " +
-				"-z max-page-size=4096 " +
 				"--enable-new-dtags " +
 				"--build-id " +
 				"--warn-shared-textrel " +
@@ -186,6 +187,10 @@ IEnumerable<Config> GetLinkerConfigs ()
 					}
 				}
 
+				uint maxPageSize = MonoAndroidHelper.ZipAlignmentToPageSize (ZipAlignmentPages);
+				targetLinkerArgs.Add ("-z");
+				targetLinkerArgs.Add ($"max-page-size={maxPageSize}");
+
 				string targetArgs = String.Join (" ", targetLinkerArgs);
 				yield return new Config {
 					LinkerPath = ld,
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs
@@ -63,11 +63,12 @@ public sealed class ApplicationConfig
 			public uint   jnienv_registerjninatives_method_token;
 			public uint   jni_remapping_replacement_type_count;
 			public uint   jni_remapping_replacement_method_index_entry_count;
+			public uint   zip_alignment_mask;
 			public uint   mono_components_mask;
 			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";
@@ -326,12 +327,17 @@ static ApplicationConfig ReadApplicationConfig (EnvironmentFile envFile)
 						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 24: // zip_alignment_mask: uint32_t / .word | .long
+						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile.Path}:{item.LineNumber}': {field [0]}");
+						ret.zip_alignment_mask = ConvertFieldToUInt32 ("zip_alignment_mask", envFile.Path, parser.SourceFilePath, item.LineNumber, field [1]);
+						break;
+
+					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;
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfig.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfig.cs
@@ -55,6 +55,9 @@ sealed class ApplicationConfig
 		public uint   jni_remapping_replacement_type_count;
 		public uint   jni_remapping_replacement_method_index_entry_count;
 
+		// 3, for 4-byte alignment (4k memory pages); 15, for 16-byte alignment (16k memory pages)
+		public uint   zip_alignment_mask;
+
 		[NativeAssembler (NumberFormat = LLVMIR.LlvmIrVariableNumberFormat.Hexadecimal)]
 		public uint   mono_components_mask;
 		public string android_package_name = String.Empty;
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs
@@ -183,6 +183,7 @@ sealed class XamarinAndroidBundledAssembly
 		public int JNIEnvRegisterJniNativesToken { get; set; }
 		public int JniRemappingReplacementTypeCount { get; set; }
 		public int JniRemappingReplacementMethodIndexEntryCount { get; set; }
+		public uint ZipAlignmentMask { get; set; }
 		public MonoComponent MonoComponents { get; set; }
 		public PackageNamingPolicy PackageNamingPolicy { get; set; }
 		public List<ITaskItem> NativeLibraries { get; set; }
@@ -244,6 +245,7 @@ protected override void Construct (LlvmIrModule module)
 				jnienv_registerjninatives_method_token = (uint)JNIEnvRegisterJniNativesToken,
 				jni_remapping_replacement_type_count = (uint)JniRemappingReplacementTypeCount,
 				jni_remapping_replacement_method_index_entry_count = (uint)JniRemappingReplacementMethodIndexEntryCount,
+				zip_alignment_mask = ZipAlignmentMask,
 				mono_components_mask = (uint)MonoComponents,
 				android_package_name = AndroidPackageName,
 			};
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
@@ -712,5 +712,22 @@ internal static void DumpMarshalMethodsToConsole (string heading, IDictionary<st
 				}
 			}
 		}
+
+		public static uint ZipAlignmentToMask (int alignment) => ZipAlignmentToMaskOrPageSize (alignment, needMask: true);
+		public static uint ZipAlignmentToPageSize (int alignment) => ZipAlignmentToMaskOrPageSize (alignment, needMask: false);
+
+		static uint ZipAlignmentToMaskOrPageSize (int alignment, bool needMask)
+		{
+			const uint pageSize4k = 4096;
+			const uint pageMask4k = 3;
+			const uint pageSize16k = 16384;
+			const uint pageMask16k = 15;
+
+			return alignment switch {
+				4  => needMask ? pageMask4k : pageSize4k,
+				16 => needMask ? pageMask16k : pageSize16k,
+				_  => throw new InvalidOperationException ($"Internal error: unsupported zip page alignment value {alignment}")
+			};
+		}
 	}
 }
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -1724,6 +1724,7 @@ because xbuild doesn't support framework reference assemblies.
     UseAssemblyStore="$(AndroidUseAssemblyStore)"
     EnableMarshalMethods="$(_AndroidUseMarshalMethods)"
     CustomBundleConfigFile="$(AndroidBundleConfigurationFile)"
+    ZipAlignmentPages="$(_AndroidZipAlignment)"
   >
     <Output TaskParameter="BuildId" PropertyName="_XamarinBuildId" />
   </GeneratePackageManagerJava>
@@ -2010,6 +2011,7 @@ because xbuild doesn't support framework reference assemblies.
       ApplicationSharedLibraries="@(_ApplicationSharedLibrary)"
       DebugBuild="$(AndroidIncludeDebugSymbols)"
       AndroidBinUtilsDirectory="$(AndroidBinUtilsDirectory)"
+      ZipAlignmentPages="$(_AndroidZipAlignment)"
   />
   <ItemGroup>
     <FileWrites Include="@(_ApplicationSharedLibrary)" />
@@ -2354,6 +2356,7 @@ because xbuild doesn't support framework reference assemblies.
 	<Delete Files="%(ApkAbiFilesSigned.FullPath)" Condition=" '$(AndroidUseApkSigner)' == 'true' "/>
 	<AndroidZipAlign Condition=" '$(AndroidUseApkSigner)' == 'true' "
 		Source="%(ApkAbiFilesIntermediate.Identity)"
+		Alignment="$(_AndroidZipAlignment)"
 		DestinationDirectory="$(OutDir)"
 		ToolPath="$(ZipAlignToolPath)"
 		ToolExe="$(ZipalignToolExe)"
@@ -2389,6 +2392,7 @@ because xbuild doesn't support framework reference assemblies.
 	<Message Text="Unaligned android package '%(ApkAbiFilesUnaligned.FullPath)'"  Condition=" '$(AndroidUseApkSigner)' != 'True' And '$(AndroidPackageFormat)' != 'aab' "/>
 	<AndroidZipAlign Condition=" '$(AndroidUseApkSigner)' != 'True' And '$(AndroidPackageFormat)' != 'aab' "
 		Source="%(ApkAbiFilesUnaligned.Identity)"
+		Alignment="$(_AndroidZipAlignment)"
 		DestinationDirectory="$(OutDir)"
 		ToolPath="$(ZipAlignToolPath)"
 		ToolExe="$(ZipalignToolExe)"
diff --git a/src/native/CMakeLists.txt b/src/native/CMakeLists.txt
@@ -374,6 +374,7 @@ set(POTENTIAL_XA_COMMON_COMPILER_ARGS
   -Wformat-security
   -Wformat=2
   -Wno-format-nonliteral
+  -Wno-vla-cxx-extension
   -Wimplicit-fallthrough
   -Wmisleading-indentation
   -Wnull-dereference
diff --git a/src/native/monodroid/embedded-assemblies-zip.cc b/src/native/monodroid/embedded-assemblies-zip.cc
@@ -60,9 +60,9 @@ EmbeddedAssemblies::zip_load_entry_common (size_t entry_index, std::vector<uint8
 	}
 
 	// assemblies must be 4-byte aligned, or Bad Things happen
-	if ((state.data_offset & 0x3) != 0) {
-		log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at bad offset %lu within the .apk\n", entry_name.get (), state.data_offset);
-		log_fatal (LOG_ASSEMBLY, "You MUST run `zipalign` on %s\n", strrchr (state.file_name, '/') + 1);
+	if ((state.data_offset & application_config.zip_alignment_mask) != 0) {
+		log_fatal (LOG_ASSEMBLY, "Assembly '%s' is located at bad offset %lu within the .apk", entry_name.get (), state.data_offset);
+		log_fatal (LOG_ASSEMBLY, "You MUST run `zipalign` on %s to align it on %u bytes ", strrchr (state.file_name, '/') + 1, application_config.zip_alignment_mask + 1);
 		Helpers::abort_application ();
 	}
 
diff --git a/src/native/xamarin-app-stub/application_dso_stub.cc b/src/native/xamarin-app-stub/application_dso_stub.cc
@@ -60,11 +60,13 @@ const ApplicationConfig application_config = {
 	.number_of_assemblies_in_apk = 2,
 	.bundled_assembly_name_width = 0,
 	.number_of_dso_cache_entries = 2,
+	.number_of_shared_libraries = 2,
 	.android_runtime_jnienv_class_token = 1,
 	.jnienv_initialize_method_token = 2,
 	.jnienv_registerjninatives_method_token = 3,
 	.jni_remapping_replacement_type_count = 2,
 	.jni_remapping_replacement_method_index_entry_count = 2,
+	.zip_alignment_mask = 3,
 	.mono_components_mask = MonoComponent::None,
 	.android_package_name = android_package_name,
 };
@@ -81,6 +83,7 @@ static char second_assembly_name[AssemblyNameWidth];
 XamarinAndroidBundledAssembly bundled_assemblies[] = {
 	{
 		.file_fd = -1,
+		.file_name = nullptr,
 		.data_offset = 0,
 		.data_size = 0,
 		.data = nullptr,
@@ -90,6 +93,7 @@ XamarinAndroidBundledAssembly bundled_assemblies[] = {
 
 	{
 		.file_fd = -1,
+		.file_name = nullptr,
 		.data_offset = 0,
 		.data_size = 0,
 		.data = nullptr,
@@ -117,6 +121,7 @@ AssemblyStoreSingleAssemblyRuntimeData assembly_store_bundled_assemblies[] = {
 AssemblyStoreRuntimeData assembly_store = {
 	.data_start = nullptr,
 	.assembly_count = 0,
+	.index_entry_count = 0,
 	.assemblies = nullptr,
 };
 
diff --git a/src/native/xamarin-app-stub/xamarin-app.hh b/src/native/xamarin-app-stub/xamarin-app.hh
@@ -253,6 +253,7 @@ struct ApplicationConfig
 	uint32_t jnienv_registerjninatives_method_token;
 	uint32_t jni_remapping_replacement_type_count;
 	uint32_t jni_remapping_replacement_method_index_entry_count;
+	uint32_t zip_alignment_mask; // 3, for 4-byte alignment (4k memory pages); 15, for 16-byte alignment (16k memory pages)
 	MonoComponent mono_components_mask;
 	const char *android_package_name;
 };

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,10 @@ namespace Xamarin.Android.Tasks`
`7`	`7`	`{`
`8`	`8`	`public class AndroidZipAlign : AndroidRunToolTask`
`9`	`9`	`{`
	`10`	`+ // Sometime next year the default value should be changed to 16 since it's going to be a Google Play store requirement for`
	`11`	`+ // application submissions`
	`12`	`+ internal const int DefaultZipAlignment = 4;`
	`13`	`+`
`10`	`14`	`public override string TaskPrefix => "AZA";`
`11`	`15`
`12`	`16`	`[Required]`
`@@ -15,7 +19,7 @@ public class AndroidZipAlign : AndroidRunToolTask`
`15`	`19`	`[Required]`
`16`	`20`	`public ITaskItem DestinationDirectory { get; set; }`
`17`	`21`
`18`		`- int alignment = 4;`
	`22`	`+ int alignment = DefaultZipAlignment;`
`19`	`23`	`public int Alignment {`
`20`	`24`	`get {return alignment;}`
`21`	`25`	`set {alignment = value;}`
`@@ -53,4 +57,3 @@ protected override void LogEventsFromTextOutput (string singleLine, MessageImpor`
`53`	`57`	`}`
`54`	`58`	`}`
`55`	`59`	`}`
`56`		`-`
Original file line number	Diff line number	Diff line change
`@@ -712,5 +712,22 @@ internal static void DumpMarshalMethodsToConsole (string heading, IDictionary<st`
`712`	`712`	`}`
`713`	`713`	`}`
`714`	`714`	`}`
	`715`	`+`
	`716`	`+ public static uint ZipAlignmentToMask (int alignment) => ZipAlignmentToMaskOrPageSize (alignment, needMask: true);`
	`717`	`+ public static uint ZipAlignmentToPageSize (int alignment) => ZipAlignmentToMaskOrPageSize (alignment, needMask: false);`
	`718`	`+`
	`719`	`+ static uint ZipAlignmentToMaskOrPageSize (int alignment, bool needMask)`
	`720`	`+ {`
	`721`	`+ const uint pageSize4k = 4096;`
	`722`	`+ const uint pageMask4k = 3;`
	`723`	`+ const uint pageSize16k = 16384;`
	`724`	`+ const uint pageMask16k = 15;`
	`725`	`+`
	`726`	`+ return alignment switch {`
	`727`	`+ 4 => needMask ? pageMask4k : pageSize4k,`
	`728`	`+ 16 => needMask ? pageMask16k : pageSize16k,`
	`729`	`+ _ => throw new InvalidOperationException ($"Internal error: unsupported zip page alignment value {alignment}")`
	`730`	`+ };`
	`731`	`+ }`
`715`	`732`	`}`
`716`	`733`	`}`