From 5479bc2695bae05ad43cc88906a9c6e40e3e867a Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Sun, 6 Aug 2023 13:50:50 -0400 Subject: [PATCH 1/3] [Generator] Ensure that selectors fields do not have overlapping names. Fixes #18645 The way the generator names private fields for selectors results on not compilable code when there are two selectoors A and B such that selector B == AHandle. That is, if selector A is "user", selector B will be "userHandle". This happens because Handle is quite a common postfix in security when working with user accounts (userHandler and others) and we need to ensure that the Handle postfix added by the generator is unique. As stated in the bug we could try to fix this by keeping track of the variables in the context in wich the generator is creating code. The problem with such approach is that it will make very hard to predict the name of the fields making the manual code harder to write. The PR contains examples of such manual code. To fix the situation we have moved from using Handle to XHandle as a postfix in the field names whihc reduces the chances of finding a similar corner case in the future. A test has been added to show the case in which we found the bug. Fixes #18645 --- src/AppKit/Compat.cs | 8 ++--- src/Foundation/NSArray.cs | 4 +-- src/Foundation/NSDictionary.cs | 2 +- src/Foundation/NSMutableDictionary.cs | 4 +-- src/Foundation/NSObject2.cs | 4 +-- src/Foundation/NSUrlProtocol.cs | 4 +-- src/bgen/Generator.cs | 4 +-- tests/generator/BGenTests.cs | 3 ++ tests/generator/ghissue18645.cs | 44 +++++++++++++++++++++++++++ 9 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 tests/generator/ghissue18645.cs diff --git a/src/AppKit/Compat.cs b/src/AppKit/Compat.cs index 2039f1bc711a..75ba919c516d 100644 --- a/src/AppKit/Compat.cs +++ b/src/AppKit/Compat.cs @@ -19,9 +19,9 @@ public virtual void SetTextBlocks (NSTextBlock [] array) throw new ArgumentNullException (nameof (array)); var nsa_array = NSArray.FromNSObjects (array); if (IsDirectBinding) { - global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextBlocks_Handle, nsa_array.Handle); + global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextBlocks_XHandle, nsa_array.Handle); } else { - global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextBlocks_Handle, nsa_array.Handle); + global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextBlocks_XHandle, nsa_array.Handle); } nsa_array.Dispose (); } @@ -35,9 +35,9 @@ public virtual void SetTextLists (NSTextList [] array) throw new ArgumentNullException (nameof (array)); var nsa_array = NSArray.FromNSObjects (array); if (IsDirectBinding) { - global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextLists_Handle, nsa_array.Handle); + global::ObjCRuntime.Messaging.void_objc_msgSend_IntPtr (this.Handle, selSetTextLists_XHandle, nsa_array.Handle); } else { - global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextLists_Handle, nsa_array.Handle); + global::ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr (this.SuperHandle, selSetTextLists_XHandle, nsa_array.Handle); } nsa_array.Dispose (); } diff --git a/src/Foundation/NSArray.cs b/src/Foundation/NSArray.cs index 254662efae68..c4dc95f6b353 100644 --- a/src/Foundation/NSArray.cs +++ b/src/Foundation/NSArray.cs @@ -247,7 +247,7 @@ static public NSArray FromIntPtrs (NativeHandle [] vals) internal static nuint GetCount (IntPtr handle) { #if MONOMAC - return (nuint) Messaging.UIntPtr_objc_msgSend (handle, selCountHandle); + return (nuint) Messaging.UIntPtr_objc_msgSend (handle, selCountXHandle); #else return (nuint) Messaging.UIntPtr_objc_msgSend (handle, Selector.GetHandle ("count")); #endif @@ -259,7 +259,7 @@ internal static NativeHandle GetAtIndex (NativeHandle handle, nuint i) return Messaging.NativeHandle_objc_msgSend_UIntPtr (handle, Selector.GetHandle ("objectAtIndex:"), (UIntPtr) i); #else #if MONOMAC - return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, selObjectAtIndex_Handle, (UIntPtr) i); + return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, selObjectAtIndex_XHandle, (UIntPtr) i); #else return Messaging.IntPtr_objc_msgSend_UIntPtr (handle, Selector.GetHandle ("objectAtIndex:"), (UIntPtr) i); #endif diff --git a/src/Foundation/NSDictionary.cs b/src/Foundation/NSDictionary.cs index f219284eac17..01aec739cfe2 100644 --- a/src/Foundation/NSDictionary.cs +++ b/src/Foundation/NSDictionary.cs @@ -402,7 +402,7 @@ public IEnumerator> GetEnumerator () public IntPtr LowlevelObjectForKey (IntPtr key) { #if MONOMAC - return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, selObjectForKey_Handle, key); + return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, selObjectForKey_XHandle, key); #else return ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr (this.Handle, Selector.GetHandle ("objectForKey:"), key); #endif diff --git a/src/Foundation/NSMutableDictionary.cs b/src/Foundation/NSMutableDictionary.cs index 35b56c3ed3e1..73a6e19ce058 100644 --- a/src/Foundation/NSMutableDictionary.cs +++ b/src/Foundation/NSMutableDictionary.cs @@ -319,7 +319,7 @@ public IEnumerator> GetEnumerator () public static NSMutableDictionary LowlevelFromObjectAndKey (IntPtr obj, IntPtr key) { #if MONOMAC - return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, selDictionaryWithObject_ForKey_Handle, obj, key)); + return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, selDictionaryWithObject_ForKey_XHandle, obj, key)); #else return (NSMutableDictionary) Runtime.GetNSObject (ObjCRuntime.Messaging.IntPtr_objc_msgSend_IntPtr_IntPtr (class_ptr, Selector.GetHandle ("dictionaryWithObject:forKey:"), obj, key)); #endif @@ -328,7 +328,7 @@ public static NSMutableDictionary LowlevelFromObjectAndKey (IntPtr obj, IntPtr k public void LowlevelSetObject (IntPtr obj, IntPtr key) { #if MONOMAC - ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetObject_ForKey_Handle, obj, key); + ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetObject_ForKey_XHandle, obj, key); #else ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, Selector.GetHandle ("setObject:forKey:"), obj, key); #endif diff --git a/src/Foundation/NSObject2.cs b/src/Foundation/NSObject2.cs index f88f1a0492c2..2ef3af13adac 100644 --- a/src/Foundation/NSObject2.cs +++ b/src/Foundation/NSObject2.cs @@ -880,9 +880,9 @@ public void SetValueForKeyPath (NativeHandle handle, NSString keyPath) #else #if MONOMAC if (IsDirectBinding) { - ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetValue_ForKeyPath_Handle, handle, keyPath.Handle); + ObjCRuntime.Messaging.void_objc_msgSend_IntPtr_IntPtr (this.Handle, selSetValue_ForKeyPath_XHandle, handle, keyPath.Handle); } else { - ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr_IntPtr (this.SuperHandle, selSetValue_ForKeyPath_Handle, handle, keyPath.Handle); + ObjCRuntime.Messaging.void_objc_msgSendSuper_IntPtr_IntPtr (this.SuperHandle, selSetValue_ForKeyPath_XHandle, handle, keyPath.Handle); } #else if (IsDirectBinding) { diff --git a/src/Foundation/NSUrlProtocol.cs b/src/Foundation/NSUrlProtocol.cs index 42eac05fc972..ac96916d348e 100644 --- a/src/Foundation/NSUrlProtocol.cs +++ b/src/Foundation/NSUrlProtocol.cs @@ -47,9 +47,9 @@ public NSUrlProtocol (NSUrlRequest request, NSCachedUrlResponse cachedResponse, if (client is null) throw new ArgumentNullException ("client"); if (IsDirectBinding) { - InitializeHandle (IntPtr_objc_msgSend_IntPtr_IntPtr_IntPtr (this.Handle, selInitWithRequest_CachedResponse_Client_Handle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:"); + InitializeHandle (IntPtr_objc_msgSend_IntPtr_IntPtr_IntPtr (this.Handle, selInitWithRequest_CachedResponse_Client_XHandle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:"); } else { - InitializeHandle (IntPtr_objc_msgSendSuper_IntPtr_IntPtr_IntPtr (this.SuperHandle, selInitWithRequest_CachedResponse_Client_Handle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:"); + InitializeHandle (IntPtr_objc_msgSendSuper_IntPtr_IntPtr_IntPtr (this.SuperHandle, selInitWithRequest_CachedResponse_Client_XHandle, request.Handle, cachedResponse is null ? IntPtr.Zero : cachedResponse.Handle, client.Handle), "initWithRequest:cachedResponse:client:"); } } diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index 2c7dc4485136..e788a64bf17e 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -2669,8 +2669,8 @@ public string SelectorField (string s, bool ignore_inline_directive = false) } else sb.Append (c); } - if (!InlineSelectors) - sb.Append ("Handle"); + if (!InlineSelectors) + sb.Append ("XHandle"); name = sb.ToString (); selector_names [s] = name; return name; diff --git a/tests/generator/BGenTests.cs b/tests/generator/BGenTests.cs index 5cb7df715580..8c10e27fbaf6 100644 --- a/tests/generator/BGenTests.cs +++ b/tests/generator/BGenTests.cs @@ -824,6 +824,9 @@ public void DiamondProtocol () [Test] public void GHIssue9065_Sealed () => BuildFile (Profile.iOS, nowarnings: true, "ghissue9065.cs"); + [Test] + public void GHIssue18645_DuplicatedFiled() => BuildFile (Profile.iOS, nowarnings: true, "ghissue18645.cs"); + // looking for [BindingImpl (BindingImplOptions.Optimizable)] bool IsOptimizable (MethodDefinition method) { diff --git a/tests/generator/ghissue18645.cs b/tests/generator/ghissue18645.cs new file mode 100644 index 000000000000..c759e44b6982 --- /dev/null +++ b/tests/generator/ghissue18645.cs @@ -0,0 +1,44 @@ +using Foundation; +using ObjCRuntime; + +namespace GHIssue18645{ + + [Protocol] + [BaseType (typeof(NSObject))] + interface ASCredentialIdentity { + + [Abstract] + [Export ("user")] + string User { get; } + + [Abstract] + [NullAllowed, Export ("recordIdentifier")] + string RecordIdentifier { get; } + + [Abstract] + [Export ("rank")] + nint Rank { get; set; } + } + + [BaseType (typeof (NSObject))] + interface ASPasskeyCredentialIdentity : ASCredentialIdentity { + [Export ("relyingPartyIdentifier")] + string RelyingPartyIdentifier { get; } + + [Export ("userName")] + string UserName { get; } + + [Export ("credentialID", ArgumentSemantic.Copy)] + NSData CredentialID { get; } + + [Export ("userHandle", ArgumentSemantic.Copy)] + NSData UserHandle { get; } + + [NullAllowed, Export ("recordIdentifier")] + new string RecordIdentifier { get; } + + [Export ("rank")] + new nint Rank { get; set; } + } + +} From 9787f0aa495ec3447deafa0a5fa3e8ef73b9fecb Mon Sep 17 00:00:00 2001 From: GitHub Actions Autoformatter Date: Sun, 6 Aug 2023 18:02:28 +0000 Subject: [PATCH 2/3] Auto-format source code --- src/bgen/Generator.cs | 2 +- tests/generator/BGenTests.cs | 2 +- tests/generator/ghissue18645.cs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bgen/Generator.cs b/src/bgen/Generator.cs index e788a64bf17e..a77b9e975c78 100644 --- a/src/bgen/Generator.cs +++ b/src/bgen/Generator.cs @@ -2669,7 +2669,7 @@ public string SelectorField (string s, bool ignore_inline_directive = false) } else sb.Append (c); } - if (!InlineSelectors) + if (!InlineSelectors) sb.Append ("XHandle"); name = sb.ToString (); selector_names [s] = name; diff --git a/tests/generator/BGenTests.cs b/tests/generator/BGenTests.cs index 8c10e27fbaf6..a9023c1a2052 100644 --- a/tests/generator/BGenTests.cs +++ b/tests/generator/BGenTests.cs @@ -825,7 +825,7 @@ public void DiamondProtocol () public void GHIssue9065_Sealed () => BuildFile (Profile.iOS, nowarnings: true, "ghissue9065.cs"); [Test] - public void GHIssue18645_DuplicatedFiled() => BuildFile (Profile.iOS, nowarnings: true, "ghissue18645.cs"); + public void GHIssue18645_DuplicatedFiled () => BuildFile (Profile.iOS, nowarnings: true, "ghissue18645.cs"); // looking for [BindingImpl (BindingImplOptions.Optimizable)] bool IsOptimizable (MethodDefinition method) diff --git a/tests/generator/ghissue18645.cs b/tests/generator/ghissue18645.cs index c759e44b6982..0819c3462ce6 100644 --- a/tests/generator/ghissue18645.cs +++ b/tests/generator/ghissue18645.cs @@ -1,10 +1,10 @@ using Foundation; using ObjCRuntime; -namespace GHIssue18645{ +namespace GHIssue18645 { [Protocol] - [BaseType (typeof(NSObject))] + [BaseType (typeof (NSObject))] interface ASCredentialIdentity { [Abstract] From 53f265714efbbac7a9c3fd7a52c499baf79250fd Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Mon, 7 Aug 2023 15:44:30 -0400 Subject: [PATCH 3/3] Fix mmp regression tests. --- tests/mmp-regression/link-frameworks-1/LinkFrameworks1.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/mmp-regression/link-frameworks-1/LinkFrameworks1.cs b/tests/mmp-regression/link-frameworks-1/LinkFrameworks1.cs index 2acb5ec0b944..0ea354e6caad 100644 --- a/tests/mmp-regression/link-frameworks-1/LinkFrameworks1.cs +++ b/tests/mmp-regression/link-frameworks-1/LinkFrameworks1.cs @@ -47,9 +47,9 @@ static void TestFrameworks () switch (field.Name) { case "selConformsToProtocolHandle": - case "selDescriptionHandle": - case "selHashHandle": - case "selIsEqual_Handle": + case "selDescriptionXHandle": + case "selHashXHandle": + case "selIsEqual_XHandle": case "class_ptr": // Unrelated fields continue;