From 718cc5df393cd873eaf53b1a5f6470fdcbbd0ea1 Mon Sep 17 00:00:00 2001 From: Chris Hamons Date: Thu, 1 Mar 2018 09:36:06 -0600 Subject: [PATCH] [mmp] Add stripping of 32-bit dylibs to work with new App Store restrictions (#3387) - https://github.com/xamarin/xamarin-macios/issues/3367 - App Store will now fail builds if you add in a 32-bit dylib - If you are a 32-bit app you don't need the 64-bit part of your fat dylib anyway - Add --optimize=-trim-architectures to allow customization of behavior, as not everyone uses app store In addition, while writing tests for this is was noticed that mmp tests did not "really" run Release configuration correctly in most cases. Fixing this turned out to be a bit of a pain, but necessary to correctly test this (and other things). - Turns out that /p:configuration:debug is not sufficient to tell mmp to do the right thing - That, in most projects, sets the DebugSymbols property, which really is what is checked. - However, two of our projects did not have that, so we always did release mmp work. - Removed configuration property for tests and added real "Release" configuration option --- docs/website/mmp-errors.md | 13 ++ docs/website/mtouch-errors.md | 3 + tests/common/MachO.cs | 77 +++++++++++ tests/common/mac/ProjectTestHelpers.cs | 127 +++++++++++++++-- tests/common/mac/SystemMonoExample.csproj | 15 +- tests/common/mac/UnifiedExample.csproj | 20 ++- tests/common/mac/XM45Example.csproj | 16 ++- tests/mmptest/mmptest.csproj | 7 + tests/mmptest/src/CodeStrippingTests.cs | 158 ++++++++++++++++++++++ tests/mmptest/src/MMPTest.cs | 32 +++-- tests/mtouch/MTouch.cs | 72 +--------- tests/mtouch/mtouch.csproj | 3 + tools/common/Driver.cs | 9 +- tools/common/MachO.cs | 21 ++- tools/common/Optimizations.cs | 19 +++ tools/common/StringUtils.cs | 7 +- tools/mmp/driver.cs | 29 +++- tools/mtouch/Application.cs | 19 --- 18 files changed, 518 insertions(+), 129 deletions(-) create mode 100644 tests/common/MachO.cs create mode 100644 tests/mmptest/src/CodeStrippingTests.cs diff --git a/docs/website/mmp-errors.md b/docs/website/mmp-errors.md index db3002f0bf14..ac30d2cde932 100644 --- a/docs/website/mmp-errors.md +++ b/docs/website/mmp-errors.md @@ -235,6 +235,8 @@ Mixed-mode assemblies can not be processed by the linker. See https://msdn.microsoft.com/en-us/library/x0w2664k.aspx for more information on mixed-mode assemblies. + + ### MM2106: Could not optimize the call to BlockLiteral.SetupBlock[Unsafe] in * at offset * because *. The linker reports this warning when it can't optimize a call to BlockLiteral.SetupBlock or Block.SetupBlockUnsafe. @@ -258,6 +260,15 @@ To remove the warning either remove the optimization argument to mmp, or pass By default this option will be automatically enabled whenever it's possible and safe to do so. +### MM2108: '{0}' was stripped of architectures except '{1}' to comply with App Store restrictions. This could break exisiting codesigning signatures. Consider stripping the library with lipo or disabling with --optimize=-trim-architectures"); + +The App Store now rejects applications which contain libraries and frameworks which contain 32-bit variants. The library was stripped of unused archtectures when copied into the final application bundle. + +This is in general safe, and will reduce application bundle size as an added benefit. However, any bundled framework that is code signed will have its signature invalidated (and resigned later if the application is signed). + +Consider using `lipo` to remove the unnecessary archtectures permanently from the source library. If the application is not being published to the App Store, this removal can be disabled by passing --optimize=-trim-architectures as Additional MMP Arguments. + + # MM3xxx: AOT ## MM30xx: AOT (general) errors @@ -363,6 +374,8 @@ See the [equivalent mtouch warning](mtouch-errors.md#MT5218). ### MM5310: install_name_tool failed with an error code '{0}'. Check build log for details. +### MM5311: lipo failed with an error code '{0}'. Check build log for details. + diff --git a/docs/website/mtouch-errors.md b/docs/website/mtouch-errors.md index 9bf87c5a0be8..497031618646 100644 --- a/docs/website/mtouch-errors.md +++ b/docs/website/mtouch-errors.md @@ -1325,6 +1325,8 @@ To remove the warning either remove the optimization argument to mtouch, or pass By default this option will be automatically enabled whenever it's possible and safe to do so. + + # MT3xxx: AOT error messages + # MT6xxx: mtouch internal tools error messages diff --git a/tests/common/MachO.cs b/tests/common/MachO.cs new file mode 100644 index 000000000000..bd9bb639c465 --- /dev/null +++ b/tests/common/MachO.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using System.IO; + +namespace Xamarin.Tests +{ + public static class MachO + { + public static List GetArchitectures (string file) + { + var result = new List (); + + using (var fs = File.OpenRead (file)) { + using (var reader = new BinaryReader (fs)) { + int magic = reader.ReadInt32 (); + switch ((uint) magic) { + case 0xCAFEBABE: // little-endian fat binary + throw new NotImplementedException ("little endian fat binary"); + case 0xBEBAFECA: + int architectures = System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()); + for (int i = 0; i < architectures; i++) { + result.Add (GetArch (System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()), System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()))); + // skip to next entry + reader.ReadInt32 (); // offset + reader.ReadInt32 (); // size + reader.ReadInt32 (); // align + } + break; + case 0xFEEDFACE: // little-endian mach-o header + case 0xFEEDFACF: // little-endian 64-big mach-o header + result.Add (GetArch (reader.ReadInt32 (), reader.ReadInt32 ())); + break; + case 0xCFFAEDFE: + case 0xCEFAEDFE: + result.Add (GetArch (System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()), System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()))); + break; + default: + throw new Exception (string.Format ("File '{0}' is neither a Universal binary nor a Mach-O binary (magic: 0x{1})", file, magic.ToString ("x"))); + } + } + } + + return result; + } + + static string GetArch (int cputype, int cpusubtype) + { + const int ABI64 = 0x01000000; + const int X86 = 7; + const int ARM = 12; + + switch (cputype) { + case ARM : // arm + switch (cpusubtype) { + case 6: return "armv6"; + case 9: return "armv7"; + case 11: return "armv7s"; + default: + return "unknown arm variation: " + cpusubtype.ToString (); + } + case ARM | ABI64: + switch (cpusubtype) { + case 0: + return "arm64"; + default: + return "unknown arm/64 variation: " + cpusubtype.ToString (); + } + case X86: // x86 + return "i386"; + case X86 | ABI64: // x64 + return "x86_64"; + } + + return string.Format ("unknown: {0}/{1}", cputype, cpusubtype); + } + } +} diff --git a/tests/common/mac/ProjectTestHelpers.cs b/tests/common/mac/ProjectTestHelpers.cs index e5911581c64f..6d6b9c976d48 100644 --- a/tests/common/mac/ProjectTestHelpers.cs +++ b/tests/common/mac/ProjectTestHelpers.cs @@ -43,6 +43,94 @@ internal class MessageTool : Tool } } + static class FrameworkBuilder + { + const string PListText = @" + + + + BuildMachineOSBuild + 16B2657 + CFBundleDevelopmentRegion + English + CFBundleExecutable + Foo + CFBundleIdentifier + com.test.Foo + CFBundleInfoDictionaryVersion + 6.0 + CFBundleName + Foo + CFBundlePackageType + FMWK + CFBundleShortVersionString + 6.9 + CFBundleSignature + ???? + CFBundleSupportedPlatforms + + MacOSX + + CFBundleVersion + 1561.40.100 + DTCompiler + com.apple.compilers.llvm.clang.1_0 + DTPlatformBuild + 9Q85j + DTPlatformVersion + GM + DTSDKBuild + 17E138 + DTSDKName + macosx10.13internal + DTXcode + 0930 + DTXcodeBuild + 9Q85j + +"; + + public static string CreateFatFramework (string tmpDir) + { + Func f = x => Path.Combine (tmpDir, x); + File.WriteAllText (f ("foo.c"), "int Answer () { return 42; }"); + File.WriteAllText (f ("Info.plist"), PListText); + + TI.RunAndAssert ($"clang -m32 -c -o {f ("foo_32.o")} {f ("foo.c")}"); + TI.RunAndAssert ($"clang -m64 -c -o {f ("foo_64.o")} {f ("foo.c")}"); + TI.RunAndAssert ($"clang -m32 -dynamiclib -o {f ("foo_32.dylib")} {f ("foo_32.o")}"); + TI.RunAndAssert ($"clang -m64 -dynamiclib -o {f ("foo_64.dylib")} {f ("foo_64.o")}"); + TI.RunAndAssert ($"lipo -create {f ("foo_32.dylib")} {f ("foo_64.dylib")} -output {f ("Foo")}"); + TI.RunAndAssert ($"install_name_tool -id @rpath/Foo.framework/Foo {f ("Foo")}"); + TI.RunAndAssert ($"mkdir -p {f ("Foo.framework/Versions/A/Resources")}"); + TI.RunAndAssert ($"cp {f ("Foo")} {f ("Foo.framework/Versions/A/Foo")}"); + TI.RunAndAssert ($"cp {f ("Info.plist")} {f ("Foo.framework/Versions/A/Resources/")}"); + TI.RunAndAssert ($"ln -s Versions/A/Foo {f ("Foo.framework/Foo")}"); + TI.RunAndAssert ($"ln -s Versions/A/Resources {f ("Foo.framework/Resources")}"); + TI.RunAndAssert ($"ln -s Versions/A {f ("Foo.framework/Current")}"); + return f ("Foo.framework"); + } + + public static string CreateThinFramework (string tmpDir, bool sixtyFourBits = true) + { + Func f = x => Path.Combine (tmpDir, x); + File.WriteAllText (f ("foo.c"), "int Answer () { return 42; }"); + File.WriteAllText (f ("Info.plist"), PListText); + + string bitnessArg = sixtyFourBits ? "-m64" : "-m32"; + TI.RunAndAssert ($"clang {bitnessArg} -c -o {f ("foo.o")} {f ("foo.c")}"); + TI.RunAndAssert ($"clang {bitnessArg} -dynamiclib -o {f ("Foo")} {f ("foo.o")}"); + TI.RunAndAssert ($"install_name_tool -id @rpath/Foo.framework/Foo {f ("Foo")}"); + TI.RunAndAssert ($"mkdir -p {f ("Foo.framework/Versions/A/Resources")}"); + TI.RunAndAssert ($"cp {f ("Foo")} {f ("Foo.framework/Versions/A/Foo")}"); + TI.RunAndAssert ($"cp {f ("Info.plist")} {f ("Foo.framework/Versions/A/Resources/")}"); + TI.RunAndAssert ($"ln -s Versions/A/Foo {f ("Foo.framework/Foo")}"); + TI.RunAndAssert ($"ln -s Versions/A/Resources {f ("Foo.framework/Resources")}"); + TI.RunAndAssert ($"ln -s Versions/A {f ("Foo.framework/Current")}"); + return f ("Foo.framework"); + } + } + // Hide the hacks and provide a nice interface for writting tests that build / run XM projects static class TI { @@ -54,6 +142,8 @@ public class UnifiedTestConfig public bool FSharp { get; set; } public bool XM45 { get; set; } public bool DiagnosticMSBuild { get; set; } + public bool Release { get; set; } = false; + public string ProjectName { get; set; } = ""; public string TestCode { get; set; } = ""; public string TestDecl { get; set; } = ""; @@ -98,6 +188,15 @@ public static Version FindMonoVersion () return new Version (versionRegex.Match (output).Value.Split (' ')[2]); } + public static string RunAndAssert (string exe) + { + var parts = exe.Split (new char [] { ' ' }, 2); + if (parts.Length == 1) + return RunAndAssert (exe, "", "Command: " + exe); + else + return RunAndAssert (parts[0], parts[1], "Command: " + exe); + } + public static string RunAndAssert (string exe, string args, string stepName, bool shouldFail = false, Func getAdditionalFailInfo = null, string[] environment = null) { StringBuilder output = new StringBuilder (); @@ -128,7 +227,7 @@ public static void CleanUnifiedProject (string csprojTarget, bool useMSBuild = f RunAndAssert ("/Library/Frameworks/Mono.framework/Commands/" + (useMSBuild ? "msbuild" : "xbuild"), new StringBuilder (csprojTarget + " /t:clean"), "Clean"); } - public static string BuildProject (string csprojTarget, bool isUnified, bool diagnosticMSBuild = false, bool shouldFail = false, bool useMSBuild = false, string configuration = null, string[] environment = null) + public static string BuildProject (string csprojTarget, bool isUnified, bool diagnosticMSBuild = false, bool shouldFail = false, bool useMSBuild = false, bool release = false, string[] environment = null) { string rootDirectory = FindRootDirectory (); @@ -146,10 +245,14 @@ public static string BuildProject (string csprojTarget, bool isUnified, bool dia buildArgs.Append (diagnosticMSBuild ? " /verbosity:diagnostic " : " /verbosity:normal "); buildArgs.Append (" /property:XamarinMacFrameworkRoot=" + rootDirectory + "/Library/Frameworks/Xamarin.Mac.framework/Versions/Current "); - if (!string.IsNullOrEmpty (configuration)) - buildArgs.Append ($" /property:Configuration={configuration} "); - } else + if (release) + buildArgs.Append ("/property:Configuration=Release "); + else + buildArgs.Append ("/property:Configuration=Debug "); + + } else { buildArgs.Append (" build "); + } buildArgs.Append (StringUtils.Quote (csprojTarget)); @@ -269,20 +372,20 @@ public static string GenerateUnifiedExecutableProject (UnifiedTestConfig config) return GenerateEXEProject (config); } - public static string GenerateAndBuildUnifiedExecutable (UnifiedTestConfig config, bool shouldFail = false, bool useMSBuild = false, string configuration = null, string[] environment = null) + public static string GenerateAndBuildUnifiedExecutable (UnifiedTestConfig config, bool shouldFail = false, bool useMSBuild = false, string[] environment = null) { string csprojTarget = GenerateUnifiedExecutableProject (config); - return BuildProject (csprojTarget, isUnified: true, diagnosticMSBuild: config.DiagnosticMSBuild, shouldFail: shouldFail, useMSBuild: useMSBuild, configuration: configuration, environment: environment); + return BuildProject (csprojTarget, isUnified: true, diagnosticMSBuild: config.DiagnosticMSBuild, shouldFail: shouldFail, useMSBuild: useMSBuild, release: config.Release, environment: environment); } public static string RunGeneratedUnifiedExecutable (UnifiedTestConfig config) { string bundleName = config.AssemblyName != "" ? config.AssemblyName : config.ProjectName.Split ('.')[0]; - string exePath = Path.Combine (config.TmpDir, "bin/Debug/" + bundleName + ".app/Contents/MacOS/" + bundleName); + string exePath = Path.Combine (config.TmpDir, "bin/" + (config.Release ? "Release/" : "Debug/") + bundleName + ".app/Contents/MacOS/" + bundleName); return RunEXEAndVerifyGUID (config.TmpDir, config.guid, exePath); } - public static OutputText TestUnifiedExecutable (UnifiedTestConfig config, bool shouldFail = false, bool useMSBuild = false, string configuration = null, string[] environment = null) + public static OutputText TestUnifiedExecutable (UnifiedTestConfig config, bool shouldFail = false, bool useMSBuild = false, string[] environment = null) { // If we've already generated guid bits for this config, don't tack on a second copy if (config.guid == Guid.Empty) @@ -291,7 +394,7 @@ public static OutputText TestUnifiedExecutable (UnifiedTestConfig config, bool s config.TestCode += GenerateOutputCommand (config.TmpDir, config.guid); } - string buildOutput = GenerateAndBuildUnifiedExecutable (config, shouldFail, useMSBuild, configuration, environment); + string buildOutput = GenerateAndBuildUnifiedExecutable (config, shouldFail, useMSBuild, environment); if (shouldFail) return new OutputText (buildOutput, ""); @@ -313,7 +416,7 @@ public static OutputText TestClassicExecutable (string tmpDir, string testCode = return new OutputText (buildOutput, runOutput); } - public static OutputText TestSystemMonoExecutable (UnifiedTestConfig config, bool shouldFail = false, string configuration = null) + public static OutputText TestSystemMonoExecutable (UnifiedTestConfig config, bool shouldFail = false) { config.guid = Guid.NewGuid (); var projectName = "SystemMonoExample"; @@ -321,11 +424,11 @@ public static OutputText TestSystemMonoExecutable (UnifiedTestConfig config, boo config.ProjectName = $"{projectName}.csproj"; string csprojTarget = GenerateSystemMonoEXEProject (config); - string buildOutput = BuildProject (csprojTarget, isUnified: true, diagnosticMSBuild: config.DiagnosticMSBuild, shouldFail: shouldFail, configuration: configuration); + string buildOutput = BuildProject (csprojTarget, isUnified: true, diagnosticMSBuild: config.DiagnosticMSBuild, shouldFail: shouldFail, release: config.Release); if (shouldFail) return new OutputText (buildOutput, ""); - string exePath = Path.Combine (config.TmpDir, "bin", configuration ?? "Debug", projectName + ".app", "Contents", "MacOS", projectName); + string exePath = Path.Combine (config.TmpDir, "bin", config.Release ? "Release" : "Debug", projectName + ".app", "Contents", "MacOS", projectName); string runOutput = RunEXEAndVerifyGUID (config.TmpDir, config.guid, exePath); return new OutputText (buildOutput, runOutput); } diff --git a/tests/common/mac/SystemMonoExample.csproj b/tests/common/mac/SystemMonoExample.csproj index aa261b648fca..b1b580f29d0e 100644 --- a/tests/common/mac/SystemMonoExample.csproj +++ b/tests/common/mac/SystemMonoExample.csproj @@ -19,13 +19,22 @@ false false false - false - false 4 false true + false %CODE% - falsex86_64Mac DeveloperDeveloper ID Installer + + + false + false + false + false + 4 + false + true +%CODE% + diff --git a/tests/common/mac/UnifiedExample.csproj b/tests/common/mac/UnifiedExample.csproj index 2f0a361a2f91..062781c09497 100644 --- a/tests/common/mac/UnifiedExample.csproj +++ b/tests/common/mac/UnifiedExample.csproj @@ -13,15 +13,31 @@ Xamarin.Mac + true bin\Debug false false false false - false - false + true + true 4 false +%CODE% + + + pdbonly + true + bin\Release + prompt + 4 + false + false + false + true + true + true + SdkOnly %CODE% diff --git a/tests/common/mac/XM45Example.csproj b/tests/common/mac/XM45Example.csproj index d2adacb599b1..64c86009e28a 100644 --- a/tests/common/mac/XM45Example.csproj +++ b/tests/common/mac/XM45Example.csproj @@ -15,18 +15,28 @@ true + true bin\Debug false false false - false - false - false 4 false %CODE% false + + pdbonly + true + bin\Release + prompt + 4 + false + false + false +%CODE% + + diff --git a/tests/mmptest/mmptest.csproj b/tests/mmptest/mmptest.csproj index 331b20d67e5b..1851fde91c00 100644 --- a/tests/mmptest/mmptest.csproj +++ b/tests/mmptest/mmptest.csproj @@ -96,6 +96,13 @@ BundlerTool.cs + + + MachO.cs + + + StringUtils.cs + diff --git a/tests/mmptest/src/CodeStrippingTests.cs b/tests/mmptest/src/CodeStrippingTests.cs new file mode 100644 index 000000000000..1ab8b64cf7e5 --- /dev/null +++ b/tests/mmptest/src/CodeStrippingTests.cs @@ -0,0 +1,158 @@ +using System; +using System.IO; +using System.Linq; +using NUnit.Framework; + +using Xamarin.Utils; + +namespace Xamarin.MMP.Tests +{ + public class CodeStrippingTests + { + static Func LipoStripConditional = s => s.Contains ("lipo") && s.Contains ("-thin"); + static Func LipoStripSkipPosixConditional = s => LipoStripConditional (s) && !s.Contains ("libMonoPosixHelper.dylib"); + + static Func DidAnyLipoStrip = output => output.SplitLines ().Any (LipoStripConditional); + static Func DidAnyLipoStripSkipPosix = output => output.SplitLines ().Any (LipoStripSkipPosixConditional); + + static TI.UnifiedTestConfig CreateStripTestConfig (bool? strip, string tmpDir, string additionalMMPArgs = "") + { + TI.UnifiedTestConfig test = new TI.UnifiedTestConfig (tmpDir) { }; + + if (strip.HasValue) + test.CSProjConfig = $"--optimize={(strip.Value ? "+" : "-")}trim-architectures {additionalMMPArgs}"; + else if (!string.IsNullOrEmpty (additionalMMPArgs)) + test.CSProjConfig = $"{additionalMMPArgs}"; + + return test; + } + + void AssertStrip (string libPath, bool shouldStrip) + { + var archsFound = Xamarin.Tests.MachO.GetArchitectures (libPath); + if (shouldStrip) { + Assert.AreEqual (1, archsFound.Count, "Did not contain one archs"); + Assert.True (archsFound.Contains ("x86_64"), "Did not contain x86_64"); + } else { + Assert.AreEqual (2, archsFound.Count, "Did not contain two archs"); + Assert.True (archsFound.Contains ("i386"), "Did not contain i386"); + Assert.True (archsFound.Contains ("x86_64"), "Did not contain x86_64"); + } + } + + void StripTestCore (TI.UnifiedTestConfig test, bool debugStrips, bool releaseStrips, string libPath, bool shouldWarn) + { + string buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + Assert.AreEqual (debugStrips, DidAnyLipoStrip (buildOutput), "Debug lipo usage did not match expectations"); + AssertStrip (Path.Combine (test.TmpDir, "bin/Debug/UnifiedExample.app/", libPath), shouldStrip: debugStrips); + Assert.AreEqual (shouldWarn && debugStrips, buildOutput.Contains ("MM2108"), "Debug warning did not match expectations"); + + test.Release = true; + buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + Assert.AreEqual (releaseStrips, DidAnyLipoStrip (buildOutput), "Release lipo usage did not match expectations"); + AssertStrip (Path.Combine (test.TmpDir, "bin/Release/UnifiedExample.app/", libPath), shouldStrip: releaseStrips); + Assert.AreEqual (shouldWarn && releaseStrips, buildOutput.Contains ("MM2108"), "Release warning did not match expectations"); + } + + [TestCase (null, false, true)] + [TestCase (true, true, true)] + [TestCase (false, false, false)] + public void ShouldStripMonoPosixHelper (bool? strip, bool debugStrips, bool releaseStrips) + { + MMPTests.RunMMPTest (tmpDir => + { + TI.UnifiedTestConfig test = CreateStripTestConfig (strip, tmpDir); + + StripTestCore (test, debugStrips, releaseStrips, "Contents/MonoBundle/libMonoPosixHelper.dylib", shouldWarn: false); + }); + } + + [TestCase (null, false, true)] + [TestCase (true, true, true)] + [TestCase (false, false, false)] + public void ShouldStripUserFramework (bool? strip, bool debugStrips, bool releaseStrips) + { + MMPTests.RunMMPTest (tmpDir => + { + var frameworkPath = FrameworkBuilder.CreateFatFramework (tmpDir); + TI.UnifiedTestConfig test = CreateStripTestConfig (strip, tmpDir, $"--native-reference={frameworkPath}"); + + StripTestCore (test, debugStrips, releaseStrips, "Contents/Frameworks/Foo.framework/Foo", shouldWarn: true); + }); + } + + const string MonoPosixOffset = "Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/libMonoPosixHelper.dylib"; + + [TestCase (true, true)] + [TestCase (false, false)] + public void ExplictStripOption_ThirdPartyLibrary_AndWarnsIfSo (bool? strip, bool shouldStrip) + { + MMPTests.RunMMPTest (tmpDir => + { + string originalLocation = Path.Combine (TI.FindRootDirectory (), MonoPosixOffset); + string newLibraryLocation = Path.Combine (tmpDir, "libTest.dylib"); + File.Copy (originalLocation, newLibraryLocation); + + TI.UnifiedTestConfig test = CreateStripTestConfig (strip, tmpDir, $" --native-reference=\"{newLibraryLocation}\""); + test.Release = true; + + string buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + Assert.AreEqual (shouldStrip, DidAnyLipoStrip (buildOutput), "lipo usage did not match expectations"); + Assert.AreEqual (shouldStrip, buildOutput.Contains ("MM2108"), "Warning did not match expectations"); + }); + } + + void AssertNoLipoOrWarning (string buildOutput, string context) + { + Assert.False (DidAnyLipoStrip (buildOutput), "lipo incorrectly run in context: " + context + "\n" + buildOutput); + Assert.False (buildOutput.Contains ("MM2108"), "MM2108 incorrectly given in in context: " + context + "\n" + buildOutput); + } + + void AssertLipoOnlyMonoPosix (string buildOutput, string context) + { + Assert.False (DidAnyLipoStripSkipPosix (buildOutput), "lipo incorrectly run in context outside of libMonoPosixHelper: " + context + "\n" + buildOutput); + Assert.False (buildOutput.Contains ("MM2108"), "MM2108 incorrectly given in in context: " + context + "\n" + buildOutput); + } + + [TestCase (false)] + [TestCase (true)] + public void ThirdPartyLibrary_WithIncorrectBitness_ShouldWarnOnRelease (bool sixtyFourBits) + { + MMPTests.RunMMPTest (tmpDir => + { + var frameworkPath = FrameworkBuilder.CreateFatFramework (tmpDir); + + TI.UnifiedTestConfig test = CreateStripTestConfig (null, tmpDir, $" --native-reference=\"{frameworkPath}\""); + + // Should always skip lipo/warning in Debug + string buildOutput = TI.TestUnifiedExecutable(test).BuildOutput; + AssertNoLipoOrWarning (buildOutput, "Debug"); + + // Should always lipo/warn in Release + test.Release = true; + buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + Assert.True (DidAnyLipoStrip (buildOutput), $"lipo did not run in release\n{buildOutput}"); + Assert.True (buildOutput.Contains ("MM2108"), $"MM2108 not given in release\n{buildOutput}"); + + }); + } + + [TestCase] + public void ThirdPartyLibrary_WithCorrectBitness_ShouldNotStripOrWarn () + { + MMPTests.RunMMPTest (tmpDir => + { + var frameworkPath = FrameworkBuilder.CreateThinFramework (tmpDir); + + TI.UnifiedTestConfig test = CreateStripTestConfig (null, tmpDir, $" --native-reference=\"{frameworkPath}\""); + + string buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + AssertNoLipoOrWarning (buildOutput, "Debug"); + + test.Release = true; + buildOutput = TI.TestUnifiedExecutable (test).BuildOutput; + AssertLipoOnlyMonoPosix (buildOutput, "Release"); // libMonoPosixHelper.dylib will lipo in Release + }); + } + } +} diff --git a/tests/mmptest/src/MMPTest.cs b/tests/mmptest/src/MMPTest.cs index 2fc36dc10905..70b2666c9a47 100644 --- a/tests/mmptest/src/MMPTest.cs +++ b/tests/mmptest/src/MMPTest.cs @@ -219,11 +219,13 @@ public void FilePathCollisionShouldNotFailBuild () }); } - [Test] - public void Unified_HelloWorld_ShouldHaveNoWarnings () + [TestCase (false)] + [TestCase (true)] + public void Unified_HelloWorld_ShouldHaveNoWarnings (bool release) { RunMMPTest (tmpDir => { TI.UnifiedTestConfig test = new TI.UnifiedTestConfig (tmpDir); + test.Release = release; // Due to https://bugzilla.xamarin.com/show_bug.cgi?id=48311 we can get warnings related to the registrar Func hasLegitWarning = results => @@ -639,20 +641,22 @@ public void UnifiedDebugBuilds_ShouldLinkToPartialStatic_UnlessDisabled () { TI.UnifiedTestConfig test = new TI.UnifiedTestConfig (tmpDir) { XM45 = xm45 }; + test.Release = true; string buildResults = TI.TestUnifiedExecutable (test).BuildOutput; - Assert.IsFalse (buildResults.Contains ("Xamarin.Mac.registrar"), "Release build should not use partial static registrar"); + Assert.IsFalse (buildResults.Contains ("Xamarin.Mac.registrar"), "Release build should not use partial static registrar\n" + buildResults); + test.Release = false; test.CSProjConfig = "true"; buildResults = TI.TestUnifiedExecutable (test).BuildOutput; - Assert.IsTrue (buildResults.Contains ("Xamarin.Mac.registrar"), "Debug build should use partial static registrar" ); + Assert.IsTrue (buildResults.Contains ("Xamarin.Mac.registrar"), "Debug build should use partial static registrar\n" + buildResults ); test.CSProjConfig = "true--registrar=dynamicx86_64"; buildResults = TI.TestUnifiedExecutable (test).BuildOutput; - Assert.IsFalse (buildResults.Contains ("Xamarin.Mac.registrar"), "registrar=dynamic build should not use partial static registrar"); + Assert.IsFalse (buildResults.Contains ("Xamarin.Mac.registrar"), "registrar=dynamic build should not use partial static registrar\n" + buildResults); test.CSProjConfig = "true--registrar=partialx86_64"; buildResults = TI.TestUnifiedExecutable (test).BuildOutput; - Assert.IsTrue (buildResults.Contains ("Xamarin.Mac.registrar"), "registrar=partial build should use partial static registrar"); + Assert.IsTrue (buildResults.Contains ("Xamarin.Mac.registrar"), "registrar=partial build should use partial static registrar\n" + buildResults); } }); } @@ -709,14 +713,15 @@ public void HttpClientHandler (string mmpHandler, string expectedHandler) } [Test] - [TestCase ("Debug")] - [TestCase ("Release")] - public void SystemMonoNotEmbedded (string configuration) + [TestCase (false)] + [TestCase (true)] + public void SystemMonoNotEmbedded (bool release) { RunMMPTest (tmpDir => { TI.UnifiedTestConfig test = new TI.UnifiedTestConfig (tmpDir); + test.Release = release; test.CSProjConfig = "--embed-mono=no"; - TI.TestSystemMonoExecutable (test, configuration: configuration); + TI.TestSystemMonoExecutable (test); }); } @@ -788,7 +793,7 @@ public void OldXcodeTest () TI.UnifiedTestConfig test = new TI.UnifiedTestConfig (tmpDir) { CSProjConfig = "True", // This makes the msbuild tasks pass /debug to mmp }; - TI.TestUnifiedExecutable (test, shouldFail: false, configuration: "Debug", environment: new string [] { "MD_APPLE_SDK_ROOT", Path.GetDirectoryName (Path.GetDirectoryName (oldXcode)) }); + TI.TestUnifiedExecutable (test, shouldFail: false, environment: new string [] { "MD_APPLE_SDK_ROOT", Path.GetDirectoryName (Path.GetDirectoryName (oldXcode)) }); }); } @@ -842,6 +847,9 @@ public void Optimizations (string opt) CSProjConfig = $"--optimize={opt}" + "Full", }; + // register-protocols requires static registrar which is default in Release + test.Release = true; + var rv = TI.TestUnifiedExecutable (test, shouldFail: false); rv.Messages.AssertNoWarnings (); rv.Messages.AssertErrorCount (0); @@ -859,7 +867,7 @@ public void MM0132 (string opt) "Full", }; var rv = TI.TestUnifiedExecutable (test, shouldFail: false); - rv.Messages.AssertWarning (132, $"Unknown optimization: '{opt}'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock, register-protocols, inline-dynamic-registration-supported."); + rv.Messages.AssertWarning (132, $"Unknown optimization: '{opt}'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, blockliteral-setupblock, register-protocols, inline-dynamic-registration-supported, trim-architectures."); rv.Messages.AssertErrorCount (0); }); } diff --git a/tests/mtouch/MTouch.cs b/tests/mtouch/MTouch.cs index 0755a20f9e89..be452533d94e 100644 --- a/tests/mtouch/MTouch.cs +++ b/tests/mtouch/MTouch.cs @@ -2301,7 +2301,7 @@ public void MonoFrameworkArchitectures () var mono_framework = Path.Combine (app.AppPath, "Frameworks", "Mono.framework", "Mono"); Assert.That (mono_framework, Does.Exist, "mono framework existence"); // Verify that mtouch removed armv7s from the framework. - Assert.That (GetArchitectures (mono_framework), Is.EquivalentTo (new [] { "armv7", "arm64" }), "mono framework architectures"); + Assert.That (MachO.GetArchitectures (mono_framework), Is.EquivalentTo (new [] { "armv7", "arm64" }), "mono framework architectures"); } } } @@ -4032,7 +4032,7 @@ static void VerifyGC (string file, string message) static void VerifyArchitectures (string file, string message, params string[] expected) { - var actual = GetArchitectures (file).ToArray (); + var actual = MachO.GetArchitectures (file).ToArray (); Array.Sort (expected); Array.Sort (actual); @@ -4061,74 +4061,6 @@ static void VerifyOutput (string msg, string actual, params string[] expected) Assert.Fail (text.ToString ()); } - static List GetArchitectures (string file) - { - var result = new List (); - - using (var fs = File.OpenRead (file)) { - using (var reader = new BinaryReader (fs)) { - int magic = reader.ReadInt32 (); - switch ((uint) magic) { - case 0xCAFEBABE: // little-endian fat binary - throw new NotImplementedException ("little endian fat binary"); - case 0xBEBAFECA: - int architectures = System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()); - for (int i = 0; i < architectures; i++) { - result.Add (GetArch (System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()), System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()))); - // skip to next entry - reader.ReadInt32 (); // offset - reader.ReadInt32 (); // size - reader.ReadInt32 (); // align - } - break; - case 0xFEEDFACE: // little-endian mach-o header - case 0xFEEDFACF: // little-endian 64-big mach-o header - result.Add (GetArch (reader.ReadInt32 (), reader.ReadInt32 ())); - break; - case 0xCFFAEDFE: - case 0xCEFAEDFE: - result.Add (GetArch (System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()), System.Net.IPAddress.NetworkToHostOrder (reader.ReadInt32 ()))); - break; - default: - throw new Exception (string.Format ("File '{0}' is neither a Universal binary nor a Mach-O binary (magic: 0x{1})", file, magic.ToString ("x"))); - } - } - } - - return result; - } - - static string GetArch (int cputype, int cpusubtype) - { - const int ABI64 = 0x01000000; - const int X86 = 7; - const int ARM = 12; - - switch (cputype) { - case ARM : // arm - switch (cpusubtype) { - case 6: return "armv6"; - case 9: return "armv7"; - case 11: return "armv7s"; - default: - return "unknown arm variation: " + cpusubtype.ToString (); - } - case ARM | ABI64: - switch (cpusubtype) { - case 0: - return "arm64"; - default: - return "unknown arm/64 variation: " + cpusubtype.ToString (); - } - case X86: // x86 - return "i386"; - case X86 | ABI64: // x64 - return "x86_64"; - } - - return string.Format ("unknown: {0}/{1}", cputype, cpusubtype); - } - public static void AssertDeviceAvailable () { if (!Configuration.include_device) diff --git a/tests/mtouch/mtouch.csproj b/tests/mtouch/mtouch.csproj index 200c133266a9..e85057881d9f 100644 --- a/tests/mtouch/mtouch.csproj +++ b/tests/mtouch/mtouch.csproj @@ -69,6 +69,9 @@ BundlerTool.cs + + MachO.cs + diff --git a/tools/common/Driver.cs b/tools/common/Driver.cs index 866bd4b6bad0..36c244e7fc58 100644 --- a/tools/common/Driver.cs +++ b/tools/common/Driver.cs @@ -134,7 +134,7 @@ static void AddSharedOptions (Application app, Mono.Options.OptionSet options) #if MONOTOUCH " inline-isdirectbinding: By default enabled (requires the linker). Tries to inline calls to NSObject.IsDirectBinding to load a constant value. Makes the app smaller, and slightly faster at runtime.\n" + #else - " inline-isdirectbinding: By default disabled, because it may (requires the linker), because . Tries to inline calls to NSObject.IsDirectBinding to load a constant value. Makes the app smaller, and slightly faster at runtime.\n" + + " inline-isdirectbinding: By default disabled, because it may require the linker. Tries to inline calls to NSObject.IsDirectBinding to load a constant value. Makes the app smaller, and slightly faster at runtime.\n" + #endif #if MONOTOUCH " remove-dynamic-registrar: By default enabled when the static registrar is enabled. Removes the dynamic registrar (makes the app smaller).\n" + @@ -143,7 +143,12 @@ static void AddSharedOptions (Application app, Mono.Options.OptionSet options) " blockliteral-setupblock: By default enabled when using the static registrar. Optimizes calls to BlockLiteral.SetupBlock to avoid having to calculate the block signature at runtime.\n" + " inline-intptr-size: By default enabled for builds that target a single architecture (requires the linker). Inlines calls to IntPtr.Size to load a constant value. Makes the app smaller, and slightly faster at runtime.\n" + " inline-dynamic-registration-supported: By default always enabled (requires the linker). Optimizes calls to Runtime.DynamicRegistrationSupported to be a constant value. Makes the app smaller, and slightly faster at runtime.\n" + - " register-protocols: Remove unneeded metadata for protocol support. Makes the app smaller and reduces memory requirements.\n", + " register-protocols: Remove unneeded metadata for protocol support. Makes the app smaller and reduces memory requirements.\n" + +#if !MONOTOUCH + " trim-architectures: Remove unneeded architectures from bundled native libraries. Makes the app smaller and is required for macOS App Store submissions.\n", +#else + "", +#endif (v) => { app.Optimizations.Parse (v); }); diff --git a/tools/common/MachO.cs b/tools/common/MachO.cs index 3f6e9316a900..6c8f7d4f08da 100644 --- a/tools/common/MachO.cs +++ b/tools/common/MachO.cs @@ -11,6 +11,25 @@ namespace Xamarin { + [Flags] + public enum Abi { + None = 0, + i386 = 1, + ARMv6 = 2, + ARMv7 = 4, + ARMv7s = 8, + ARM64 = 16, + x86_64 = 32, + Thumb = 64, + LLVM = 128, + ARMv7k = 256, + SimulatorArchMask = i386 | x86_64, + DeviceArchMask = ARMv6 | ARMv7 | ARMv7s | ARMv7k | ARM64, + ArchMask = SimulatorArchMask | DeviceArchMask, + Arch64Mask = x86_64 | ARM64, + Arch32Mask = i386 | ARMv6 | ARMv7 | ARMv7s | ARMv7k, + } + public class MachO { /* definitions from: /usr/include/mach-o/loader.h */ @@ -305,7 +324,6 @@ public static IEnumerable GetNativeDependencies (string libraryName) return result; } -#if MTOUCH public static List GetArchitectures (string file) { var result = new List (); @@ -409,7 +427,6 @@ public static bool IsDynamicFramework (string filename) return true; } -#endif } public class StaticLibrary diff --git a/tools/common/Optimizations.cs b/tools/common/Optimizations.cs index 8530c2e6a011..ce67746d807f 100644 --- a/tools/common/Optimizations.cs +++ b/tools/common/Optimizations.cs @@ -22,6 +22,11 @@ public class Optimizations "remove-dynamic-registrar", #else "", // dummy value to make indices match up between XM and XI +#endif +#if MONOTOUCH + "", // dummy value to make indices match up between XM and XI +#else + "trim-architectures", #endif }; @@ -36,6 +41,7 @@ enum Opt RegisterProtocols, InlineDynamicRegistrationSupported, RemoveDynamicRegistrar, + TrimArchitectures, } bool? [] values; @@ -79,6 +85,11 @@ public bool? RemoveDynamicRegistrar { get { return values [(int) Opt.RemoveDynamicRegistrar]; } set { values [(int) Opt.RemoveDynamicRegistrar] = value; } } + + public bool? TrimArchitectures { + get { return values [(int) Opt.TrimArchitectures]; } + set { values [(int) Opt.TrimArchitectures] = value; } + } public Optimizations () { @@ -92,6 +103,8 @@ public void Initialize (Application app) if (!values [i].HasValue) continue; switch ((Opt) i) { + case Opt.TrimArchitectures: + break; // Does not require linker case Opt.RegisterProtocols: case Opt.RemoveDynamicRegistrar: if (app.Registrar != RegistrarMode.Static) { @@ -181,6 +194,12 @@ public void Initialize (Application app) } } +#if !MONOTOUCH + // By default on macOS trim-architectures for Release and not for debug + if (!TrimArchitectures.HasValue) + TrimArchitectures = !app.EnableDebug; +#endif + if (Driver.Verbosity > 3) Driver.Log (4, "Enabled optimizations: {0}", string.Join (", ", values.Select ((v, idx) => v == true ? opt_names [idx] : string.Empty).Where ((v) => !string.IsNullOrEmpty (v)))); } diff --git a/tools/common/StringUtils.cs b/tools/common/StringUtils.cs index 8e48372f428d..7cb68a3c4c19 100644 --- a/tools/common/StringUtils.cs +++ b/tools/common/StringUtils.cs @@ -65,4 +65,9 @@ public static Version ParseVersion (string v) return Version.Parse (v); } } -} \ No newline at end of file + + static class StringExtensions + { + public static string [] SplitLines (this string s) => s.Split (new [] { Environment.NewLine }, StringSplitOptions.None); + } +} diff --git a/tools/mmp/driver.cs b/tools/mmp/driver.cs index 87e44bb44c79..1afb826d0126 100644 --- a/tools/mmp/driver.cs +++ b/tools/mmp/driver.cs @@ -1171,6 +1171,9 @@ static void HandleFramework (StringBuilder args, string framework, bool weak) CreateDirectoryIfNeeded (frameworks_dir); Application.UpdateDirectory (framework, frameworks_dir); frameworks_copied_to_bundle_dir = true; + + if (App.Optimizations.TrimArchitectures == true) + LipoLibrary (framework, Path.Combine (name, Path.Combine (frameworks_dir, name + ".framework", name))); } static int Compile () @@ -1632,7 +1635,7 @@ static void ProcessNativeLibrary (HashSet processed, string library, Lis if (ignored_assemblies.Contains (library)) return; - // If we're as passed in framework, ignore + // If we're passed in a framework, ignore if (App.Frameworks.Contains (library)) return; @@ -1695,6 +1698,11 @@ static void ProcessNativeLibrary (HashSet processed, string library, Lis } bool isStaticLib = real_src.EndsWith (".a", StringComparison.Ordinal); + bool isDynamicLib = real_src.EndsWith (".dylib", StringComparison.Ordinal); + + if (isDynamicLib && App.Optimizations.TrimArchitectures == true) + LipoLibrary (name, dest); + if (native_references.Contains (real_src)) { if (!isStaticLib) { int ret = XcodeRun ("install_name_tool -id", string.Format ("{0} {1}", StringUtils.Quote("@executable_path/../" + BundleName + "/" + name), StringUtils.Quote(dest))); @@ -1724,6 +1732,21 @@ static void ProcessNativeLibrary (HashSet processed, string library, Lis } } + static void LipoLibrary (string name, string dest) + { + var existingArchs = Xamarin.MachO.GetArchitectures (dest); + // If we have less than two, it will either match by default or + // we'll fail a link/launch time with a better error so bail + if (existingArchs.Count () < 2) + return; + + int ret = XcodeRun ("lipo", $"{StringUtils.Quote (dest)} -thin {arch} -output {StringUtils.Quote (dest)}"); + if (ret != 0) + throw new MonoMacException (5311, true, "lipo failed with an error code '{0}'. Check build log for details.", ret); + if (name != "MonoPosixHelper") + ErrorHelper.Warning (2108, $"{name} was stripped of architectures except {arch} to comply with App Store restrictions. This could break exisiting codesigning signatures. Consider stripping the library with lipo or disabling with --optimize=-trim-architectures"); + } + static void CreateSymLink (string directory, string real, string link) { string cd = Environment.CurrentDirectory; @@ -1863,10 +1886,10 @@ static void CopyAssemblies () { if (App.EnableDebug) { var mdbfile = asm + ".mdb"; if (File.Exists (mdbfile)) - File.Copy (mdbfile, Path.Combine (mmp_dir, Path.GetFileName (mdbfile)), true); + CopyFileAndRemoveReadOnly (mdbfile, Path.Combine (mmp_dir, Path.GetFileName (mdbfile))); var pdbfile = Path.ChangeExtension (asm, ".pdb"); if (File.Exists (pdbfile)) - File.Copy (pdbfile, Path.Combine (mmp_dir, Path.GetFileName (pdbfile)), true); + CopyFileAndRemoveReadOnly (pdbfile, Path.Combine (mmp_dir, Path.GetFileName (pdbfile))); } if (File.Exists (configfile)) File.Copy (configfile, Path.Combine (mmp_dir, Path.GetFileName (configfile)), true); diff --git a/tools/mtouch/Application.cs b/tools/mtouch/Application.cs index 01bfcfcb60c7..7c3600a30790 100644 --- a/tools/mtouch/Application.cs +++ b/tools/mtouch/Application.cs @@ -25,25 +25,6 @@ public enum BitCodeMode { MarkerOnly = 3, } - [Flags] - public enum Abi { - None = 0, - i386 = 1, - ARMv6 = 2, - ARMv7 = 4, - ARMv7s = 8, - ARM64 = 16, - x86_64 = 32, - Thumb = 64, - LLVM = 128, - ARMv7k = 256, - SimulatorArchMask = i386 | x86_64, - DeviceArchMask = ARMv6 | ARMv7 | ARMv7s | ARMv7k | ARM64, - ArchMask = SimulatorArchMask | DeviceArchMask, - Arch64Mask = x86_64 | ARM64, - Arch32Mask = i386 | ARMv6 | ARMv7 | ARMv7s | ARMv7k, - } - public static class AbiExtensions { public static string AsString (this Abi self) {