[linker] Remove non-bitcode compatible code, and show a warning. (xam…

…arin#5551)

* [linker] Remove non-bitcode compatible code, and show a warning.

Remove code not currently compatible with bitcode and replace it with an
exception instead (otherwise we'll assert at runtime).

Also show a warning when we detect this.

This is quite helpful when looking at watch device test runs to filter out
failures we already know about.

This fixes point #2 in xamarin#4763.

* Improve documentation.

* Simplify linker code by using a substep.

* Fix whitespace issues.

* Improve reporting.

* Add support for reporting more than one MT2105 at the same time when making
  the errors instead of warnings.
* Only report MT2105 for methods that haven't been linked away.
* Format the error message nicer for properties.

* Tweak a bit for warning tests to pass.

* Use ExceptionalSubStep to provide better error information.

* Adjust where linker warnings/errors are reported from to avoid a NullReferenceException.

docs/website/mtouch-errors.md

@@ -1869,6 +1869,41 @@ 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.
 
+<a name="MT2105" />
+
+### MT2105: The method {method} contains a '{handlerType}' exception clause, which is currently not supported when compiling for bitcode. This method will throw an exception if called.
+
+Currently Xamarin.iOS does not support the 'filter' exception clauses when
+compiling to bitcode. Any methods containing such code will throw a
+NotSupportedException exception when the method is executed (the exception
+will be thrown at method entry, even if execution would follow a path that did
+not involve the 'filter' exception clause).
+
+This is an example of code that's not supported:
+
+```csharp
+void MyMethod ()
+{
+	try {
+		throw new Exception ("FilterMe");
+	} catch (Exception e)
+		when (e.Message == "FilterMe") // <- This is the filter clause.
+	{
+		Console.WriteLine ("filtered");
+	}
+}
+```
+
+This method will behave like the following example, throwing an exception at
+method entry:
+
+```csharp
+void MyMethod ()
+{
+	throw new NotSupportedException ("This method contains IL not supported when compiled to bitcode.");
+}
+```
+
 <a name="MT2200" />
 <a name="MT2201" />
 <a name="MT2202" />
@@ -1886,6 +1921,33 @@ Something unexpected occured when trying to mark the conversion methods for smar
 
 <!-- MT2200 - MT2209 used by the above error -->
 
+<a name="MT2210" />
+<a name="MT2211" />
+<a name="MT2212" />
+<a name="MT2213" />
+<a name="MT2214" />
+<a name="MT2215" />
+<a name="MT2216" />
+<a name="MT2217" />
+<a name="MT2218" />
+<a name="MT2219" />
+
+### MT221x: Incompatible Code For Bitcode Remover failed processing `...`.
+
+Something unexpected occured when trying to remove incompatible code for
+bitcode from the application. The assembly causing the issue is named in the
+error message. In order to fix this issue the assembly will need to be
+provided in a new issue on
+[github](https://github.com/xamarin/xamarin-macios/issues/new) along with a
+complete build log with verbosity enabled (i.e. `-v -v -v -v` in the
+**Additional mtouch arguments** in the project's watchOS Build options).
+
+It's usually possible to work around this by adding
+`--optimize=-remove-unsupported-il-for-bitcode` to the **Additional mtouch arguments**
+in the project's watchOS Build options.
+
+<!-- MT2210 - MT2219 used by the above error -->
+
 ## MT3xxx: AOT error messages
 
 <!--

docs/website/optimizations.md

@@ -604,3 +604,47 @@ It is always enabled by default (as long as the static registrar is enabled).
 
 The default behavior can be overridden by passing
 `--optimize=[+|-]static-delegate-to-block-lookup` to mtouch/mmp.
+
+## Remove unsupported IL for bitcode
+
+Removes unsupported IL for bitcode, and replaces it with a `NotSupportedException`.
+
+There are certain types of IL that Xamarin.iOS doesn't support when compiling
+to bitcode. This optimization will replace the unsupported IL with an
+`NotSupportedException`, and will emit a warning at build time.
+
+This ensures that any unsupported IL will be detected at runtime even when not
+compiling to bitcode (in particular it will mean that the behavior between
+Debug and Release device builds is identical, since Debug builds do not
+compile to bitcode, while Release builds do).
+
+This optimization will change the following code:
+
+```csharp
+void Method ()
+{
+	try {
+		throw new Exception ("FilterMe");
+	} catch (Exception e) when (e.Message == "FilterMe") {
+		Console.WriteLine ("filtered");
+	}
+}
+```
+
+into the following:
+
+```csharp
+void Method ()
+{
+	throw new NotSupportedException ("This method contains IL not supported when compiled to bitcode.");
+}
+```
+
+This optimization does not require the linker to be enabled, it will process
+all assemblies, even those not linked.
+
+It is only applicable to watchOS, and then it's enabled by default when
+building for device.
+
+The default behavior can be overridden by passing
+`--optimize=[+|-]remove-unsupported-il-for-bitcode` to mtouch/mmp.

tests/linker/ios/link sdk/BitcodeTest.cs

@@ -0,0 +1,96 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+using ObjCRuntime;
+
+using NUnit.Framework;
+
+namespace LinkSdk {
+	[TestFixture]
+	public class BitcodeTest {
+		[Test]
+		public void FilterClauseTest ()
+		{
+			var supported = true;
+#if __WATCHOS__
+			if (Runtime.Arch == Arch.DEVICE)
+				supported = false;
+#endif
+			if (supported) {
+				Assert.AreEqual (0, FilterClause (), "Filter me");
+				Assert.AreEqual (10, FilterClauseProperty, "Filter me getter");
+				Assert.DoesNotThrow (() => FilterClauseProperty = 20, "Filter me setter");
+			} else {
+				Assert.Throws<NotSupportedException> (() => FilterClause (), "Filter me not supported");
+				Assert.Throws<NotSupportedException> (() => { var x = FilterClauseProperty; }, "Filter me getter not supported");
+				Assert.Throws<NotSupportedException> (() => FilterClauseProperty = 30, "Filter me setter not supported");
+			}
+		}
+
+		// A method with a filter clause
+		// mtouch will show a warning for this method (MT2105) when building for watchOS device. This is expected.
+		static int FilterClause ()
+		{
+			try {
+				throw new Exception ("FilterMe");
+			} catch (Exception e) when (e.Message == "FilterMe") {
+				return 0;
+			} catch {
+				return 1;
+			}
+		}
+
+		// A property with a filter clause
+		// mtouch will show a warning for this property (MT2105) when building for watchOS device. This is expected.
+		static int FilterClauseProperty {
+			get {
+				try {
+					throw new Exception ("FilterMe");
+				} catch (Exception e) when (e.Message == "FilterMe") {
+					return 10;
+				} catch {
+					return 11;
+				}
+			}
+			set {
+				try {
+					throw new Exception ("FilterMe");
+				} catch (Exception e) when (e.Message == "FilterMe") {
+				} catch {
+					Assert.Fail ("Filter failure: {0}", value);
+				}
+			}
+		}
+
+		[Test]
+		public void FaultClauseTest ()
+		{
+			// First assert that the method that is supposed to have a fault clause actually has a fault clause.
+			// This is somewhat complicated because it's not the method we call that has the fault clause, but a generated method in a generated class.
+			// This is because we only have an indirect way of making csc produce a fault clause
+			var enumeratorType = GetType ().GetNestedTypes (BindingFlags.NonPublic).First ((v) => v.Name.Contains ($"<{nameof (FaultClause)}>"));
+			var method = enumeratorType.GetMethod ("MoveNext", BindingFlags.Instance | BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public);
+			var body = method.GetMethodBody ();
+			Assert.IsTrue (body.ExceptionHandlingClauses.Any ((v) => v.Flags == ExceptionHandlingClauseOptions.Fault), "Any fault clauses");
+
+			// Then assert that the method can be called successfully.
+			var rv = FaultClause ().ToArray ();
+			Assert.AreEqual (1, rv.Count (), "Count");
+			Assert.AreEqual (1, rv [0], "Item 1");
+
+		}
+
+		// The C# compiler uses fault clauses for 'using' blocks inside iterators.
+		static IEnumerable<int> FaultClause ()
+		{
+			using (var d = new Disposable ()) {
+				yield return 1;
+			}
+		}
+		class Disposable : IDisposable {
+			void IDisposable.Dispose () { }
+		}
+	}
+}

tests/linker/ios/link sdk/link sdk.csproj

@@ -192,6 +192,7 @@
     <Compile Include="..\..\CommonLinkSdkTest.cs">
       <Link>CommonLinkSdkTest.cs</Link>
     </Compile>
+    <Compile Include="BitcodeTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <InterfaceDefinition Include="LaunchScreen.storyboard" Condition="'$(TargetFrameworkIdentifier)' != 'Xamarin.WatchOS'" />

tests/mtouch/MTouch.cs

@@ -1752,7 +1752,7 @@ public void MT0132 ()
 				mtouch.Linker = MTouchLinker.LinkSdk;
 				mtouch.Optimize = new string [] { "foo" };
 				mtouch.AssertExecute (MTouchAction.BuildSim, "build");
-				mtouch.AssertWarning (132, "Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, inline-runtime-arch, blockliteral-setupblock, register-protocols, inline-dynamic-registration-supported, static-block-to-delegate-lookup, remove-dynamic-registrar.");
+				mtouch.AssertWarning (132, "Unknown optimization: 'foo'. Valid optimizations are: remove-uithread-checks, dead-code-elimination, inline-isdirectbinding, inline-intptr-size, inline-runtime-arch, blockliteral-setupblock, register-protocols, inline-dynamic-registration-supported, static-block-to-delegate-lookup, remove-dynamic-registrar, remove-unsupported-il-for-bitcode.");
 			}
 		}
 
@@ -2910,6 +2910,65 @@ public void MT1216 ()
 			}
 		}
 
+		[Test]
+		public void MT2105 ()
+		{
+
+			using (var ext = new MTouchTool ()) {
+				var code = @"
+class TestClass {
+	// A method with a filter clause
+	static int FilterClause ()
+	{
+		try {
+			throw new System.Exception (""FilterMe"");
+		} catch (System.Exception e) when (e.Message == ""FilterMe"") {
+			return 0;
+		} catch {
+			return 1;
+		}
+	}
+	static int FilterClauseProperty {
+		get {
+			try {
+				throw new System.Exception (""FilterMe"");
+			} catch (System.Exception e) when (e.Message == ""FilterMe"") {
+				return 10;
+			} catch {
+				return 11;
+			}
+		}
+		set {
+			try {
+				throw new System.Exception (""FilterMe"");
+			} catch (System.Exception e) when (e.Message == ""FilterMe"") {
+			} catch {
+				System.Console.WriteLine (""Filter failure: {0}"", value);
+			}
+		}
+	}
+}
+				";
+				ext.Profile = Profile.watchOS;
+				ext.Linker = MTouchLinker.LinkSdk;
+				ext.CreateTemporaryDirectory ();
+				ext.CreateTemporaryWatchKitExtension (extraCode: code, extraArg: "/debug");
+				ext.WarnAsError = new int [] { 2105 };
+				ext.AssertExecuteFailure (MTouchAction.BuildDev);
+				ext.AssertError (2105, "The method TestClass.FilterClause contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This method will throw an exception if called.", "testApp.cs", 9);
+				ext.AssertError (2105, "The property TestClass.FilterClauseProperty contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This property will throw an exception if called.", "testApp.cs", 19);
+				ext.AssertError (2105, "The property TestClass.FilterClauseProperty contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This property will throw an exception if called.", "testApp.cs", 28);
+				ext.AssertErrorCount (3);
+
+				ext.Optimize = new string [] { "remove-unsupported-il-for-bitcode" };
+				ext.AssertExecuteFailure (MTouchAction.BuildSim);
+				ext.AssertError (2105, "The method TestClass.FilterClause contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This method will throw an exception if called.", "testApp.cs", 9);
+				ext.AssertError (2105, "The property TestClass.FilterClauseProperty contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This property will throw an exception if called.", "testApp.cs", 19);
+				ext.AssertError (2105, "The property TestClass.FilterClauseProperty contains a 'Filter' exception clause, which is currently not supported when compiling for bitcode. This property will throw an exception if called.", "testApp.cs", 28);
+				ext.AssertErrorCount (3);
+			}
+		}
+
 		[Test]
 		public void MT5211 ()
 		{
@@ -3510,10 +3569,11 @@ public void MT2003 ()
 				mtouch.CreateTemporaryApp ();
 				mtouch.Linker = MTouchLinker.DontLink;
 				mtouch.Debug = true; // makes simlauncher possible, which speeds up the build
-				mtouch.Optimize = new string [] { "-inline-intptr-size" };
+				mtouch.Optimize = new string [] { "-inline-intptr-size", "remove-unsupported-il-for-bitcode" };
 				mtouch.AssertExecute (MTouchAction.BuildSim);
 				mtouch.AssertWarning (2003, "Option '--optimize=-inline-intptr-size' will be ignored since linking is disabled");
-				mtouch.AssertWarningCount (1);
+				mtouch.AssertWarning (2003, "Option '--optimize=remove-unsupported-il-for-bitcode' will be ignored since it's only applicable to watchOS.");
+				mtouch.AssertWarningCount (2);
 			}
 		}
 

tests/mtouch/MTouchTool.cs

@@ -621,7 +621,7 @@ public void WidgetPerformUpdate (Action<NCUpdateResult> completionHandler)
 				File.WriteAllText (plist_path, info_plist);
 		}
 
-		public void CreateTemporaryWatchKitExtension (string code = null)
+		public void CreateTemporaryWatchKitExtension (string code = null, string extraCode = null, string extraArg = "")
 		{
 			string testDir;
 			if (RootAssembly == null) {
@@ -641,9 +641,12 @@ protected NotificationController (System.IntPtr handle) : base (handle) {}
 }";
 			}
 
+			if (extraCode != null)
+				code += extraCode;
+
 			AppPath = app;
 			Extension = true;
-			RootAssembly = MTouch.CompileTestAppLibrary (testDir, code: code, profile: Profile);
+			RootAssembly = MTouch.CompileTestAppLibrary (testDir, code: code, extraArg: extraArg, profile: Profile);
 
 			File.WriteAllText (Path.Combine (app, "Info.plist"), @"<?xml version=""1.0"" encoding=""UTF-8""?>
 <!DOCTYPE plist PUBLIC ""-//Apple//DTD PLIST 1.0//EN"" ""http://www.apple.com/DTDs/PropertyList-1.0.dtd"">

tools/common/DerivedLinkContext.cs

@@ -17,6 +17,12 @@ public class DerivedLinkContext : LinkContext
 		internal Target Target;
 		Symbols required_symbols;
 
+		// Any errors or warnings during the link process that won't prevent linking from continuing can be stored here.
+		// This is typically used to show as many problems as possible per build (so that the user doesn't have to fix one thing, rebuild, fix another, rebuild, fix another, etc).
+		// Obviously if the problem is such that cascading errors may end up reported, this should not be used.
+		// The errors/warnings will be shown when the linker is done.
+		public List<Exception> Exceptions = new List<Exception> ();
+
 		// SDK candidates - they will be preserved only if the application (not the SDK) uses it
 		List<ICustomAttributeProvider> srs_data_contract = new List<ICustomAttributeProvider> ();
 		List<ICustomAttributeProvider> xml_serialization = new List<ICustomAttributeProvider> ();