From f6062a0d74e1a2012bc5a068991a7b39ef6863f8 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Tue, 20 May 2025 21:00:46 +0200 Subject: [PATCH 01/15] [bgen] Don't emit/use [Preserve] attributes anymore. Fixes #19524. Fixes https://github.com/dotnet/macios/issues/19524. --- src/bgen/Generator.cs | 26 +++++++++++--------------- src/foundation.cs | 2 -- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index 91d31987b642..07674f4c69ae 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -1597,11 +1597,6 @@ void GenerateTrampolinesForQueue (TrampolineInfo [] queue) print ("//\n// This class bridges native block invocations that call into C#\n//"); PrintExperimentalAttribute (ti.Type); print ("static internal class {0} {{", ti.StaticName); indent++; - // it can't be conditional without fixing https://github.com/mono/linker/issues/516 - // but we have a workaround in place because we can't fix old, binary bindings so... - // print ("[Preserve (Conditional=true)]"); - // For .NET we fix it using the DynamicDependency attribute below - print ("[Preserve (Conditional = true)]"); print ("[UnmanagedCallersOnly]"); print ("[UserDelegateType (typeof ({0}))]", ti.UserDelegate); print ("internal static unsafe {0} Invoke ({1}) {{", ti.ReturnType, ti.Parameters); @@ -1674,7 +1669,6 @@ void GenerateTrampolinesForQueue (TrampolineInfo [] queue) print ("invoker = block->GetDelegateForBlock<{0}> ();", ti.DelegateName); indent--; print ("}"); print (""); - print ("[Preserve (Conditional=true)]"); print_generated_code (); print ("public unsafe static {0}? Create (IntPtr block)\n{{", ti.UserDelegate); indent++; print ("if (block == IntPtr.Zero)"); indent++; @@ -1812,13 +1806,11 @@ void GenerateStrongDictionaryTypes () if (BindingTouch.SupportsXmlDocumentation) { print ($"/// Creates a new with default (empty) values."); } - print ("[Preserve (Conditional = true)]"); print ("public {0} () : base (new NSMutableDictionary ()) {{}}\n", typeName); if (BindingTouch.SupportsXmlDocumentation) { print ($"/// Creates a new from the values that are specified in ."); print ($"/// The dictionary to use to populate the properties of this type."); } - print ("[Preserve (Conditional = true)]"); print ("public {0} (NSDictionary? dictionary) : base (dictionary) {{}}\n", typeName); foreach (var pi in dictType.GatherProperties (this)) { @@ -4986,6 +4978,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, } } + var dynamicDependencies = new List (); if (instanceMethods.Any () || instanceProperties.Any ()) { // Tell the trimmer to not remove any instance method/property if the interface itself isn't trimmed away. // These members are required for the registrar to determine if a particular implementing method @@ -4995,11 +4988,14 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, // Ref: https://github.com/dotnet/runtime/issues/37352#issuecomment-644385807 var docIds = instanceMethods .Select (mi => DocumentationManager.GetDocId (mi, includeDeclaringType: false, alwaysIncludeParenthesis: true)) - .Concat (instanceProperties.Select (v => v.Name)) - .OrderBy (name => name); - foreach (var docId in docIds) { + .Concat (instanceProperties.Select (v => v.Name)); + dynamicDependencies.AddRange (docIds); + } + // Tell the trimmer to not remove the wrapper type if the interface itself isn't trimmed away + dynamicDependencies.Add ($"T:{(type.Namespace is not null ? type.Namespace + "." : "")}{TypeName}Wrapper"); + if (dynamicDependencies.Count > 0) { + foreach (var docId in dynamicDependencies.OrderBy (v => v)) print ($"[DynamicDependencyAttribute (\"{docId}\")]"); - } print ("[BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]"); print ($"static I{TypeName} ()"); print ("{"); @@ -5112,7 +5108,6 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, indent++; // ctor (IntPtr, bool) PrintExperimentalAttribute (type); - print ("[Preserve (Conditional = true)]"); print ("public {0}Wrapper ({1} handle, bool owns)", TypeName, NativeHandleType); print ("\t: base (handle, owns)"); print ("{"); @@ -5330,6 +5325,9 @@ public void PrintPreserveAttribute (ICustomAttributeProvider mi) if (p is null) return; + if (!BindThirdPartyLibrary) + throw new InvalidOperationException ($"Found [Preserve] on {FormatProvider (mi)}: [Preserve] is deprecated, so don't use it."); + if (p.AllMembers) print ("[Preserve (AllMembers = true)]"); else if (p.Conditional) @@ -6522,7 +6520,6 @@ public void Generate (Type type) } else print ("internal {0}? {1};", Nomenclator.GetDelegateName (mi), miname); - print ("[Preserve (Conditional = true)]"); if (isProtocolEventBacked) print ("[Export (\"{0}\")]", FindSelector (dtype, mi)); @@ -6629,7 +6626,6 @@ public void Generate (Type type) selRespondsToSelector = "selRespondsToSelector"; } - print ("[Preserve (Conditional = true)]"); print ("public override bool RespondsToSelector (Selector? sel)"); print ("{"); ++indent; diff --git a/src/foundation.cs b/src/foundation.cs index a51ef9df340a..30580e3e4710 100644 --- a/src/foundation.cs +++ b/src/foundation.cs @@ -7394,8 +7394,6 @@ interface NSTimer { [Export ("fireDate", ArgumentSemantic.Copy)] NSDate FireDate { get; set; } - // Note: preserving this member allows us to re-enable the `Optimizable` binding flag - [Preserve (Conditional = true)] [Export ("invalidate")] void Invalidate (); From be87853b10473756815b094845cc7b04d2134a75 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 22 May 2025 09:38:41 +0200 Subject: [PATCH 02/15] Use different DDA overload. --- src/bgen/Generator.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index 07674f4c69ae..25e893e33f5e 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -4988,14 +4988,15 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, // Ref: https://github.com/dotnet/runtime/issues/37352#issuecomment-644385807 var docIds = instanceMethods .Select (mi => DocumentationManager.GetDocId (mi, includeDeclaringType: false, alwaysIncludeParenthesis: true)) - .Concat (instanceProperties.Select (v => v.Name)); + .Concat (instanceProperties.Select (v => v.Name)) + .Select (v => $"\"{v}\""); dynamicDependencies.AddRange (docIds); } // Tell the trimmer to not remove the wrapper type if the interface itself isn't trimmed away - dynamicDependencies.Add ($"T:{(type.Namespace is not null ? type.Namespace + "." : "")}{TypeName}Wrapper"); + dynamicDependencies.Add ($"DynamicallyAccessedMemberTypes.PublicConstructors, typeof ({TypeName}Wrapper)"); if (dynamicDependencies.Count > 0) { - foreach (var docId in dynamicDependencies.OrderBy (v => v)) - print ($"[DynamicDependencyAttribute (\"{docId}\")]"); + foreach (var dd in dynamicDependencies.OrderBy (v => v)) + print ($"[DynamicDependencyAttribute ({dd})]"); print ("[BindingImpl (BindingImplOptions.GeneratedCode | BindingImplOptions.Optimizable)]"); print ($"static I{TypeName} ()"); print ("{"); From 670f60b6e825b34ede8ac2d038fd45666d249f1b Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 26 May 2025 12:47:06 +0200 Subject: [PATCH 03/15] [dotnet-linker] Preserve a bit more. --- .../Steps/ManagedRegistrarLookupTablesStep.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 0d3461aa7a45..a2aee8954856 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -393,8 +393,14 @@ void GenerateConstructINativeObject (TypeDefinition registrarType) var ctor = abr.CurrentAssembly.MainModule.ImportReference (ctorRef); // we need to preserve the constructor because it might not be used anywhere else - if (IsTrimmed (ctor)) - Annotations.Mark (ctor.Resolve ()); + if (IsTrimmed (ctor)) { + var ctorDefinition = ctor.Resolve (); + Annotations.Mark (ctorDefinition); + foreach (var instr in ctorDefinition.Body.Instructions) { + if (instr.Operand is MethodReference mr) + Annotations.Mark (mr.Resolve ()); + } + } if (!ManagedRegistrarStep.IsOpenType (type)) { EnsureVisible (createInstanceMethod, ctor); From ac90a691183aee3048d8194fcdc1aec5af84a607 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 26 May 2025 12:57:50 +0200 Subject: [PATCH 04/15] NSTimer fix --- src/Foundation/NSTimer.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Foundation/NSTimer.cs b/src/Foundation/NSTimer.cs index a3566cd4886c..ce8edfa9d5ea 100644 --- a/src/Foundation/NSTimer.cs +++ b/src/Foundation/NSTimer.cs @@ -21,6 +21,7 @@ // Copyright 2011-2014 Xamarin Inc. // using System; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Collections; using System.Runtime.InteropServices; @@ -124,5 +125,18 @@ public NSTimer (NSDate date, TimeSpan when, Action action, System.Boole : this (date, when.TotalSeconds, new NSTimerActionDispatcher (action), NSTimerActionDispatcher.Selector, null, repeats) { } + + // The dependency attribute is required because: + // * We inject a call to Invalidate in the generated Dispose method + // * We optimize Dispose methods to not touch fields that aren't otherwise used, which we do by + // removing the contents of the Dispose method before the linker runs, then after the linker runs + // we determine which fields the Dispose method touches have been linked away and remove any such code. + // We won't remove the call to Invalidate, but the linker may have trimmed away the Invalidate method + /// itself (because we temporarly removed the call to it). So make sure the linker doesn't remove it. + [DynamicDependency ("Invalidate()")] + static NSTimer () + { + GC.KeepAlive (null); + } } } From 15d6197594385242affc9b7ce8d7d3dbe4997b07 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Tue, 27 May 2025 11:42:34 +0200 Subject: [PATCH 05/15] Preserve the Create method --- src/bgen/Generator.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index 25e893e33f5e..7ace2c6678ca 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -1669,6 +1669,12 @@ void GenerateTrampolinesForQueue (TrampolineInfo [] queue) print ("invoker = block->GetDelegateForBlock<{0}> ();", ti.DelegateName); indent--; print ("}"); print (""); + print ("[DynamicDependency (nameof (Create))]"); + print ($"static {ti.NativeInvokerName} ()"); + print ("{"); + print ("\tGC.KeepAlive (null);"); // need to do _something_ (doesn't seem to matter what), otherwise the static cctor (and the DynamicDependency attributes) are trimmed away. + print ("}"); + print (""); print_generated_code (); print ("public unsafe static {0}? Create (IntPtr block)\n{{", ti.UserDelegate); indent++; print ("if (block == IntPtr.Zero)"); indent++; From d4ca89e31cc1363fa6f659c079fedff4d663399e Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 29 May 2025 19:51:00 +0200 Subject: [PATCH 06/15] Fix trimming away cctors. --- src/bgen/Generator.cs | 2 +- .../Steps/SetBeforeFieldInitStep.cs | 6 +++++ tools/linker/CoreOptimizeGeneratedCode.cs | 22 ++++++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index 7ace2c6678ca..fa16ce2a4ffa 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -4999,7 +4999,7 @@ void GenerateProtocolTypes (Type type, string class_visibility, string TypeName, dynamicDependencies.AddRange (docIds); } // Tell the trimmer to not remove the wrapper type if the interface itself isn't trimmed away - dynamicDependencies.Add ($"DynamicallyAccessedMemberTypes.PublicConstructors, typeof ({TypeName}Wrapper)"); + dynamicDependencies.Add ($"DynamicallyAccessedMemberTypes.Interfaces | DynamicallyAccessedMemberTypes.PublicConstructors, typeof ({TypeName}Wrapper)"); if (dynamicDependencies.Count > 0) { foreach (var dd in dynamicDependencies.OrderBy (v => v)) print ($"[DynamicDependencyAttribute ({dd})]"); diff --git a/tools/dotnet-linker/Steps/SetBeforeFieldInitStep.cs b/tools/dotnet-linker/Steps/SetBeforeFieldInitStep.cs index 22f70f051ef0..1ea1dcb0943b 100644 --- a/tools/dotnet-linker/Steps/SetBeforeFieldInitStep.cs +++ b/tools/dotnet-linker/Steps/SetBeforeFieldInitStep.cs @@ -45,6 +45,12 @@ protected override void Process (TypeDefinition type) if (Configuration.DerivedLinkContext.App.Optimizations.RegisterProtocols != true) return; + if (Configuration.DerivedLinkContext.App.XamarinRuntime == Bundler.XamarinRuntime.NativeAOT) { + // We can't remove the static constructor in the trimmer if we're using NativeAOT, + // because NativeAOT needs it for its own trimming logic. + return; + } + if (!type.IsBeforeFieldInit && type.IsInterface && type.HasMethods) { var cctor = type.GetTypeConstructor (); if (cctor is not null && cctor.IsBindingImplOptimizableCode (LinkContext)) diff --git a/tools/linker/CoreOptimizeGeneratedCode.cs b/tools/linker/CoreOptimizeGeneratedCode.cs index 80a5ffe3bd89..7159d1e678c4 100644 --- a/tools/linker/CoreOptimizeGeneratedCode.cs +++ b/tools/linker/CoreOptimizeGeneratedCode.cs @@ -1413,7 +1413,27 @@ bool ProcessProtocolInterfaceStaticConstructor (MethodDefinition method) Driver.Log (4, "Optimized static constructor in the protocol interface {0} (static constructor was cleared and custom attributes removed)", method.DeclaringType.FullName); method.Body.Instructions.Clear (); method.Body.Instructions.Add (Instruction.Create (OpCodes.Ret)); - method.CustomAttributes.Clear (); + + // Only remove DynamicDependency attributes that takes a single string argument. + // The generator generates other DynamicDependency attributes, and we don't want to remove those. + for (var i = method.CustomAttributes.Count - 1; i >= 0; i--) { + var ca = method.CustomAttributes [i]; + + if (!ca.AttributeType.Is ("System.Diagnostics.CodeAnalysis", "DynamicDependencyAttribute")) + continue; + + if (!ca.HasConstructorArguments) + continue; + + if (ca.ConstructorArguments.Count != 1) + continue; + + if (!ca.ConstructorArguments [0].Type.Is ("System", "String")) + continue; + + method.CustomAttributes.RemoveAt (i); + } + return true; } } From e677f642135003369f696de3540eda17222ad8ca Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 29 May 2025 19:51:08 +0200 Subject: [PATCH 07/15] Test fix --- .../Photos/LivePhotoEditingContextTest.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/monotouch-test/Photos/LivePhotoEditingContextTest.cs b/tests/monotouch-test/Photos/LivePhotoEditingContextTest.cs index a1be30146ac6..6cdb82e2c34c 100644 --- a/tests/monotouch-test/Photos/LivePhotoEditingContextTest.cs +++ b/tests/monotouch-test/Photos/LivePhotoEditingContextTest.cs @@ -48,6 +48,17 @@ public void Linker () [UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "This test pokes at internals, so it's expected to not be trimmer safe. It works though, so unless something changes, we're going to assume it's trimmer-compatible.")] [UnconditionalSuppressMessage ("Trimming", "IL2075", Justification = "This test pokes at internals, so it's expected to not be trimmer safe. It works though, so unless something changes, we're going to assume it's trimmer-compatible.")] [Test] +#if __MACOS__ + [DynamicDependency ("Invoke", "ObjCRuntime.Trampolines.SDPHLivePhotoFrameProcessingBlock", "Microsoft.macOS")] +#elif __TVOS__ + [DynamicDependency ("Invoke", "ObjCRuntime.Trampolines.SDPHLivePhotoFrameProcessingBlock", "Microsoft.tvOS")] +#elif __MACCATALYST__ + [DynamicDependency ("Invoke", "ObjCRuntime.Trampolines.SDPHLivePhotoFrameProcessingBlock", "Microsoft.MacCatalyst")] +#elif __IOS__ + [DynamicDependency ("Invoke", "ObjCRuntime.Trampolines.SDPHLivePhotoFrameProcessingBlock", "Microsoft.iOS")] +#else +#error Unknown platform +#endif public unsafe void FrameProcessingBlock2 () { if (!Runtime.DynamicRegistrationSupported) From fb0023a2aa7ab50f717bdaf1021fa843d7b0582e Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 29 May 2025 19:51:19 +0200 Subject: [PATCH 08/15] A few more test variations. --- tests/common/shared-dotnet.csproj | 16 ++++++++-------- tests/common/shared-dotnet.mk | 2 +- tests/common/test-variations.csproj | 20 +++++++++++++++++++- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/tests/common/shared-dotnet.csproj b/tests/common/shared-dotnet.csproj index 5046f15f6b03..6029147e1e16 100644 --- a/tests/common/shared-dotnet.csproj +++ b/tests/common/shared-dotnet.csproj @@ -61,14 +61,6 @@ $(CustomBeforeMicrosoftCommonTargets);$(MSBuildThisFileDirectory)SupportedOSPlatformVersions.targets - - - - $(DefineConstants);NATIVEAOT - - false - - + + + $(DefineConstants);NATIVEAOT + + false + +