From 50c5ae57f28d50da39b586ae4dc050fb95185011 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 8 Jun 2023 16:51:10 +0200 Subject: [PATCH 1/5] [msbuild] Dispose a few streams when done with them in the unzip/decompress logic. (#18410) Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1829715. --- msbuild/Xamarin.MacDev.Tasks/Decompress.cs | 4 ++-- .../Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/msbuild/Xamarin.MacDev.Tasks/Decompress.cs b/msbuild/Xamarin.MacDev.Tasks/Decompress.cs index db0bfb7b4b47..1be06290795b 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Decompress.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Decompress.cs @@ -130,7 +130,7 @@ public static bool TryDecompress (TaskLoggingHelper log, string zip, string reso static bool TryDecompressUsingUnzip (TaskLoggingHelper log, string zip, string resource, string decompressionDir) { - var archive = ZipFile.OpenRead (zip); + using var archive = ZipFile.OpenRead (zip); resource = resource.Replace ('\\', zipDirectorySeparator); var entry = archive.GetEntry (resource); if (entry is null) { @@ -165,7 +165,7 @@ static bool TryDecompressUsingSystemIOCompression (TaskLoggingHelper log, string resource = resource.Replace ('\\', zipDirectorySeparator); var resourceAsDir = resource + zipDirectorySeparator; - var archive = ZipFile.OpenRead (zip); + using var archive = ZipFile.OpenRead (zip); foreach (var entry in archive.Entries) { var entryPath = entry.FullName; if (entryPath.Length == 0) diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs index 92cfff92bf8c..92787e318191 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs @@ -252,7 +252,7 @@ static bool TryGetSidecarManifest (TaskLoggingHelper log, string resources, [Not static bool TryGetInfoPlist (TaskLoggingHelper log, string resourcePath, string xcframework, [NotNullWhen (true)] out PDictionary? plist) { var manifestPath = Path.Combine (xcframework, "Info.plist"); - var stream = CompressionHelper.TryGetPotentiallyCompressedFile (log, resourcePath, manifestPath); + using var stream = CompressionHelper.TryGetPotentiallyCompressedFile (log, resourcePath, manifestPath); if (stream is null) { plist = null; return false; From 707a2db3e1b1dc5b0888401a8fd477be83679287 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 8 Jun 2023 16:51:29 +0200 Subject: [PATCH 2/5] [msbuild] Make sure to copy the manifest from a binding resource package back to Windows. Fixes #18402. (#18419) This fixes a build problem on windows when a project has a project reference to a binding project. The binding resource package from the binding project would lack the manifest, and thus the main project wouldn't detect it as a binding resource package, effectively not seeing any native resource from the binding project. Fixes https://github.com/xamarin/xamarin-macios/issues/18402. --- .../Tasks/CreateBindingResourcePackageBase.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/CreateBindingResourcePackageBase.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/CreateBindingResourcePackageBase.cs index fc2e29866966..8286a742ecd5 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/CreateBindingResourcePackageBase.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/CreateBindingResourcePackageBase.cs @@ -96,6 +96,7 @@ public override bool Execute () Log.LogWarning (MSBStrings.W7100, bindingOutputPath); } } + packagedFiles.Add (manifestPath); } PackagedFiles = packagedFiles.Select (v => new TaskItem (v)).ToArray (); From e3088879a619ba2af5d5d739d1d212243e810f24 Mon Sep 17 00:00:00 2001 From: Ivan Povazan <55002338+ivanpovazan@users.noreply.github.com> Date: Mon, 12 Jun 2023 13:45:38 +0200 Subject: [PATCH 3/5] Reduce the size of `__LINKEDIT Export Info` section in stripped binaries (#18408) # Description This PR reduces the application's SOD (size on disk) by making `__LINKEDIT Export Info` section smaller in the stripped Mach-O binaries. The feature is controlled by `_ExportSymbolsExplicitly` MSBuild property and can be disabled by specifying: `-p:_ExportSymbolsExplicitly=true` Fixes #18332 # Initial problem It has been noticed that during stripping, the strip tool does not resize the export info section after it removes the symbols. Instead it only zeroes out the entries (achieved by calling `prune_trie` function): - https://github.com/apple-oss-distributions/cctools/blob/cctools-986/misc/strip.c - https://github.com/apple-oss-distributions/ld64/blob/ld64-711/src/other/PruneTrie.cpp Thanks @lambdageek for helping to track this down. # Approach As Xamarin build process already collects all the [required symbols][1] needed for the application to run and preserves them during the strip phase, we can use the same file to instruct clang toolchain to export only those symbols via the command line options: `-exported_symbols_list ` ([source][2]). This will make the export info section only include what is necessary for the runtime - and at the same time eliminate the problem of the `strip` tool which does not resize stripped symbols. # Investigation setup The issue is observable by building and inspecting the test application: https://github.com/xamarin/xamarin-macios/blob/main/tests/dotnet/MySingleView/MySingleView.csproj and targeting iOS platform in Release mode. ## Results: | Measure | MySingleView - main | MySingleView - this PR | Diff (%) | | :--- | ---: | ---: | ---: | | SOD (bytes) | 13668940 | 13458476 | -1.5% | | .ipa (bytes) | 4829368 | 4827928 | -0.03% | Even though zeroes are compressed well, the SOD is still affected and unused section takes around 1.5% of the most simplistic app size. Much bigger impact has been noted when trying out a MAUI iOS template app with NativeAOT where the `__LINKEDIT Export Info` zeroes take up to 20MB of the SOD, but also with the regular macOS applications: https://github.com/dotnet/runtime/issues/86707 ### Repro current state of MySingleView.app with stripped binary 1. Build the app (you can ignore the need to run the sample, I just did it to make sure the changes do not break anything) ```bash make run-device ``` 2. Print the load commands - [load_cmds_strip.list][3] ```bash otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_strip.list ``` - We are interested in the export info section: ``` cmd LC_DYLD_INFO_ONLY ... export_off 5942960 export_size 207712 ``` 3. Create a hex dump of the export info section - [hex_dump_strip.list][4] ``` bash xxd -s 5942960 -l 207712 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_strip.list ``` - NOTE: Notice around ~200kb of zeroes from ~0x005ab490 to ~0x005dda00 4. Verify exported symbols are correct - [dyld_info_strip.list][5] ``` bash dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_strip.list ``` ### Repro current state of MySingleView.app with unstripped binary 1. Build the app (the make target preserves the symbols) ```bash make run-device-no-strip ``` 2. Print the load commands - [load_cmds_nostrip.list][6] ```bash otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_nostrip.list ``` - We are interested in the export info section: ``` cmd LC_DYLD_INFO_ONLY ... export_off 5942960 export_size 207712 ``` 3. Create a hex dump of the export info section - [hex_dump_nostrip.list][7] ``` bash xxd -s 5942960 -l 207712 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_nostrip.list ``` - Notice that the range: ~ 0x005ab490 to ~ 0x005dda00 now includes exported symbol entries 4. Verify exported symbols are correct - [dyld_info_nostrip.list][8] ``` bash dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_nostrip.list ``` ### Repro the new approach 1. Build the app (the make target uses the new approach) ```bash make run-device-export-syms ``` 2. Print the load commands - [load_cmds_export.list][9] ```bash otool -l bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > load_cmds_export.list ``` - We are interested in the export info section ***notice the reduced size of the section***: ``` cmd LC_DYLD_INFO_ONLY ... export_off 5942432 export_size 1048 ``` 3. Create a hex dump of the export info section - [hex_dump_export.list][10] ``` bash xxd -s 5942432 -l 1048 bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > hex_dump_export.list ``` 4. Verify exported symbols are correct - [dyld_info_export.list][11] ``` bash dyld_info -exports bin/Release/net7.0-ios/ios-arm64/MySingleView.app/MySingleView > dyld_info_export.list ``` --- ## Additional benefits With this approach we could also switch the way strip tool is invoked to always strip all debug and local symbols via `strip -S -x` instead of passing the file with symbols to preserve. This would remove the warning that we are currently getting (which is being ignored): ``` /Applications/Xcode_14.3.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/strip: warning: removing global symbols from a final linked no longer supported. Use -exported_symbols_list at link time when building... ``` ## Other references: - https://github.com/qyang-nj/llios/blob/main/exported_symbol/README.md [1]: https://github.com/xamarin/xamarin-macios/blob/11e7883da04d80c59e4ffbbc955a3e0e0060ff90/tools/dotnet-linker/Steps/GenerateReferencesStep.cs#L38-L44 [2]: https://developer.apple.com/library/archive/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryDesignGuidelines.html [3]: https://gist.github.com/ivanpovazan/d53f8d10be5e4ea9f39a41ea540aa7fa [4]: https://gist.github.com/ivanpovazan/60637422f3ff8cb5f437ddd06a21d9c1 [5]: https://gist.github.com/ivanpovazan/352595ad15c2ac02f38dcb3bd4130642 [6]: https://gist.github.com/ivanpovazan/bf700161f2f3691d1d7381c98d4fa0be [7]: https://gist.github.com/ivanpovazan/44269e4fff5ebd58a4d181451e5c106f [8]: https://gist.github.com/ivanpovazan/38c5afe076502d514a77420af0e10b01 [9]: https://gist.github.com/ivanpovazan/3f663c3c630005f5a578605d48ba807e [10]: https://gist.github.com/ivanpovazan/0bb84f64281d05ab20438aeaed64f13c [11]: https://gist.github.com/ivanpovazan/78b3ba2288f53a2316b9bc46964e7e4f --------- Co-authored-by: Rolf Bjarne Kvinge --- dotnet/targets/Xamarin.Shared.Sdk.props | 3 +++ dotnet/targets/Xamarin.Shared.Sdk.targets | 20 ++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/dotnet/targets/Xamarin.Shared.Sdk.props b/dotnet/targets/Xamarin.Shared.Sdk.props index bffc29378639..1cad3b80ab3d 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.props +++ b/dotnet/targets/Xamarin.Shared.Sdk.props @@ -55,6 +55,9 @@ <_RequiresILLinkPack Condition="'$(_RequiresILLinkPack)' == ''">true + + + <_ExportSymbolsExplicitly Condition="'$(_ExportSymbolsExplicitly)' == ''">true diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets index 4939b749a45e..8515abdfba33 100644 --- a/dotnet/targets/Xamarin.Shared.Sdk.targets +++ b/dotnet/targets/Xamarin.Shared.Sdk.targets @@ -1180,6 +1180,10 @@ + + <_ExportedSymbolsFile Condition="'$(_ExportedSymbolsFile)' == '' and '$(_MtouchSymbolsList)' == ''">/dev/null + <_ExportedSymbolsFile Condition="'$(_ExportedSymbolsFile)' == ''">$(_MtouchSymbolsList) + <_XamarinMainLibraries Include="$(_XamarinNativeLibraryDirectory)/$(_LibXamarinName)" /> @@ -1196,6 +1200,9 @@ <_MainLinkerFlags Include="-liconv" /> <_MainLinkerFlags Include="-lcompression" /> + <_MainLinkerFlags Condition="'$(_ExportSymbolsExplicitly)' == 'true'" Include="-exported_symbols_list" /> + <_MainLinkerFlags Condition="'$(_ExportSymbolsExplicitly)' == 'true'" Include="$(_ExportedSymbolsFile)" /> + <_LinkNativeExecutableInputs Include="@(_NativeExecutableObjectFiles)" /> <_LinkNativeExecutableInputs Include="@(_XamarinMainLibraries)" /> @@ -1282,13 +1289,21 @@ <_NativeReferences Include="@(_FileNativeReference)" Condition="'%(_FileNativeReference.LinkToExecutable)' != 'false'" /> + + + + + <_AllLinkerFlags Include="@(_AssemblyLinkerFlags);@(_MainLinkerFlags);@(_CustomLinkFlags)" /> + <_AllLinkerFlags Condition="'$(_ExportSymbolsExplicitly)' != 'true'" Include="@(_ReferencesLinkerFlags)" /> + + - - - From ba5becc790f347bbfb6ed79e4a7506a95e51f885 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 12 Jun 2023 18:44:11 +0200 Subject: [PATCH 4/5] [configure] Make it possible to run configure from any directory. (#18427) --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index ff6dc8ddae2e..e31e61d1de79 100755 --- a/configure +++ b/configure @@ -47,6 +47,7 @@ Usage: configure [options] EOL } +cd "$(dirname "${BASH_SOURCE[0]}")" CONFIGURED_FILE=configure.inc rm -f $CONFIGURED_FILE From 859a48a73a5e59fcc4b85a90ed13fb2b41b0d33c Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 12 Jun 2023 18:44:25 +0200 Subject: [PATCH 5/5] [tools] Automatically re-create ProductConstants.cs when the current commit changes. (#18426) This fixes a frequent issue where building locally again would create test apps where the registrar would fail due to mismatched runtime versions. --- tools/common/Make.common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/common/Make.common b/tools/common/Make.common index a8ec899bb0be..246eca00f0e7 100644 --- a/tools/common/Make.common +++ b/tools/common/Make.common @@ -60,7 +60,7 @@ $(Q) if ! diff $@ $@.tmp >/dev/null; then $(CP) $@.tmp $@; git diff "$@"; echo "The file $(TOP)/tools/common/SdkVersions.cs has been automatically re-generated; please commit the changes."; exit 1; fi $(Q) touch $@ -../common/ProductConstants.cs: ../common/ProductConstants.in.cs Makefile $(TOP)/Make.config +../common/ProductConstants.cs: ../common/ProductConstants.in.cs Makefile $(TOP)/Make.config $(TOP)/.git/index $(Q_GEN) sed \ -e "s/@IOS_VERSION@/$(IOS_PACKAGE_VERSION_MAJOR).$(IOS_PACKAGE_VERSION_MINOR).$(IOS_PACKAGE_VERSION_REV)/g" \ -e "s/@TVOS_VERSION@/$(IOS_PACKAGE_VERSION_MAJOR).$(IOS_PACKAGE_VERSION_MINOR).$(IOS_PACKAGE_VERSION_REV)/g" \