From 453d701f63d577e86d9cf779c5e9099290099dbb Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 19 Jun 2023 12:33:43 +0200 Subject: [PATCH 01/17] MSR: Construct NSObjects and INativeObjects without reflection --- Makefile | 2 +- src/Foundation/NSObject2.cs | 12 + src/ObjCRuntime/IManagedRegistrar.cs | 5 +- src/ObjCRuntime/INativeObject.cs | 8 + src/ObjCRuntime/RegistrarHelper.cs | 16 ++ src/ObjCRuntime/Runtime.cs | 120 +++++++- tools/common/StaticRegistrar.cs | 3 + tools/dotnet-linker/AppBundleRewriter.cs | 58 ++++ .../Steps/ManagedRegistrarLookupTablesStep.cs | 264 +++++++++++++++++- .../Steps/ManagedRegistrarStep.cs | 8 +- 10 files changed, 477 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 44ef689a537f..2517b4b86665 100644 --- a/Makefile +++ b/Makefile @@ -180,7 +180,7 @@ git-clean-all: @git submodule foreach -q --recursive 'git clean -xffdq && git reset --hard -q' @for dir in $(DEPENDENCY_DIRECTORIES); do if test -d $(CURDIR)/$$dir; then echo "Cleaning $$dir" && cd $(CURDIR)/$$dir && git clean -xffdq && git reset --hard -q && git submodule foreach -q --recursive 'git clean -xffdq'; else echo "Skipped $$dir (does not exist)"; fi; done - @if [ -n "$(ENABLE_XAMARIN)" ] || [ -n "$(ENABLE_DOTNET)"]; then \ + @if [ -n "$(ENABLE_XAMARIN)" ] || [ -n "$(ENABLE_DOTNET)" ]; then \ CONFIGURE_FLAGS=""; \ if [ -n "$(ENABLE_XAMARIN)" ]; then \ echo "Xamarin-specific build has been re-enabled"; \ diff --git a/src/Foundation/NSObject2.cs b/src/Foundation/NSObject2.cs index f88f1a0492c2..07724f85c92f 100644 --- a/src/Foundation/NSObject2.cs +++ b/src/Foundation/NSObject2.cs @@ -69,6 +69,15 @@ public class NSObjectFlag { } #endif +#if NET + public interface INSObjectFactory { + // The method will be implemented via custom linker step if the managed static registrar is used + // for NSObject subclasses which have an (NativeHandle) or (IntPtr) constructor. + [MethodImpl(MethodImplOptions.NoInlining)] + virtual static NSObject ConstructNSObject (NativeHandle handle) => null; + } +#endif + #if NET && !COREBUILD [ObjectiveCTrackedType] [SupportedOSPlatform ("ios")] @@ -81,6 +90,9 @@ public partial class NSObject : INativeObject #if !COREBUILD , IEquatable , IDisposable +#endif +#if NET + , INSObjectFactory #endif { #if !COREBUILD diff --git a/src/ObjCRuntime/IManagedRegistrar.cs b/src/ObjCRuntime/IManagedRegistrar.cs index 1c2c370d78f5..426fdf9563e1 100644 --- a/src/ObjCRuntime/IManagedRegistrar.cs +++ b/src/ObjCRuntime/IManagedRegistrar.cs @@ -6,7 +6,6 @@ // // Copyright 2023 Microsoft Corp - #if NET #nullable enable @@ -34,6 +33,10 @@ interface IManagedRegistrar { // This method will be called once per assembly, and the implementation has to // add all the interface -> wrapper type mappings to the dictionary. void RegisterWrapperTypes (Dictionary type); + // Create an instance of a managed NSObject subclass for an existing Objective-C object. + INativeObject? ConstructNSObject (RuntimeTypeHandle typeHandle, NativeHandle nativeHandle); + // Create an instance of a managed NSObject subclass for an existing Objective-C object. + INativeObject? ConstructINativeObject (RuntimeTypeHandle typeHandle, NativeHandle nativeHandle, bool owns); } } diff --git a/src/ObjCRuntime/INativeObject.cs b/src/ObjCRuntime/INativeObject.cs index 022b1ac868b5..c58d40a63476 100644 --- a/src/ObjCRuntime/INativeObject.cs +++ b/src/ObjCRuntime/INativeObject.cs @@ -1,6 +1,7 @@ #nullable enable using System; +using System.Runtime.CompilerServices; using Foundation; #if !NET @@ -15,6 +16,13 @@ NativeHandle Handle { get; } #endif + +#if NET + // The method will be implemented via custom linker step if the managed static registrar is used + // for classes which have an (NativeHandle, bool) or (IntPtr, bool) constructor. + [MethodImpl(MethodImplOptions.NoInlining)] + static virtual INativeObject? ConstructINativeObject (NativeHandle handle, bool owns) => null; +#endif } #if !COREBUILD diff --git a/src/ObjCRuntime/RegistrarHelper.cs b/src/ObjCRuntime/RegistrarHelper.cs index 6b764a7dfe1f..a790af23ec24 100644 --- a/src/ObjCRuntime/RegistrarHelper.cs +++ b/src/ObjCRuntime/RegistrarHelper.cs @@ -208,6 +208,22 @@ internal static uint LookupRegisteredTypeId (Type type) return entry.Registrar.LookupTypeId (type.TypeHandle); } + internal static T? ConstructNSObject (Type type, NativeHandle nativeHandle) + where T : class, INativeObject + { + if (!TryGetMapEntry (type.Assembly.GetName ().Name!, out var entry)) + return null; + return (T?) entry.Registrar.ConstructNSObject (type.TypeHandle, nativeHandle); + } + + internal static T? ConstructINativeObject (Type type, NativeHandle nativeHandle, bool owns) + where T : class, INativeObject + { + if (!TryGetMapEntry (type.Assembly.GetName ().Name!, out var entry)) + return null; + return (T?) entry.Registrar.ConstructINativeObject (type.TypeHandle, nativeHandle, owns); + } + // helper functions for converting between native and managed objects static NativeHandle ManagedArrayToNSArray (object array, bool retain) { diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index f411a851b93e..2b6cee3e30ac 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1304,19 +1304,46 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut return ConstructNSObject (ptr, typeof (T), MissingCtorResolution.ThrowConstructor1NotFound); } - // The generic argument T is only used to cast the return value. // The 'selector' and 'method' arguments are only used in error messages. - static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution) where T : class, INativeObject + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution) where T : NSObject { return ConstructNSObject (ptr, type, missingCtorResolution, IntPtr.Zero, default (RuntimeMethodHandle)); } - // The generic argument T is only used to cast the return value. // The 'selector' and 'method' arguments are only used in error messages. - static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T : class, INativeObject + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) + where T : NSObject +#if NET + , INSObjectFactory +#endif { if (type is null) throw new ArgumentNullException (nameof (type)); +#if NET + if (Runtime.IsManagedStaticRegistrar) { + T? instance = default; + var nativeHandle = new NativeHandle (ptr); + + if (typeof (T) != typeof (NSObject) + && (typeof (T) == type || typeof (T).IsGenericType) + && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { + instance = ConstructNSObjectViaFactoryMethod (nativeHandle); + } + + // If we couldn't create an instance of T through the factory method, we'll use the lookup table. + // This table can't instantiate generic types though. + if (!type.IsGenericType) { + instance ??= RegistrarHelper.ConstructNSObject (type, nativeHandle); + } + + if (instance is null) { + MissingCtor (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); + return null; + } + + return instance; + } +#endif var ctor = GetIntPtrConstructor (type); @@ -1337,6 +1364,22 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut #endif return (T) ctor.Invoke (ctorArguments); + +#if NET + static T? ConstructNSObjectViaFactoryMethod (NativeHandle handle) + { + NSObject? obj = T.ConstructNSObject (handle); + if (obj is T instance) { + return instance; + } + + // if the factory method returns a NSObject subclass but we don't expect that specific type, + // we need to dispose it and return null anyway + obj?.Dispose (); + + return null; + } +#endif } // The generic argument T is only used to cast the return value. @@ -1348,6 +1391,50 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut if (type.IsByRef) type = type.GetElementType ()!; +#if NET + if (Runtime.IsManagedStaticRegistrar) { + var nativeHandle = new NativeHandle (ptr); + T? instance = null; + + // - The factory method on T is only useful if we know the exact generic type (it's not INativeObject) + // and we're not just falling back to the INativeObject/NSObject factory method (which would throw + // an exception anyway). + // - If `T` is different from `type`, we'd rather use the lookup table to create the instance of the + // more specialized `type` than "just" T unless it's a generic type. We can't create instances of generic + // types through the lookup table. + // - It doesn't make sense to create an instance of an interface or an abstract class. + if (typeof (T) != typeof (INativeObject) + && typeof (T) != typeof (NSObject) + && (typeof (T) == type || typeof (T).IsGenericType) + && !(typeof (T).IsInterface || typeof (T).IsAbstract)) + { + instance = ConstructINativeObjectViaFactoryMethod (nativeHandle, owns); + } + + // if the factory didn't work and the type isn't generic, we can try the lookup table + if (instance is null && !type.IsGenericType) { + // if type is an NSObject, we prefer the NSObject lookup table + if (type != typeof (NSObject) && type.IsSubclassOf (typeof (NSObject))) { + instance = (T?)(INativeObject?) RegistrarHelper.ConstructNSObject (type, nativeHandle); + if (instance is not null && owns) { + Runtime.TryReleaseINativeObject (instance); + } + } + + if (instance is null && type != typeof (INativeObject)) { + instance = RegistrarHelper.ConstructINativeObject (type, nativeHandle, owns); + } + } + + if (instance is null) { + MissingCtor (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); + return null; + } + + return instance; + } +#endif + var ctor = GetIntPtr_BoolConstructor (type); if (ctor is null) { @@ -1368,6 +1455,25 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut ctorArguments [1] = owns; return (T?) ctor.Invoke (ctorArguments); + +#if NET + // This is a workaround for a compiler issue. + static T? ConstructINativeObjectViaFactoryMethod (NativeHandle nativeHandle, bool owns) + { + INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); + if (obj is T instance) { + return instance; + } + + // if the factory method returns an INativeObject but we don't expect that specific type, + // we need to release it and return null anyway + if (obj is not null) { + Runtime.TryReleaseINativeObject(obj); + } + + return null; + } +#endif } static IntPtr CreateNSObject (IntPtr type_gchandle, IntPtr handle, NSObject.Flags flags) @@ -1747,7 +1853,7 @@ static Type LookupINativeObjectImplementation (IntPtr ptr, Type target_type, Typ // native objects and NSObject instances. throw ErrorHelper.CreateError (8004, $"Cannot create an instance of {implementation.FullName} for the native object 0x{ptr:x} (of type '{Class.class_getName (Class.GetClassForObject (ptr))}'), because another instance already exists for this native object (of type {o.GetType ().FullName})."); } - return ConstructNSObject (ptr, implementation, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); + return (INativeObject?) ConstructNSObject (ptr, implementation!, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); } return ConstructINativeObject (ptr, owns, implementation, MissingCtorResolution.ThrowConstructor2NotFound, sel, method_handle); @@ -1804,10 +1910,6 @@ static Type LookupINativeObjectImplementation (IntPtr ptr, Type target_type, Typ // native objects and NSObject instances. throw ErrorHelper.CreateError (8004, $"Cannot create an instance of {implementation.FullName} for the native object 0x{ptr:x} (of type '{Class.class_getName (Class.GetClassForObject (ptr))}'), because another instance already exists for this native object (of type {o.GetType ().FullName})."); } - var rv = (T?) ConstructNSObject (ptr, implementation, MissingCtorResolution.ThrowConstructor1NotFound); - if (owns) - TryReleaseINativeObject (rv); - return rv; } return ConstructINativeObject (ptr, owns, implementation, MissingCtorResolution.ThrowConstructor2NotFound, sel, method_handle); diff --git a/tools/common/StaticRegistrar.cs b/tools/common/StaticRegistrar.cs index 47d8172de8a7..84df24e3b900 100644 --- a/tools/common/StaticRegistrar.cs +++ b/tools/common/StaticRegistrar.cs @@ -839,6 +839,9 @@ protected override bool ContainsPlatformReference (AssemblyDefinition assembly) } protected override IEnumerable CollectTypes (AssemblyDefinition assembly) + => GetAllTypes (assembly); + + internal static IEnumerable GetAllTypes (AssemblyDefinition assembly) { var queue = new Queue (); diff --git a/tools/dotnet-linker/AppBundleRewriter.cs b/tools/dotnet-linker/AppBundleRewriter.cs index edd7dc2ace5c..0dfa9320f6b1 100644 --- a/tools/dotnet-linker/AppBundleRewriter.cs +++ b/tools/dotnet-linker/AppBundleRewriter.cs @@ -339,6 +339,12 @@ public TypeReference ObjCRuntime_IManagedRegistrar { } } + public TypeReference Foundation_INSObjectFactory { + get { + return GetTypeReference (PlatformAssembly, "Foundation.INSObjectFactory", out var _); + } + } + public TypeReference ObjCRuntime_INativeObject { get { return GetTypeReference (PlatformAssembly, "ObjCRuntime.INativeObject", out var _); @@ -716,6 +722,49 @@ public MethodReference IManagedRegistrar_LookupTypeId { } } + public MethodReference IManagedRegistrar_ConstructNSObject { + get { + return GetMethodReference (PlatformAssembly, + ObjCRuntime_IManagedRegistrar, "ConstructNSObject", + isStatic: false, + System_RuntimeTypeHandle, + ObjCRuntime_NativeHandle); + } + } + + public MethodReference INSObjectFactory_ConstructNSObject { + get { + return GetMethodReference (PlatformAssembly, + Foundation_INSObjectFactory, "ConstructNSObject", + nameof (INSObjectFactory_ConstructNSObject), + isStatic: true, + ObjCRuntime_NativeHandle); + } + } + + public MethodReference IManagedRegistrar_ConstructINativeObject { + get { + return GetMethodReference (PlatformAssembly, + ObjCRuntime_IManagedRegistrar, "ConstructINativeObject", + nameof (IManagedRegistrar_ConstructINativeObject), + isStatic: false, + System_RuntimeTypeHandle, + ObjCRuntime_NativeHandle, + System_Boolean); + } + } + + public MethodReference INativeObject_ConstructINativeObject { + get { + return GetMethodReference (PlatformAssembly, + ObjCRuntime_INativeObject, "ConstructINativeObject", + nameof (INativeObject_ConstructINativeObject), + isStatic: true, + ObjCRuntime_NativeHandle, + System_Boolean); + } + } + public MethodReference IManagedRegistrar_RegisterWrapperTypes { get { return GetMethodReference (PlatformAssembly, ObjCRuntime_IManagedRegistrar, "RegisterWrapperTypes", (v) => @@ -1058,6 +1107,15 @@ public MethodReference Runtime_RetainAndAutoreleaseNativeObject { } } + public MethodReference Runtime_TryReleaseINativeObject { + get { + return GetMethodReference (PlatformAssembly, + ObjCRuntime_Runtime, "TryReleaseINativeObject", + isStatic: true, + ObjCRuntime_INativeObject); + } + } + public MethodReference UnmanagedCallersOnlyAttribute_Constructor { get { return GetMethodReference (CorlibAssembly, "System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute", ".ctor", (v) => v.IsDefaultConstructor ()); diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 28dc1c22f233..45173ec5e94e 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; @@ -123,6 +124,8 @@ void CreateRegistrarType (AssemblyTrampolineInfo info) GenerateLookupType (info, registrarType, types); GenerateLookupTypeId (info, registrarType, types); GenerateRegisterWrapperTypes (registrarType); + GenerateConstructNSObject (registrarType); + GenerateConstructINativeObject (registrarType); // Make sure the linker doesn't sweep away anything we just generated. Annotations.Mark (registrarType); @@ -201,6 +204,13 @@ List GetTypesToRegister (TypeDefinition registrarType, AssemblyTrampol return types; } + IEnumerable GetRelevantTypes (Func isRelevant) + => StaticRegistrar.GetAllTypes (abr.CurrentAssembly) + .Cast () + .Where (type => !IsTrimmed (type)) + .Where (type => type.Module.Assembly == abr.CurrentAssembly) + .Where (isRelevant); + bool IsTrimmed (MemberReference type) { return StaticRegistrar.IsTrimmed (type, Annotations); @@ -281,6 +291,233 @@ void GenerateLookupType (AssemblyTrampolineInfo infos, TypeDefinition registrarT il.Emit (OpCodes.Ret); } + void GenerateConstructNSObject (TypeDefinition registrarType) + { + var createInstanceMethod = registrarType.AddMethod ("ConstructNSObject", MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.Virtual | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); + var typeHandleParameter = createInstanceMethod.AddParameter ("typeHandle", abr.System_RuntimeTypeHandle); + var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); + createInstanceMethod.Overrides.Add (abr.IManagedRegistrar_ConstructNSObject); + var body = createInstanceMethod.CreateBody (out var il); + + var types = GetRelevantTypes (type => type.IsNSObject (DerivedLinkContext) && !type.IsAbstract && !type.IsInterface); + + foreach (var type in types) { + var ctorRef = FindNSObjectConstructor (type); + if (ctorRef is null) { + Driver.Log (9, $"Cannot include {type.FullName} in ConstructNSObject because it doesn't have a suitable constructor"); + continue; + } + + var ctor = abr.CurrentAssembly.MainModule.ImportReference (ctorRef); + if (IsTrimmed (ctor)) + Annotations.Mark (ctor.Resolve ()); + + // We can only add a type to the table if it's not an open type. + if (!ManagedRegistrarStep.IsOpenType (type)) { + EnsureVisible (createInstanceMethod, ctor); + + il.Emit (OpCodes.Ldarga_S, typeHandleParameter); + il.Emit (OpCodes.Ldtoken, type); + il.Emit (OpCodes.Call, abr.RuntimeTypeHandle_Equals); + var falseTarget = il.Create (OpCodes.Nop); + il.Emit (OpCodes.Brfalse_S, falseTarget); + + il.Emit (OpCodes.Ldarg, nativeHandleParameter); + if (ctor.Parameters [0].ParameterType.Is ("System", "IntPtr")) + il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); + il.Emit (OpCodes.Newobj, ctor); + il.Emit (OpCodes.Ret); + + il.Append (falseTarget); + } + + // In addition to the big lookup method, implement the static factory method on the type: + ImplementConstructNSObjectFactoryMethod (type, ctor); + } + + // return default (NSObject) + var temporary = body.AddVariable (abr.Foundation_NSObject); + il.Emit (OpCodes.Ldloca, temporary); + il.Emit (OpCodes.Initobj, abr.Foundation_NSObject); + il.Emit (OpCodes.Ldloc, temporary); + il.Emit (OpCodes.Ret); + } + + void GenerateConstructINativeObject (TypeDefinition registrarType) + { + var createInstanceMethod = registrarType.AddMethod ("ConstructINativeObject", MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.Virtual | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); + var typeHandleParameter = createInstanceMethod.AddParameter ("typeHandle", abr.System_RuntimeTypeHandle); + var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); + var ownsParameter = createInstanceMethod.AddParameter ("owns", abr.System_Boolean); + createInstanceMethod.Overrides.Add (abr.IManagedRegistrar_ConstructINativeObject); + var body = createInstanceMethod.CreateBody (out var il); + + // We generate something like this: + // if (RuntimeTypeHandle.Equals (typeof (TypeA).TypeHandle)) + // return new TypeA (nativeHandle, owns); + // if (RuntimeTypeHandle.Equals (typeof (TypeB).TypeHandle)) + // return new TypeB (nativeHandle, owns); + // return null; + + var types = GetRelevantTypes (type => type.IsNativeObject () && !type.IsAbstract && !type.IsInterface); + + foreach (var type in types) { + var ctorRef = FindINativeObjectConstructor (type); + + if (ctorRef is not null) { + 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 (!ManagedRegistrarStep.IsOpenType (type)) { + EnsureVisible (createInstanceMethod, ctor); + + il.Emit (OpCodes.Ldarga_S, typeHandleParameter); + il.Emit (OpCodes.Ldtoken, type); + il.Emit (OpCodes.Call, abr.RuntimeTypeHandle_Equals); + var falseTarget = il.Create (OpCodes.Nop); + il.Emit (OpCodes.Brfalse_S, falseTarget); + + il.Emit (OpCodes.Ldarg, nativeHandleParameter); + if (ctor.Parameters [0].ParameterType.Is ("System", "IntPtr")) + il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); + il.Emit (OpCodes.Ldarg, ownsParameter); + il.Emit (OpCodes.Newobj, ctor); + il.Emit (OpCodes.Ret); + + il.Append (falseTarget); + } + } + + // In addition to the big lookup method, implement the static factory method on the type: + ImplementConstructINativeObjectFactoryMethod (type, ctorRef); + } + + // return default (NSObject) + var temporary = body.AddVariable (abr.Foundation_NSObject); + il.Emit (OpCodes.Ldloca, temporary); + il.Emit (OpCodes.Initobj, abr.Foundation_NSObject); + il.Emit (OpCodes.Ldloc, temporary); + il.Emit (OpCodes.Ret); + } + + void ImplementConstructNSObjectFactoryMethod (TypeDefinition type, MethodReference ctor) + { + // skip creating the factory for NSObject itself + if (type.Is ("Foundation", "NSObject")) + return; + + var createInstanceMethod = type.AddMethod ("ConstructNSObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.Foundation_NSObject); + var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); + createInstanceMethod.Overrides.Add (abr.INSObjectFactory_ConstructNSObject); + var body = createInstanceMethod.CreateBody (out var il); + + if (type.HasGenericParameters) { + ctor = type.CreateMethodReferenceOnGenericType(ctor, type.GenericParameters.ToArray ()); + } + + // return new TypeA (nativeHandle); // for NativeHandle ctor + // return new TypeA ((IntPtr) nativeHandle); // for IntPtr ctor + il.Emit (OpCodes.Ldarg, nativeHandleParameter); + if (ctor.Parameters [0].ParameterType.Is ("System", "IntPtr")) + il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); + il.Emit (OpCodes.Newobj, ctor); + il.Emit (OpCodes.Ret); + + Annotations.Mark (createInstanceMethod); + } + + void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodReference? ctor) + { + // skip creating the factory for NSObject itself + if (type.Is ("Foundation", "NSObject")) + return; + + // If the type is a subclass of NSObject, we prefer the NSObject "IntPtr" constructor + var nsobjectConstructor = type.IsNSObject (DerivedLinkContext) ? FindNSObjectConstructor (type) : null; + if (nsobjectConstructor is null && ctor is null) + return; + + var createInstanceMethod = type.AddMethod ("ConstructINativeObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); + var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); + var ownsParameter = createInstanceMethod.AddParameter ("owns", abr.System_Boolean); + createInstanceMethod.Overrides.Add (abr.INativeObject_ConstructINativeObject); + var body = createInstanceMethod.CreateBody (out var il); + + if (nsobjectConstructor is not null) { + // var instance = new TypeA (nativeHandle); + // // alternatively with a cast: new TypeA ((IntPtr) nativeHandle); + // if (instance is not null && owns) + // Runtime.TryReleaseINativeObject (instance); + // return instance; + + if (type.HasGenericParameters) { + nsobjectConstructor = type.CreateMethodReferenceOnGenericType(nsobjectConstructor, type.GenericParameters.ToArray ()); + } + + var instanceVariable = body.AddVariable (abr.Foundation_NSObject); + il.Emit (OpCodes.Ldarg, nativeHandleParameter); + if (nsobjectConstructor.Parameters [0].ParameterType.Is ("System", "IntPtr")) + il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); + il.Emit (OpCodes.Newobj, nsobjectConstructor); + il.Emit (OpCodes.Stloc, instanceVariable); + + var falseTarget = il.Create (OpCodes.Nop); + il.Emit (OpCodes.Ldloc, instanceVariable); + il.Emit (OpCodes.Ldnull); + il.Emit (OpCodes.Cgt_Un); + il.Emit (OpCodes.Ldarg, ownsParameter); + il.Emit (OpCodes.And); + il.Emit (OpCodes.Brfalse_S, falseTarget); + + il.Emit (OpCodes.Ldloc, instanceVariable); + il.Emit (OpCodes.Call, abr.Runtime_TryReleaseINativeObject); + + il.Append (falseTarget); + + il.Emit (OpCodes.Ldloc, instanceVariable); + il.Emit (OpCodes.Ret); + } else if (ctor is not null) { + // return new TypeA (nativeHandle, owns); // for NativeHandle ctor + // return new TypeA ((IntPtr) nativeHandle, owns); // IntPtr ctor + + if (type.HasGenericParameters) { + ctor = type.CreateMethodReferenceOnGenericType(ctor, type.GenericParameters.ToArray ()); + } + + il.Emit (OpCodes.Ldarg, nativeHandleParameter); + if (ctor.Parameters [0].ParameterType.Is ("System", "IntPtr")) + il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); + il.Emit (OpCodes.Ldarg, ownsParameter); + il.Emit (OpCodes.Newobj, ctor); + il.Emit (OpCodes.Ret); + } else { + throw new UnreachableException (); + } + + Annotations.Mark (createInstanceMethod); + } + + static MethodReference? FindNSObjectConstructor (TypeDefinition type) + => FindConstructorByParameterTypes (type, ("ObjCRuntime", "NativeHandle")) + ?? FindConstructorByParameterTypes (type, ("System", "IntPtr")); + + static MethodReference? FindINativeObjectConstructor (TypeDefinition type) + => FindConstructorByParameterTypes (type, ("ObjCRuntime", "NativeHandle"), ("System", "Boolean")) + ?? FindConstructorByParameterTypes (type, ("System", "IntPtr"), ("System", "Boolean")); + + static MethodReference? FindConstructorByParameterTypes (TypeDefinition type, params (string Namespace, string Class)[] requiredParameters) + => type.Methods.FirstOrDefault (method => method.IsConstructor + && !method.IsStatic + && method.HasParameters + && method.Parameters.Count == requiredParameters.Length + && method.Parameters.Zip (requiredParameters).All (pair => { + var (parameter, requiredParameter) = pair; + return parameter.ParameterType.Is (requiredParameter.Namespace, requiredParameter.Class); + })); + void GenerateRegisterWrapperTypes (TypeDefinition type) { var method = type.AddMethod ("RegisterWrapperTypes", MethodAttributes.Private | MethodAttributes.Final | MethodAttributes.Virtual | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.System_Void); @@ -473,13 +710,32 @@ static string GetMethodSignature (MethodDefinition method) return $"{method?.ReturnType?.FullName ?? "(null)"} {method?.DeclaringType?.FullName ?? "(null)"}::{method?.Name ?? "(null)"} ({string.Join (", ", method?.Parameters?.Select (v => v?.ParameterType?.FullName + " " + v?.Name) ?? Array.Empty ())})"; } - void EnsureVisible (MethodDefinition caller, TypeDefinition type) + static void EnsureVisible (MethodDefinition caller, MethodReference methodRef) { + var method = methodRef.Resolve (); + var type = method.DeclaringType.Resolve (); if (type.IsNested) { - type.IsNestedPublic = true; + if (!method.IsPublic) { + method.IsFamilyOrAssembly = true; + } + + EnsureVisible (caller, type); + } else if (!method.IsPublic) { + method.IsFamilyOrAssembly = true; + } + } + + static void EnsureVisible (MethodDefinition caller, TypeReference typeRef) + { + var type = typeRef.Resolve (); + if (type.IsNested) { + if (!type.IsNestedPublic) { + type.IsNestedAssembly = true; + } + EnsureVisible (caller, type.DeclaringType); - } else { - type.IsPublic = true; + } else if (!type.IsPublic) { + type.IsNotPublic = true; } } diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarStep.cs index 33ff8b81e2dc..7daaf0c181da 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarStep.cs @@ -968,7 +968,7 @@ bool EmitConversion (MethodDefinition method, ILProcessor il, TypeReference type return false; } - bool IsOpenType (TypeReference tr) + internal static bool IsOpenType (TypeReference tr) { if (tr is GenericParameter) return true; @@ -990,13 +990,13 @@ bool IsOpenType (TypeReference tr) return IsOpenType (tr.Resolve ()); } - void EnsureVisible (MethodDefinition caller, FieldDefinition field) + static void EnsureVisible (MethodDefinition caller, FieldDefinition field) { field.IsPublic = true; EnsureVisible (caller, field.DeclaringType); } - void EnsureVisible (MethodDefinition caller, TypeDefinition type) + static void EnsureVisible (MethodDefinition caller, TypeDefinition type) { if (type.IsNested) { type.IsNestedPublic = true; @@ -1006,7 +1006,7 @@ void EnsureVisible (MethodDefinition caller, TypeDefinition type) } } - void EnsureVisible (MethodDefinition caller, MethodReference method) + static void EnsureVisible (MethodDefinition caller, MethodReference method) { var md = method.Resolve (); md.IsPublic = true; From 9f124951119fd47787b34cb10a947f249477e281 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Thu, 29 Jun 2023 14:23:24 +0000 Subject: [PATCH 02/17] Auto-format source code --- .../Steps/ManagedRegistrarLookupTablesStep.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 45173ec5e94e..b804921010b2 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -330,7 +330,7 @@ void GenerateConstructNSObject (TypeDefinition registrarType) il.Append (falseTarget); } - + // In addition to the big lookup method, implement the static factory method on the type: ImplementConstructNSObjectFactoryMethod (type, ctor); } @@ -415,7 +415,7 @@ void ImplementConstructNSObjectFactoryMethod (TypeDefinition type, MethodReferen var body = createInstanceMethod.CreateBody (out var il); if (type.HasGenericParameters) { - ctor = type.CreateMethodReferenceOnGenericType(ctor, type.GenericParameters.ToArray ()); + ctor = type.CreateMethodReferenceOnGenericType (ctor, type.GenericParameters.ToArray ()); } // return new TypeA (nativeHandle); // for NativeHandle ctor @@ -454,7 +454,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe // return instance; if (type.HasGenericParameters) { - nsobjectConstructor = type.CreateMethodReferenceOnGenericType(nsobjectConstructor, type.GenericParameters.ToArray ()); + nsobjectConstructor = type.CreateMethodReferenceOnGenericType (nsobjectConstructor, type.GenericParameters.ToArray ()); } var instanceVariable = body.AddVariable (abr.Foundation_NSObject); @@ -484,7 +484,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe // return new TypeA ((IntPtr) nativeHandle, owns); // IntPtr ctor if (type.HasGenericParameters) { - ctor = type.CreateMethodReferenceOnGenericType(ctor, type.GenericParameters.ToArray ()); + ctor = type.CreateMethodReferenceOnGenericType (ctor, type.GenericParameters.ToArray ()); } il.Emit (OpCodes.Ldarg, nativeHandleParameter); @@ -508,7 +508,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe => FindConstructorByParameterTypes (type, ("ObjCRuntime", "NativeHandle"), ("System", "Boolean")) ?? FindConstructorByParameterTypes (type, ("System", "IntPtr"), ("System", "Boolean")); - static MethodReference? FindConstructorByParameterTypes (TypeDefinition type, params (string Namespace, string Class)[] requiredParameters) + static MethodReference? FindConstructorByParameterTypes (TypeDefinition type, params (string Namespace, string Class) [] requiredParameters) => type.Methods.FirstOrDefault (method => method.IsConstructor && !method.IsStatic && method.HasParameters @@ -724,7 +724,7 @@ static void EnsureVisible (MethodDefinition caller, MethodReference methodRef) method.IsFamilyOrAssembly = true; } } - + static void EnsureVisible (MethodDefinition caller, TypeReference typeRef) { var type = typeRef.Resolve (); From 4b3126c31719b07c66750b7e6bc9179176563710 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 15:27:18 +0200 Subject: [PATCH 03/17] Fix comments --- .../Steps/ManagedRegistrarLookupTablesStep.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index b804921010b2..af149e15fb4e 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -299,6 +299,13 @@ void GenerateConstructNSObject (TypeDefinition registrarType) createInstanceMethod.Overrides.Add (abr.IManagedRegistrar_ConstructNSObject); var body = createInstanceMethod.CreateBody (out var il); + // We generate something like this: + // if (RuntimeTypeHandle.Equals (typeHandle, typeof (TypeA).TypeHandle)) + // return new TypeA (nativeHandle); + // if (RuntimeTypeHandle.Equals (typeHandle, typeof (TypeB).TypeHandle)) + // return new TypeB (nativeHandle); + // return null; + var types = GetRelevantTypes (type => type.IsNSObject (DerivedLinkContext) && !type.IsAbstract && !type.IsInterface); foreach (var type in types) { @@ -353,9 +360,9 @@ void GenerateConstructINativeObject (TypeDefinition registrarType) var body = createInstanceMethod.CreateBody (out var il); // We generate something like this: - // if (RuntimeTypeHandle.Equals (typeof (TypeA).TypeHandle)) + // if (RuntimeTypeHandle.Equals (typeHandle, typeof (TypeA).TypeHandle)) // return new TypeA (nativeHandle, owns); - // if (RuntimeTypeHandle.Equals (typeof (TypeB).TypeHandle)) + // if (RuntimeTypeHandle.Equals (typeHandle, typeof (TypeB).TypeHandle)) // return new TypeB (nativeHandle, owns); // return null; From 35fe42e55b4a9240bee907062a2442c8c6de4daa Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 16:50:35 +0200 Subject: [PATCH 04/17] Improve code based on reviews --- src/ObjCRuntime/Runtime.cs | 61 +++++++------------ .../Steps/ManagedRegistrarLookupTablesStep.cs | 44 +++++++------ 2 files changed, 48 insertions(+), 57 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 2b6cee3e30ac..6668de5dbc10 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1311,10 +1311,10 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut } // The 'selector' and 'method' arguments are only used in error messages. - static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) - where T : NSObject #if NET - , INSObjectFactory + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T: NSObject, INSObjectFactory +#else + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T: NSObject #endif { if (type is null) @@ -1327,7 +1327,16 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut if (typeof (T) != typeof (NSObject) && (typeof (T) == type || typeof (T).IsGenericType) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { - instance = ConstructNSObjectViaFactoryMethod (nativeHandle); + // instance = ConstructNSObjectViaFactoryMethod (nativeHandle); + + NSObject? obj = T.ConstructNSObject (nativeHandle); + if (obj is T t) { + instance = t; + } else if (obj is not null) { + // if the factory method returns a NSObject subclass but we expect a different type, + // we need to dispose the object we got and ignore it + obj.Dispose (); + } } // If we couldn't create an instance of T through the factory method, we'll use the lookup table. @@ -1364,22 +1373,6 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut #endif return (T) ctor.Invoke (ctorArguments); - -#if NET - static T? ConstructNSObjectViaFactoryMethod (NativeHandle handle) - { - NSObject? obj = T.ConstructNSObject (handle); - if (obj is T instance) { - return instance; - } - - // if the factory method returns a NSObject subclass but we don't expect that specific type, - // we need to dispose it and return null anyway - obj?.Dispose (); - - return null; - } -#endif } // The generic argument T is only used to cast the return value. @@ -1408,7 +1401,14 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut && (typeof (T) == type || typeof (T).IsGenericType) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { - instance = ConstructINativeObjectViaFactoryMethod (nativeHandle, owns); + INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); + if (obj is T t) { + instance = t; + } else if (obj is not null) { + // if the factory method returns an INativeObject but we expected a different type, + // we need to release it and ignore it + Runtime.TryReleaseINativeObject (obj); + } } // if the factory didn't work and the type isn't generic, we can try the lookup table @@ -1455,25 +1455,6 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut ctorArguments [1] = owns; return (T?) ctor.Invoke (ctorArguments); - -#if NET - // This is a workaround for a compiler issue. - static T? ConstructINativeObjectViaFactoryMethod (NativeHandle nativeHandle, bool owns) - { - INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); - if (obj is T instance) { - return instance; - } - - // if the factory method returns an INativeObject but we don't expect that specific type, - // we need to release it and return null anyway - if (obj is not null) { - Runtime.TryReleaseINativeObject(obj); - } - - return null; - } -#endif } static IntPtr CreateNSObject (IntPtr type_gchandle, IntPtr handle, NSObject.Flags flags) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index af149e15fb4e..c9c7c5049ad0 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -507,23 +507,33 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe Annotations.Mark (createInstanceMethod); } - static MethodReference? FindNSObjectConstructor (TypeDefinition type) - => FindConstructorByParameterTypes (type, ("ObjCRuntime", "NativeHandle")) - ?? FindConstructorByParameterTypes (type, ("System", "IntPtr")); - - static MethodReference? FindINativeObjectConstructor (TypeDefinition type) - => FindConstructorByParameterTypes (type, ("ObjCRuntime", "NativeHandle"), ("System", "Boolean")) - ?? FindConstructorByParameterTypes (type, ("System", "IntPtr"), ("System", "Boolean")); - - static MethodReference? FindConstructorByParameterTypes (TypeDefinition type, params (string Namespace, string Class) [] requiredParameters) - => type.Methods.FirstOrDefault (method => method.IsConstructor - && !method.IsStatic - && method.HasParameters - && method.Parameters.Count == requiredParameters.Length - && method.Parameters.Zip (requiredParameters).All (pair => { - var (parameter, requiredParameter) = pair; - return parameter.ParameterType.Is (requiredParameter.Namespace, requiredParameter.Class); - })); + static MethodReference? FindNSObjectConstructor (TypeDefinition type) { + return FindConstructorWithOneParameter ("ObjCRuntime", "NativeHandle") + ?? FindConstructorWithOneParameter ("System", "IntPtr"); + + MethodReference? FindConstructorWithOneParameter (string ns, string cls) + => type.Methods.FirstOrDefault (method => + method.IsConstructor + && !method.IsStatic + && method.HasParameters + && method.Parameters.Count == 1 + && method.Parameters [0].ParameterType.Is (ns, cls)); + } + + + static MethodReference? FindINativeObjectConstructor (TypeDefinition type) { + return FindConstructorWithTwoParameters ("ObjCRuntime", "NativeHandle", "System", "Boolean") + ?? FindConstructorWithTwoParameters ("System", "IntPtr", "System", "Boolean"); + + MethodReference? FindConstructorWithTwoParameters (string ns1, string cls1, string ns2, string cls2) + => type.Methods.FirstOrDefault (method => + method.IsConstructor + && !method.IsStatic + && method.HasParameters + && method.Parameters.Count == 2 + && method.Parameters [0].ParameterType.Is (ns1, cls1) + && method.Parameters [1].ParameterType.Is (ns2, cls2)); + } void GenerateRegisterWrapperTypes (TypeDefinition type) { From 06bc85a1f0a1461e033d830c1d6591ba18e35f0e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 18:48:15 +0200 Subject: [PATCH 05/17] Fix bug that causes problems when msr is not used --- src/ObjCRuntime/Runtime.cs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 6668de5dbc10..6e91c484e40d 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1312,9 +1312,9 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut // The 'selector' and 'method' arguments are only used in error messages. #if NET - static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T: NSObject, INSObjectFactory + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T : NSObject, INSObjectFactory #else - static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T: NSObject + static T? ConstructNSObject (IntPtr ptr, Type type, MissingCtorResolution missingCtorResolution, IntPtr sel, RuntimeMethodHandle method_handle) where T : class, INativeObject #endif { if (type is null) @@ -1891,6 +1891,22 @@ static Type LookupINativeObjectImplementation (IntPtr ptr, Type target_type, Typ // native objects and NSObject instances. throw ErrorHelper.CreateError (8004, $"Cannot create an instance of {implementation.FullName} for the native object 0x{ptr:x} (of type '{Class.class_getName (Class.GetClassForObject (ptr))}'), because another instance already exists for this native object (of type {o.GetType ().FullName})."); } +#if NET + if (!Runtime.IsManagedStaticRegistrar) { + // For other registrars other than managed-static the generic parameter of ConstructNSObject is used + // only to cast the return value so we can safely pass NSObject here to satisfy the constraints of the + // generic parameter. + var rv = (T?)(INativeObject?) ConstructNSObject (ptr, implementation, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); + if (owns) + TryReleaseINativeObject (rv); + return rv; + } +#else + var rv = ConstructNSObject (ptr, implementation, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); + if (owns) + TryReleaseINativeObject (rv); + return rv; +#endif } return ConstructINativeObject (ptr, owns, implementation, MissingCtorResolution.ThrowConstructor2NotFound, sel, method_handle); From 31b7dd2a917cccf99972563dfb03da7922e17d12 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 18:48:34 +0200 Subject: [PATCH 06/17] Bring back workaround to avoid vtable assertion crash --- src/ObjCRuntime/Runtime.cs | 66 +++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 6e91c484e40d..143eaa5aa8e8 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1327,16 +1327,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut if (typeof (T) != typeof (NSObject) && (typeof (T) == type || typeof (T).IsGenericType) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { - // instance = ConstructNSObjectViaFactoryMethod (nativeHandle); - - NSObject? obj = T.ConstructNSObject (nativeHandle); - if (obj is T t) { - instance = t; - } else if (obj is not null) { - // if the factory method returns a NSObject subclass but we expect a different type, - // we need to dispose the object we got and ignore it - obj.Dispose (); - } + instance = ConstructNSObjectViaFactoryMethod (nativeHandle); } // If we couldn't create an instance of T through the factory method, we'll use the lookup table. @@ -1373,6 +1364,28 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut #endif return (T) ctor.Invoke (ctorArguments); + +#if NET + // It isn't possible to call T.ConstructNSObject (...) directly from the parent function. For some + // types, the app crashes with a SIGSEGV: + // + // error: * Assertion at /Users/runner/work/1/s/src/mono/mono/mini/mini-generic-sharing.c:2283, condition `m_class_get_vtable (info->klass)' not met + // + // When the same call is made from a separate function, it works fine. + static T? ConstructNSObjectViaFactoryMethod (NativeHandle handle) + { + NSObject? obj = T.ConstructNSObject (handle); + if (obj is T instance) { + return instance; + } + + // if the factory method returns a NSObject subclass but we don't expect that specific type, + // we need to dispose it and return null anyway + obj?.Dispose (); + + return null; + } +#endif } // The generic argument T is only used to cast the return value. @@ -1401,14 +1414,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut && (typeof (T) == type || typeof (T).IsGenericType) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { - INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); - if (obj is T t) { - instance = t; - } else if (obj is not null) { - // if the factory method returns an INativeObject but we expected a different type, - // we need to release it and ignore it - Runtime.TryReleaseINativeObject (obj); - } + instance = ConstructINativeObjectViaFactoryMethod (nativeHandle, owns); } // if the factory didn't work and the type isn't generic, we can try the lookup table @@ -1455,6 +1461,30 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut ctorArguments [1] = owns; return (T?) ctor.Invoke (ctorArguments); + +#if NET + // It isn't possible to call T.ConstructINativeObject (...) directly from the parent function. For some + // types, the app crashes with a SIGSEGV: + // + // error: * Assertion at /Users/runner/work/1/s/src/mono/mono/mini/mini-generic-sharing.c:2283, condition `m_class_get_vtable (info->klass)' not met + // + // When the same call is made from a separate function, it works fine. + static T? ConstructINativeObjectViaFactoryMethod (NativeHandle nativeHandle, bool owns) + { + INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); + if (obj is T instance) { + return instance; + } + + // if the factory method returns an INativeObject but we don't expect that specific type, + // we need to release it and return null anyway + if (obj is not null) { + Runtime.TryReleaseINativeObject(obj); + } + + return null; + } +#endif } static IntPtr CreateNSObject (IntPtr type_gchandle, IntPtr handle, NSObject.Flags flags) From 70af4e035cf47a4c6ada92782dbb6c98fa901177 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 19:00:01 +0200 Subject: [PATCH 07/17] Remove local variable --- .../Steps/ManagedRegistrarLookupTablesStep.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index c9c7c5049ad0..42d6dbc46106 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -464,27 +464,24 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe nsobjectConstructor = type.CreateMethodReferenceOnGenericType (nsobjectConstructor, type.GenericParameters.ToArray ()); } - var instanceVariable = body.AddVariable (abr.Foundation_NSObject); il.Emit (OpCodes.Ldarg, nativeHandleParameter); if (nsobjectConstructor.Parameters [0].ParameterType.Is ("System", "IntPtr")) il.Emit (OpCodes.Call, abr.NativeObject_op_Implicit_IntPtr); il.Emit (OpCodes.Newobj, nsobjectConstructor); - il.Emit (OpCodes.Stloc, instanceVariable); var falseTarget = il.Create (OpCodes.Nop); - il.Emit (OpCodes.Ldloc, instanceVariable); + il.Emit (OpCodes.Dup); il.Emit (OpCodes.Ldnull); il.Emit (OpCodes.Cgt_Un); il.Emit (OpCodes.Ldarg, ownsParameter); il.Emit (OpCodes.And); il.Emit (OpCodes.Brfalse_S, falseTarget); - il.Emit (OpCodes.Ldloc, instanceVariable); + il.Emit (OpCodes.Dup); il.Emit (OpCodes.Call, abr.Runtime_TryReleaseINativeObject); il.Append (falseTarget); - il.Emit (OpCodes.Ldloc, instanceVariable); il.Emit (OpCodes.Ret); } else if (ctor is not null) { // return new TypeA (nativeHandle, owns); // for NativeHandle ctor From d7680e96817f1bccdda597757dbb77fa62a3498e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 30 Jun 2023 19:25:03 +0200 Subject: [PATCH 08/17] Add _Xamarin_ prefix --- src/Foundation/NSObject2.cs | 2 +- src/ObjCRuntime/INativeObject.cs | 2 +- src/ObjCRuntime/Runtime.cs | 4 ++-- tools/dotnet-linker/AppBundleRewriter.cs | 12 ++++++------ .../Steps/ManagedRegistrarLookupTablesStep.cs | 8 ++++---- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Foundation/NSObject2.cs b/src/Foundation/NSObject2.cs index 07724f85c92f..9279874b5ce4 100644 --- a/src/Foundation/NSObject2.cs +++ b/src/Foundation/NSObject2.cs @@ -74,7 +74,7 @@ public interface INSObjectFactory { // The method will be implemented via custom linker step if the managed static registrar is used // for NSObject subclasses which have an (NativeHandle) or (IntPtr) constructor. [MethodImpl(MethodImplOptions.NoInlining)] - virtual static NSObject ConstructNSObject (NativeHandle handle) => null; + virtual static NSObject _Xamarin_ConstructNSObject (NativeHandle handle) => null; } #endif diff --git a/src/ObjCRuntime/INativeObject.cs b/src/ObjCRuntime/INativeObject.cs index c58d40a63476..2fd4e9e6b4cc 100644 --- a/src/ObjCRuntime/INativeObject.cs +++ b/src/ObjCRuntime/INativeObject.cs @@ -21,7 +21,7 @@ NativeHandle Handle { // The method will be implemented via custom linker step if the managed static registrar is used // for classes which have an (NativeHandle, bool) or (IntPtr, bool) constructor. [MethodImpl(MethodImplOptions.NoInlining)] - static virtual INativeObject? ConstructINativeObject (NativeHandle handle, bool owns) => null; + static virtual INativeObject? _Xamarin_ConstructINativeObject (NativeHandle handle, bool owns) => null; #endif } diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 143eaa5aa8e8..e9b7057d20e6 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1374,7 +1374,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut // When the same call is made from a separate function, it works fine. static T? ConstructNSObjectViaFactoryMethod (NativeHandle handle) { - NSObject? obj = T.ConstructNSObject (handle); + NSObject? obj = T._Xamarin_ConstructNSObject (handle); if (obj is T instance) { return instance; } @@ -1471,7 +1471,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut // When the same call is made from a separate function, it works fine. static T? ConstructINativeObjectViaFactoryMethod (NativeHandle nativeHandle, bool owns) { - INativeObject? obj = T.ConstructINativeObject (nativeHandle, owns); + INativeObject? obj = T._Xamarin_ConstructINativeObject (nativeHandle, owns); if (obj is T instance) { return instance; } diff --git a/tools/dotnet-linker/AppBundleRewriter.cs b/tools/dotnet-linker/AppBundleRewriter.cs index 0dfa9320f6b1..928ce640d33f 100644 --- a/tools/dotnet-linker/AppBundleRewriter.cs +++ b/tools/dotnet-linker/AppBundleRewriter.cs @@ -732,11 +732,11 @@ public MethodReference IManagedRegistrar_ConstructNSObject { } } - public MethodReference INSObjectFactory_ConstructNSObject { + public MethodReference INSObjectFactory__Xamarin_ConstructNSObject { get { return GetMethodReference (PlatformAssembly, - Foundation_INSObjectFactory, "ConstructNSObject", - nameof (INSObjectFactory_ConstructNSObject), + Foundation_INSObjectFactory, "_Xamarin_ConstructNSObject", + nameof (INSObjectFactory__Xamarin_ConstructNSObject), isStatic: true, ObjCRuntime_NativeHandle); } @@ -754,11 +754,11 @@ public MethodReference IManagedRegistrar_ConstructINativeObject { } } - public MethodReference INativeObject_ConstructINativeObject { + public MethodReference INativeObject__Xamarin_ConstructINativeObject { get { return GetMethodReference (PlatformAssembly, - ObjCRuntime_INativeObject, "ConstructINativeObject", - nameof (INativeObject_ConstructINativeObject), + ObjCRuntime_INativeObject, "_Xamarin_ConstructINativeObject", + nameof (INativeObject__Xamarin_ConstructINativeObject), isStatic: true, ObjCRuntime_NativeHandle, System_Boolean); diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 42d6dbc46106..2aa651b01ce6 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -416,9 +416,9 @@ void ImplementConstructNSObjectFactoryMethod (TypeDefinition type, MethodReferen if (type.Is ("Foundation", "NSObject")) return; - var createInstanceMethod = type.AddMethod ("ConstructNSObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.Foundation_NSObject); + var createInstanceMethod = type.AddMethod ("_Xamarin_ConstructNSObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.Foundation_NSObject); var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); - createInstanceMethod.Overrides.Add (abr.INSObjectFactory_ConstructNSObject); + createInstanceMethod.Overrides.Add (abr.INSObjectFactory__Xamarin_ConstructNSObject); var body = createInstanceMethod.CreateBody (out var il); if (type.HasGenericParameters) { @@ -447,10 +447,10 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe if (nsobjectConstructor is null && ctor is null) return; - var createInstanceMethod = type.AddMethod ("ConstructINativeObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); + var createInstanceMethod = type.AddMethod ("_Xamarin_ConstructINativeObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); var ownsParameter = createInstanceMethod.AddParameter ("owns", abr.System_Boolean); - createInstanceMethod.Overrides.Add (abr.INativeObject_ConstructINativeObject); + createInstanceMethod.Overrides.Add (abr.INativeObject__Xamarin_ConstructINativeObject); var body = createInstanceMethod.CreateBody (out var il); if (nsobjectConstructor is not null) { From 950a3cc29962222dc3887459ba6570becfad2d09 Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Fri, 30 Jun 2023 17:41:09 +0000 Subject: [PATCH 09/17] Auto-format source code --- .../dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 2aa651b01ce6..662e9604e37a 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -504,7 +504,8 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe Annotations.Mark (createInstanceMethod); } - static MethodReference? FindNSObjectConstructor (TypeDefinition type) { + static MethodReference? FindNSObjectConstructor (TypeDefinition type) + { return FindConstructorWithOneParameter ("ObjCRuntime", "NativeHandle") ?? FindConstructorWithOneParameter ("System", "IntPtr"); @@ -518,7 +519,8 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe } - static MethodReference? FindINativeObjectConstructor (TypeDefinition type) { + static MethodReference? FindINativeObjectConstructor (TypeDefinition type) + { return FindConstructorWithTwoParameters ("ObjCRuntime", "NativeHandle", "System", "Boolean") ?? FindConstructorWithTwoParameters ("System", "IntPtr", "System", "Boolean"); From dc2f5b88bf5f40eb0e2c9a87c6f3480853ea8bdf Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Sat, 1 Jul 2023 13:46:12 +0200 Subject: [PATCH 10/17] Revert unnecessary modification to Makefile --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2517b4b86665..44ef689a537f 100644 --- a/Makefile +++ b/Makefile @@ -180,7 +180,7 @@ git-clean-all: @git submodule foreach -q --recursive 'git clean -xffdq && git reset --hard -q' @for dir in $(DEPENDENCY_DIRECTORIES); do if test -d $(CURDIR)/$$dir; then echo "Cleaning $$dir" && cd $(CURDIR)/$$dir && git clean -xffdq && git reset --hard -q && git submodule foreach -q --recursive 'git clean -xffdq'; else echo "Skipped $$dir (does not exist)"; fi; done - @if [ -n "$(ENABLE_XAMARIN)" ] || [ -n "$(ENABLE_DOTNET)" ]; then \ + @if [ -n "$(ENABLE_XAMARIN)" ] || [ -n "$(ENABLE_DOTNET)"]; then \ CONFIGURE_FLAGS=""; \ if [ -n "$(ENABLE_XAMARIN)" ]; then \ echo "Xamarin-specific build has been re-enabled"; \ From 1abd943ec47e82f6453388d17cbcaaf78ef13ecf Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Sat, 1 Jul 2023 18:06:19 +0200 Subject: [PATCH 11/17] Avoid introducing new public API --- src/Foundation/NSObject2.cs | 3 ++- src/ObjCRuntime/INativeObject.cs | 3 ++- tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Foundation/NSObject2.cs b/src/Foundation/NSObject2.cs index 9279874b5ce4..d4b920df3179 100644 --- a/src/Foundation/NSObject2.cs +++ b/src/Foundation/NSObject2.cs @@ -70,7 +70,8 @@ public class NSObjectFlag { #endif #if NET - public interface INSObjectFactory { + // This interface will be made public when the managed static registrar is used. + internal interface INSObjectFactory { // The method will be implemented via custom linker step if the managed static registrar is used // for NSObject subclasses which have an (NativeHandle) or (IntPtr) constructor. [MethodImpl(MethodImplOptions.NoInlining)] diff --git a/src/ObjCRuntime/INativeObject.cs b/src/ObjCRuntime/INativeObject.cs index 2fd4e9e6b4cc..58c085400740 100644 --- a/src/ObjCRuntime/INativeObject.cs +++ b/src/ObjCRuntime/INativeObject.cs @@ -20,8 +20,9 @@ NativeHandle Handle { #if NET // The method will be implemented via custom linker step if the managed static registrar is used // for classes which have an (NativeHandle, bool) or (IntPtr, bool) constructor. + // This method will be made public when the managed static registrar is used. [MethodImpl(MethodImplOptions.NoInlining)] - static virtual INativeObject? _Xamarin_ConstructINativeObject (NativeHandle handle, bool owns) => null; + internal static virtual INativeObject? _Xamarin_ConstructINativeObject (NativeHandle handle, bool owns) => null; #endif } diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index 662e9604e37a..ad14e573610b 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -418,6 +418,7 @@ void ImplementConstructNSObjectFactoryMethod (TypeDefinition type, MethodReferen var createInstanceMethod = type.AddMethod ("_Xamarin_ConstructNSObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.Foundation_NSObject); var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); + abr.Foundation_INSObjectFactory.Resolve ().IsPublic = true; createInstanceMethod.Overrides.Add (abr.INSObjectFactory__Xamarin_ConstructNSObject); var body = createInstanceMethod.CreateBody (out var il); @@ -450,6 +451,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe var createInstanceMethod = type.AddMethod ("_Xamarin_ConstructINativeObject", MethodAttributes.Public | MethodAttributes.Static | MethodAttributes.NewSlot | MethodAttributes.HideBySig, abr.ObjCRuntime_INativeObject); var nativeHandleParameter = createInstanceMethod.AddParameter ("nativeHandle", abr.ObjCRuntime_NativeHandle); var ownsParameter = createInstanceMethod.AddParameter ("owns", abr.System_Boolean); + abr.INativeObject__Xamarin_ConstructINativeObject.Resolve ().IsPublic = true; createInstanceMethod.Overrides.Add (abr.INativeObject__Xamarin_ConstructINativeObject); var body = createInstanceMethod.CreateBody (out var il); From f380f4c814fb210b4236d0bd9eccfc1068b0283f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 5 Jul 2023 07:13:14 +0200 Subject: [PATCH 12/17] Report specific error when it is not possible to create instances of generic types --- docs/website/mtouch-errors.md | 25 +++++ src/ObjCRuntime/Runtime.cs | 175 ++++++++++++++++++++++------------ 2 files changed, 141 insertions(+), 59 deletions(-) diff --git a/docs/website/mtouch-errors.md b/docs/website/mtouch-errors.md index 7faab8252854..e2007dd3e382 100644 --- a/docs/website/mtouch-errors.md +++ b/docs/website/mtouch-errors.md @@ -3749,3 +3749,28 @@ This exception will have an inner exception which gives the reason for the failu ### MT8036: Failed to convert the value at index {index} from {type} to {type}. This exception will have an inner exception which gives the reason for the failure. + + + +### MT8037: Failed to marshal the Objective-C object {handle} (type: {objc_type}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {managed_type}. + +This occurs when the Xamarin.iOS runtime finds an Objective-C object without a +corresponding managed wrapper object, and when trying to create that managed +wrapper, it turns out it's not possible. This error is specific to the managed +static registrar. + +There are a few reasons this may happen: + +* A managed wrapper existed at some point, but was collected by the GC. If the + native object is still alive, and later resurfaces to managed code, the + Xamarin.iOS runtime will try to re-create a managed wrapper instance. In + most cases the problem here is that the managed wrapper shouldn't have been + collected by the GC in the first place. + + Possible causes include: + + * Manually calling Dispose too early on the managed wrapper. + * Incorrect bindings for third-party libraries. + * Reference-counting bugs in third-party libraries. + +* This indicates a bug in Xamarin.iOS. Please file a new issue on [github](https://github.com/xamarin/xamarin-macios/issues/new). diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index e9b7057d20e6..a593288a1aa0 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1254,38 +1254,74 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut } msg.Append (")."); + if (sel != IntPtr.Zero || method_handle.Value != IntPtr.Zero) { - msg.AppendLine (); - msg.AppendLine ("Additional information:"); - if (sel != IntPtr.Zero) - msg.Append ("\tSelector: ").Append (Selector.GetName (sel)).AppendLine (); - if (method_handle.Value != IntPtr.Zero) { - try { - var method = MethodBase.GetMethodFromHandle (method_handle); - msg.Append ($"\tMethod: "); - if (method is not null) { - // there's no good built-in function to format a MethodInfo :/ - msg.Append (method.DeclaringType?.FullName ?? string.Empty); - msg.Append ("."); - msg.Append (method.Name); - msg.Append ("("); - var parameters = method.GetParameters (); - for (var i = 0; i < parameters.Length; i++) { - if (i > 0) - msg.Append (", "); - msg.Append (parameters [i].ParameterType.FullName); - } - msg.Append (")"); - } else { - msg.Append ($"Unable to resolve RuntimeMethodHandle 0x{method_handle.Value.ToString ("x")}"); + AppendAdditionalInformation (msg, sel, method_handle); + } + + throw ErrorHelper.CreateError (8027, msg.ToString ()); + } + +#if NET + static void CannotCreateManagedInstanceOfGenericType (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolution resolution, IntPtr sel, RuntimeMethodHandle method_handle) + { + Debug.Assert (Runtime.IsManagedStaticRegistrar); + Debug.Assert (type.IsGenericType); + + if (resolution == MissingCtorResolution.Ignore) + return; + + if (klass == IntPtr.Zero) + klass = Class.GetClassForObject (ptr); + + var msg = new StringBuilder (); + msg.Append ("Failed to create a managed counterpart of the Objective-C object 0x"); + msg.Append (ptr.ToString ("x")); + msg.Append (" (type: "); + msg.Append (new Class (klass).Name); + msg.Append ("). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type '"); + msg.Append (type.FullName); + msg.Append ("'."); + + if (sel != IntPtr.Zero || method_handle.Value != IntPtr.Zero) { + AppendAdditionalInformation (msg, sel, method_handle); + } + + throw ErrorHelper.CreateError (8037, msg.ToString ()); + } +#endif + + static void AppendAdditionalInformation (StringBuilder msg, IntPtr sel, RuntimeMethodHandle method_handle) + { + msg.AppendLine (); + msg.AppendLine ("Additional information:"); + if (sel != IntPtr.Zero) + msg.Append ("\tSelector: ").Append (Selector.GetName (sel)).AppendLine (); + if (method_handle.Value != IntPtr.Zero) { + try { + var method = MethodBase.GetMethodFromHandle (method_handle); + msg.Append ($"\tMethod: "); + if (method is not null) { + // there's no good built-in function to format a MethodInfo :/ + msg.Append (method.DeclaringType?.FullName ?? string.Empty); + msg.Append ("."); + msg.Append (method.Name); + msg.Append ("("); + var parameters = method.GetParameters (); + for (var i = 0; i < parameters.Length; i++) { + if (i > 0) + msg.Append (", "); + msg.Append (parameters [i].ParameterType.FullName); } - msg.AppendLine (); - } catch (Exception ex) { - msg.Append ($"\tMethod: Unable to resolve RuntimeMethodHandle 0x{method_handle.Value.ToString ("x")}: {ex.Message}"); + msg.Append (")"); + } else { + msg.Append ($"Unable to resolve RuntimeMethodHandle 0x{method_handle.Value.ToString ("x")}"); } + msg.AppendLine (); + } catch (Exception ex) { + msg.Append ($"\tMethod: Unable to resolve RuntimeMethodHandle 0x{method_handle.Value.ToString ("x")}: {ex.Message}"); } } - throw ErrorHelper.CreateError (8027, msg.ToString ()); } static NSObject? ConstructNSObject (IntPtr ptr, IntPtr klass, MissingCtorResolution missingCtorResolution) @@ -1324,21 +1360,36 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut T? instance = default; var nativeHandle = new NativeHandle (ptr); - if (typeof (T) != typeof (NSObject) - && (typeof (T) == type || typeof (T).IsGenericType) + // We want to create an instance of `type` and if we have the chance to use the factory method + // on the generic type, we will prefer it to using the lookup table. + if (typeof (T) == type // TODO can I drop the IsGenericType check? would it break some test? + && typeof (T) != typeof (NSObject) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { instance = ConstructNSObjectViaFactoryMethod (nativeHandle); } - // If we couldn't create an instance of T through the factory method, we'll use the lookup table. - // This table can't instantiate generic types though. - if (!type.IsGenericType) { - instance ??= RegistrarHelper.ConstructNSObject (type, nativeHandle); + // Generic types can only be instantiated through the factory method and if that failed, we can't + // fall back to the lookup tables and we need to stop here. + if (type.IsGenericType && instance is null) { + CannotCreateManagedInstanceOfGenericType (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); + return null; + } + + // If we couldn't create an instance of T through the factory method, we'll use the lookup table + // based on the RuntimeTypeHandle. + // + // This isn't possible for generic types - we don't know the type arguments at compile time + // (otherwise we would be able to create an instance of T through the factory method). + // For non-generic types, we can call the NativeHandle constructor based on the RuntimeTypeHandle. + + if (instance is null) { + instance = RegistrarHelper.ConstructNSObject (type, nativeHandle); } if (instance is null) { + // If we couldn't create an instance using the lookup table either, it means `type` doesn't contain + // a suitable constructor. MissingCtor (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); - return null; } return instance; @@ -1366,7 +1417,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut return (T) ctor.Invoke (ctorArguments); #if NET - // It isn't possible to call T.ConstructNSObject (...) directly from the parent function. For some + // It isn't possible to call T._Xamarin_ConstructNSObject (...) directly from the parent function. For some // types, the app crashes with a SIGSEGV: // // error: * Assertion at /Users/runner/work/1/s/src/mono/mono/mini/mini-generic-sharing.c:2283, condition `m_class_get_vtable (info->klass)' not met @@ -1402,39 +1453,45 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut var nativeHandle = new NativeHandle (ptr); T? instance = null; - // - The factory method on T is only useful if we know the exact generic type (it's not INativeObject) - // and we're not just falling back to the INativeObject/NSObject factory method (which would throw - // an exception anyway). - // - If `T` is different from `type`, we'd rather use the lookup table to create the instance of the - // more specialized `type` than "just" T unless it's a generic type. We can't create instances of generic - // types through the lookup table. - // - It doesn't make sense to create an instance of an interface or an abstract class. - if (typeof (T) != typeof (INativeObject) + // We want to create an instance of `type` and if we have the chance to use the factory method + // on the generic type, we will prefer it to using the lookup table. + if (typeof (T) == type + && typeof (T) != typeof (INativeObject) && typeof (T) != typeof (NSObject) - && (typeof (T) == type || typeof (T).IsGenericType) - && !(typeof (T).IsInterface || typeof (T).IsAbstract)) - { + && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { instance = ConstructINativeObjectViaFactoryMethod (nativeHandle, owns); } - // if the factory didn't work and the type isn't generic, we can try the lookup table - if (instance is null && !type.IsGenericType) { - // if type is an NSObject, we prefer the NSObject lookup table - if (type != typeof (NSObject) && type.IsSubclassOf (typeof (NSObject))) { - instance = (T?)(INativeObject?) RegistrarHelper.ConstructNSObject (type, nativeHandle); - if (instance is not null && owns) { - Runtime.TryReleaseINativeObject (instance); - } - } + // Generic types can only be instantiated through the factory method and if that failed, we can't + // fall back to the lookup tables and we need to stop here. + if (type.IsGenericType && instance is null) { + CannotCreateManagedInstanceOfGenericType (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); + return null; + } - if (instance is null && type != typeof (INativeObject)) { - instance = RegistrarHelper.ConstructINativeObject (type, nativeHandle, owns); + // If we couldn't create an instance of T through the factory method, we'll use the lookup table + // based on the RuntimeTypeHandle. + // + // This isn't possible for generic types - we don't know the type arguments at compile time + // (otherwise we would be able to create an instance of T through the factory method). + // For non-generic types, we can call the NativeHandle constructor based on the RuntimeTypeHandle. + + // If type is an NSObject, we prefer the NSObject lookup table + if (instance is null && type != typeof (NSObject) && type.IsSubclassOf (typeof (NSObject))) { + instance = (T?)(INativeObject?) RegistrarHelper.ConstructNSObject (type, nativeHandle); + if (instance is not null && owns) { + Runtime.TryReleaseINativeObject (instance); } } + if (instance is null && type != typeof (INativeObject)) { + instance = RegistrarHelper.ConstructINativeObject (type, nativeHandle, owns); + } + if (instance is null) { + // If we couldn't create an instance using the lookup table either, it means `type` doesn't contain + // a suitable constructor. MissingCtor (ptr, IntPtr.Zero, type, missingCtorResolution, sel, method_handle); - return null; } return instance; @@ -1463,7 +1520,7 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut return (T?) ctor.Invoke (ctorArguments); #if NET - // It isn't possible to call T.ConstructINativeObject (...) directly from the parent function. For some + // It isn't possible to call T._Xamarin_ConstructINativeObject (...) directly from the parent function. For some // types, the app crashes with a SIGSEGV: // // error: * Assertion at /Users/runner/work/1/s/src/mono/mono/mini/mini-generic-sharing.c:2283, condition `m_class_get_vtable (info->klass)' not met From 828d2c7f8e3d34cd39b6b648a95b718a3be09ebe Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 7 Aug 2023 12:24:45 +0200 Subject: [PATCH 13/17] Update error message --- docs/website/mtouch-errors.md | 4 ++-- src/ObjCRuntime/Runtime.cs | 10 ++-------- tools/mtouch/Errors.designer.cs | 9 +++++++++ tools/mtouch/Errors.resx | 4 ++++ 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/docs/website/mtouch-errors.md b/docs/website/mtouch-errors.md index e2007dd3e382..034dd76d5bd9 100644 --- a/docs/website/mtouch-errors.md +++ b/docs/website/mtouch-errors.md @@ -3750,9 +3750,9 @@ This exception will have an inner exception which gives the reason for the failu This exception will have an inner exception which gives the reason for the failure. - + -### MT8037: Failed to marshal the Objective-C object {handle} (type: {objc_type}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {managed_type}. +### MX8056: Failed to marshal the Objective-C object {handle} (type: {objc_type}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {managed_type}. This occurs when the Xamarin.iOS runtime finds an Objective-C object without a corresponding managed wrapper object, and when trying to create that managed diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 8c682a319eb7..7c48f8e11744 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1281,19 +1281,13 @@ static void CannotCreateManagedInstanceOfGenericType (IntPtr ptr, IntPtr klass, klass = Class.GetClassForObject (ptr); var msg = new StringBuilder (); - msg.Append ("Failed to create a managed counterpart of the Objective-C object 0x"); - msg.Append (ptr.ToString ("x")); - msg.Append (" (type: "); - msg.Append (new Class (klass).Name); - msg.Append ("). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type '"); - msg.Append (type.FullName); - msg.Append ("'."); + msg.AppendFormat (Xamarin.Bundler.Errors.MX8056 /* Failed to marshal the Objective-C object 0x{0} (type: {1}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {2}. */, ptr.ToString ("x"), new Class (klass).Name, type.FullName); if (sel != IntPtr.Zero || method_handle.Value != IntPtr.Zero) { AppendAdditionalInformation (msg, sel, method_handle); } - throw ErrorHelper.CreateError (8037, msg.ToString ()); + throw ErrorHelper.CreateError (8056, msg.ToString ()); } #endif diff --git a/tools/mtouch/Errors.designer.cs b/tools/mtouch/Errors.designer.cs index d770fa6bddc0..797627573c54 100644 --- a/tools/mtouch/Errors.designer.cs +++ b/tools/mtouch/Errors.designer.cs @@ -4292,5 +4292,14 @@ public static string MX8055 { return ResourceManager.GetString("MX8055", resourceCulture); } } + + /// + /// Looks up a localized string similar to Failed to marshal the Objective-C object 0x{0} (type: {1}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {2}.. + /// + public static string MX8056 { + get { + return ResourceManager.GetString("MX8056", resourceCulture); + } + } } } diff --git a/tools/mtouch/Errors.resx b/tools/mtouch/Errors.resx index 8ebe9f75b579..e88b0756970f 100644 --- a/tools/mtouch/Errors.resx +++ b/tools/mtouch/Errors.resx @@ -2254,4 +2254,8 @@ Could not find the type 'ObjCRuntime.__Registrar__' in the assembly '{0}'. + + + Failed to marshal the Objective-C object 0x{0} (type: {1}). Could not find an existing managed instance for this object, nor was it possible to create a new managed instance of generic type {2}. + From 026876dd92b46097a74a36536a70db6a2288412a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 7 Aug 2023 13:06:42 +0200 Subject: [PATCH 14/17] Remove asserts --- src/ObjCRuntime/Runtime.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 7c48f8e11744..10fb8b876820 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1271,9 +1271,6 @@ static void MissingCtor (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolut #if NET static void CannotCreateManagedInstanceOfGenericType (IntPtr ptr, IntPtr klass, Type type, MissingCtorResolution resolution, IntPtr sel, RuntimeMethodHandle method_handle) { - Debug.Assert (Runtime.IsManagedStaticRegistrar); - Debug.Assert (type.IsGenericType); - if (resolution == MissingCtorResolution.Ignore) return; From 1429f2d5f07e24a754be7a4a97871ee30bcd3e62 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 7 Aug 2023 13:07:25 +0200 Subject: [PATCH 15/17] Improve generated code --- .../Steps/ManagedRegistrarLookupTablesStep.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs index ad14e573610b..803a063e1de7 100644 --- a/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs +++ b/tools/dotnet-linker/Steps/ManagedRegistrarLookupTablesStep.cs @@ -342,11 +342,8 @@ void GenerateConstructNSObject (TypeDefinition registrarType) ImplementConstructNSObjectFactoryMethod (type, ctor); } - // return default (NSObject) - var temporary = body.AddVariable (abr.Foundation_NSObject); - il.Emit (OpCodes.Ldloca, temporary); - il.Emit (OpCodes.Initobj, abr.Foundation_NSObject); - il.Emit (OpCodes.Ldloc, temporary); + // return default (NSObject); + il.Emit (OpCodes.Ldnull); il.Emit (OpCodes.Ret); } @@ -403,10 +400,7 @@ void GenerateConstructINativeObject (TypeDefinition registrarType) } // return default (NSObject) - var temporary = body.AddVariable (abr.Foundation_NSObject); - il.Emit (OpCodes.Ldloca, temporary); - il.Emit (OpCodes.Initobj, abr.Foundation_NSObject); - il.Emit (OpCodes.Ldloc, temporary); + il.Emit (OpCodes.Ldnull); il.Emit (OpCodes.Ret); } @@ -512,7 +506,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe ?? FindConstructorWithOneParameter ("System", "IntPtr"); MethodReference? FindConstructorWithOneParameter (string ns, string cls) - => type.Methods.FirstOrDefault (method => + => type.Methods.SingleOrDefault (method => method.IsConstructor && !method.IsStatic && method.HasParameters @@ -527,7 +521,7 @@ void ImplementConstructINativeObjectFactoryMethod (TypeDefinition type, MethodRe ?? FindConstructorWithTwoParameters ("System", "IntPtr", "System", "Boolean"); MethodReference? FindConstructorWithTwoParameters (string ns1, string cls1, string ns2, string cls2) - => type.Methods.FirstOrDefault (method => + => type.Methods.SingleOrDefault (method => method.IsConstructor && !method.IsStatic && method.HasParameters From 698fcbfb837e38ef64ac9f347f29484a25a6e874 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 7 Aug 2023 13:08:41 +0200 Subject: [PATCH 16/17] Simplify runtime code --- src/ObjCRuntime/Runtime.cs | 30 +++--------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 10fb8b876820..feecc918471d 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1421,18 +1421,7 @@ static void AppendAdditionalInformation (StringBuilder msg, IntPtr sel, RuntimeM // // When the same call is made from a separate function, it works fine. static T? ConstructNSObjectViaFactoryMethod (NativeHandle handle) - { - NSObject? obj = T._Xamarin_ConstructNSObject (handle); - if (obj is T instance) { - return instance; - } - - // if the factory method returns a NSObject subclass but we don't expect that specific type, - // we need to dispose it and return null anyway - obj?.Dispose (); - - return null; - } + => T._Xamarin_ConstructNSObject (handle) as T; #endif } @@ -1524,20 +1513,7 @@ static void AppendAdditionalInformation (StringBuilder msg, IntPtr sel, RuntimeM // // When the same call is made from a separate function, it works fine. static T? ConstructINativeObjectViaFactoryMethod (NativeHandle nativeHandle, bool owns) - { - INativeObject? obj = T._Xamarin_ConstructINativeObject (nativeHandle, owns); - if (obj is T instance) { - return instance; - } - - // if the factory method returns an INativeObject but we don't expect that specific type, - // we need to release it and return null anyway - if (obj is not null) { - Runtime.TryReleaseINativeObject(obj); - } - - return null; - } + => T._Xamarin_ConstructINativeObject (nativeHandle, owns) as T; #endif } @@ -1918,7 +1894,7 @@ static Type LookupINativeObjectImplementation (IntPtr ptr, Type target_type, Typ // native objects and NSObject instances. throw ErrorHelper.CreateError (8004, $"Cannot create an instance of {implementation.FullName} for the native object 0x{ptr:x} (of type '{Class.class_getName (Class.GetClassForObject (ptr))}'), because another instance already exists for this native object (of type {o.GetType ().FullName})."); } - return (INativeObject?) ConstructNSObject (ptr, implementation!, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); + return ConstructNSObject (ptr, implementation!, MissingCtorResolution.ThrowConstructor1NotFound, sel, method_handle); } return ConstructINativeObject (ptr, owns, implementation, MissingCtorResolution.ThrowConstructor2NotFound, sel, method_handle); From f834c3b82fe10c492f365c4a031197c13a33646c Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 14 Aug 2023 16:55:08 +0200 Subject: [PATCH 17/17] Remove outdated comment --- src/ObjCRuntime/Runtime.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index feecc918471d..3db1cc5e3b48 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -1359,7 +1359,7 @@ static void AppendAdditionalInformation (StringBuilder msg, IntPtr sel, RuntimeM // We want to create an instance of `type` and if we have the chance to use the factory method // on the generic type, we will prefer it to using the lookup table. - if (typeof (T) == type // TODO can I drop the IsGenericType check? would it break some test? + if (typeof (T) == type && typeof (T) != typeof (NSObject) && !(typeof (T).IsInterface || typeof (T).IsAbstract)) { instance = ConstructNSObjectViaFactoryMethod (nativeHandle);