Use UnsafeAccessorType in System.Private.CoreLib and the BCL #115583

jkoritzinsky · 2025-05-14T21:00:11Z

Use the new UnsafeAccessorTypeAttribute to provide a more strongly-typed and straightforward mechanism to solve our layering issues in the product than the traditional Reflection API surface.

Copilot

Pull Request Overview

This PR replaces legacy reflection‐based access with the new UnsafeAccessorTypeAttribute mechanism to provide strongly-typed, direct calls for improved performance and maintainability across core libraries. Key changes include:

Replacing reflection calls with extern methods annotated with UnsafeAccessor attributes in libraries like System.Security.Cryptography, System.Private.CoreLib, and System.Net.Requests.
Removing intermediate reflection caching and delegate creation in favor of direct unsafe access to type members.
Updating several helper and interop classes to use these new patterns for cleaner API interactions.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/XmlKeyHelper.cs	Replaces reflection-based XML element access with unsafe accessor extern methods.
src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs	Eliminates reflection for BinaryFormatter-related instantiation in favor of unsafe accessors.
src/libraries/System.Private.CoreLib/src/System/ComponentModel/DefaultValueAttribute.cs	Switches from ad hoc reflection delegates to a direct unsafe accessor call for conversion.
src/libraries/System.Private.CoreLib/src/System/AppDomain.cs	Substitutes reflection-based principal retrieval with unsafe accessor methods.
src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs	Uses unsafe accessor extern methods to directly get/set private fields instead of reflection.
src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs	Utilizes an unsafe accessor method to create object array delegates.
src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs	Implements an unsafe accessor call to perform type conversion reliably under trimming.
src/coreclr/System.Private.CoreLib/src/System/Diagnostics/StackFrameHelper.cs	Replaces dynamic delegate creation with unsafe accessor caching for source line info.
src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs	Refactors licensing interop to use static unsafe accessor methods for COM activation.

Comments suppressed due to low confidence (4)

src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs:893

Confirm that switching to the UnsafeAccessor-based SetSavedLicenseKey call maintains the required initialization order and COM activation assumptions without side effects.

SetSavedLicenseKey(_licContext, _targetRcwType, key);

src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs:1696

Ensure that the ref-return usage from GetImpersonationLevel supports inline assignment on all targeted platforms and behaves equivalently to the previous reflection-based implementation.

GetImpersonationLevel(GetSettings(handler)) = request.ImpersonationLevel;

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs:32

Verify that passing null as the first parameter to CreateObjectArrayDelegate is safe and does not lead to unexpected behavior in scenarios where an instance value might be required.

return CreateObjectArrayDelegate(null, delegateType, invoker);

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs:116

Review the removal of the fallback logic for s_binaryFormatterType and s_deserializeMethod to ensure that the new unsafe accessor approach does not introduce regressions in environments with differing binary formatter support.

return true;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/XmlKeyHelper.cs

Co-authored-by: Jan Kotas <[email protected]>

teo-tsirpanis · 2025-05-15T01:39:49Z

There's also this:

runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/AssemblyName.cs

Lines 168 to 193 in c7c961a

    
                   private static Func<string, AssemblyName>? s_getAssemblyName; 
        
                   private static Func<string, AssemblyName> InitGetAssemblyName() 
        
                   { 
        
                       Type readerType = Type.GetType( 
        
                               "System.Reflection.Metadata.MetadataReader, System.Reflection.Metadata", 
        
                               throwOnError: true)!; 
        
                       MethodInfo? getAssemblyNameMethod = readerType.GetMethod( 
        
                           "GetAssemblyName", 
        
                           BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static, 
        
                           null, 
        
                           [typeof(string)], 
        
                           null) ?? 
        
                           throw new MissingMethodException(readerType.FullName, "GetAssemblyName"); 
        
                       return s_getAssemblyName = getAssemblyNameMethod.CreateDelegate<Func<string, AssemblyName>>(); 
        
                   } 
        
                   /* 
        
                    * Get the AssemblyName for a given file. This will only work 
        
                    * if the file contains an assembly manifest. This method causes 
        
                    * the file to be opened and closed. 
        
                    */ 
        
                   public static AssemblyName GetAssemblyName(string assemblyFile) 
        
                   { 
        
                       return (s_getAssemblyName ?? InitGetAssemblyName())(assemblyFile); 
        
                   }

src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs

src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

…descriptor for new settings property

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs

...aries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.InvokeNativeHandler.cs

src/libraries/System.Private.CoreLib/src/System/AppDomain.cs

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

src/libraries/System.Private.CoreLib/src/System/AppDomain.cs

jkotas

A few style nits. LGTM otherwise. Thank you!

jkotas · 2025-05-24T00:31:37Z

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs

@@ -222,7 +196,7 @@ private static Delegate CreateObjectArrayDelegateRefEmit(Type delegateType, Func
                            // for example when running on CoreClr with PublishAot=true, this will allow IL to be emitted.
                            // If we are running on a runtime that really doesn't support dynamic code, like NativeAOT,
                            // CanEmitObjectArrayDelegate will be flipped to 'false', and this method won't be invoked.
-                            return ForceAllowDynamicCodeLightup.ForceAllowDynamicCodeDelegate?.Invoke();
+                            return ForceAllowDynamicCode();


Suggested change

return ForceAllowDynamicCode();

return ForceAllowDynamicCode(null);

[UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="ForceAllowDynamicCode")]

static extern IDisposable ForceAllowDynamicCode(AssemblyBuilder? _);

Make it local?

jkotas · 2025-05-24T00:33:09Z

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs

@@ -63,7 +37,7 @@ internal static Delegate CreateObjectArrayDelegate(Type delegateType, Func<objec
            }
            else
            {
-                return DynamicDelegateLightup.CreateObjectArrayDelegate(delegateType, handler);
+                return CreateObjectArrayDelegate(null, delegateType, handler);


Suggested change

return CreateObjectArrayDelegate(null, delegateType, handler);

return CreateObjectArrayDelegate(null, delegateType, handler);

[UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="CreateObjectArrayDelegate")]

static extern Delegate CreateObjectArrayDelegate(

[UnsafeAccessorType("Internal.Runtime.Augments.DynamicDelegateAugments, System.Private.CoreLib")] object? _,

Type delegateType,

Func<object?[], object?> invoker);

jkotas · 2025-05-24T00:40:48Z

src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/TypeDescriptor.cs

@@ -3044,6 +3045,10 @@ private sealed class TypeDescriptorComObject
        // string method is the failure is silent during type load making diagnosing the issue difficult.
        private sealed class ComNativeDescriptorProxy : TypeDescriptionProvider
        {
+            [UnsafeAccessor(UnsafeAccessorKind.Constructor)]
+            [return: UnsafeAccessorType("System.Windows.Forms.ComponentModel.Com2Interop.ComNativeDescriptor, System.Windows.Forms")]
+            private static extern object CreateComNativeDescriptor();


Make it local?

(Our style https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/coding-style.md prefers to have fields the first thing in the type.)

jkotas · 2025-05-24T00:43:11Z

src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs

+        [UnsafeAccessor(UnsafeAccessorKind.Method)]
+        private static extern bool Contains(
+            [UnsafeAccessorType(LicInfoHelperLicenseContextTypeName)] object? licInfoHelperContext,
+            string assemblyName);

        // RCW Activation


Style nit: Move fields to the top

stephentoub · 2025-05-28T21:03:28Z

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

-                }
+                Type? taskOfHttpResponseMessageType = Type.GetType(TaskOfHttpResponseMessageTypeName, throwOnError: false);
+
+                PropertyInfo? taskResultProperty = taskOfHttpResponseMessageType?.GetProperty("Result");


There's no way to express Task<HttpResponseMessage> and its Result via UnsafeAccessorType?

We can't represent an unknown type argument for a type in the UnsafeAccessor logic. Maybe I can play some games with my own wrapper method. I'll see what I can figure out.

stephentoub · 2025-05-28T21:04:31Z

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

+                        object sendTask = SendAsync(httpClient, requestMessage, cancellationToken);
+                        await ((Task)sendTask).ConfigureAwait(false);
+                        responseMessage = taskResultProperty.GetValue(sendTask)!;


Suggested change

object sendTask = SendAsync(httpClient, requestMessage, cancellationToken);

await ((Task)sendTask).ConfigureAwait(false);

responseMessage = taskResultProperty.GetValue(sendTask)!;

Task sendTask = (Task)SendAsync(httpClient, requestMessage, cancellationToken);

await sendTask.ConfigureAwait(false);

responseMessage = taskResultProperty.GetValue(sendTask)!;

?

stephentoub · 2025-05-28T21:05:02Z

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs

+                            object sendTask = SendAsync(httpClient, requestMessage, cancellationToken);
+                            await ((Task)sendTask).ConfigureAwait(false);
+                            responseMessage = taskResultProperty.GetValue(sendTask)!;


Suggested change

object sendTask = SendAsync(httpClient, requestMessage, cancellationToken);

await ((Task)sendTask).ConfigureAwait(false);

responseMessage = taskResultProperty.GetValue(sendTask)!;

Task sendTask = (Task)SendAsync(httpClient, requestMessage, cancellationToken);

await sendTask.ConfigureAwait(false);

responseMessage = taskResultProperty.GetValue(sendTask)!;

stephentoub · 2025-05-28T21:08:13Z

src/tools/illink/src/linker/Linker.Steps/UnsafeAccessorMarker.cs

+		{
+			// A type named in UnsafeAccessorType may point to a type that is forwarded.
+			// Mark the type forwarders so we can avoid needing to rewrite the type name
+			// in the UnsafeAccessorTypeAttribute.


NAOT's trimmer already handles UnsafeAccessorType appropriately?

Yep.

The problem here is that while both the linker and NAOT can follow type forwards, the linker must also preserve the type forwards for CoreCLR to use at runtime. Otherwise we'll trim away the type forwards we used and then CoreCLR won't have them available (what was happening beforehand).

jkoritzinsky added 2 commits May 13, 2025 17:30

Use UnsafeAccessorType in StackFrameHelper

11e9cd1

Convert ComActivator to use UnsafeAccessorType

6166c51

jkoritzinsky requested review from Copilot and AaronRobinsonMSFT May 14, 2025 21:00

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 14, 2025

dotnet-policy-service bot assigned jkoritzinsky May 14, 2025

Copilot AI reviewed May 14, 2025

View reviewed changes

jkoritzinsky added 8 commits May 14, 2025 14:39

ConvertDefaultValueAttribute to use UnsafeAccessorType

df05d6b

Move AppDomain to use UnsafeAccessorType

22cd4be

Convert ResourceReader to use UnsafeAccessorType

7cee5e3

Convert X509ResourceClient to use UnsafeAccessor

7b696e7

Convert TypeDescriptor to use UnsafeAccessorType

15d985f

Convert DelegateHelpers to use UnsafeAccessorType

77ba50d

Convert HttpWebRequest to use UnsafeAccessorType

fe47df6

Convert XmlKeyHelper to use UnsafeAccessorType

d71b334

jkoritzinsky force-pushed the unsafe-accessor-type-usage branch from 4e1e330 to d71b334 Compare May 14, 2025 21:39

jkotas reviewed May 14, 2025

View reviewed changes

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/XmlKeyHelper.cs Outdated Show resolved Hide resolved

Update XmlKeyHelper.cs

a1f3bc7

Co-authored-by: Jan Kotas <[email protected]>

AaronRobinsonMSFT reviewed May 15, 2025

View reviewed changes

src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 15, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 15, 2025

View reviewed changes

src/coreclr/System.Private.CoreLib/src/Internal/Runtime/InteropServices/ComActivator.cs Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 15, 2025

View reviewed changes

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs Outdated Show resolved Hide resolved

build-analysis bot mentioned this pull request May 15, 2025

[wasm][AOT] emcc : error - received SIGKILL (-9) #89402

Open

Remove tests that test removed reflection infra and add librarybuild …

1691219

…descriptor for new settings property

This was referenced May 20, 2025

Test failure System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_Create_Invalid #94498

Open

System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters Failed #110454

Open

Fix exception for WindowsPrincipal

eb866c9

This was referenced May 21, 2025

ExplicitConversion_FromSingle failing due to NaN != NaN #103347

Open

OSX failure on StringTests.StartsWithNoMatch_StringComparison #112195

Open

marek-safar reviewed May 21, 2025

View reviewed changes

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs Show resolved Hide resolved

build-analysis bot mentioned this pull request May 21, 2025

ProcessThreadTests.TestStartTimeProperty failure in CI #105526

Open

Fix GetStatusCode return

337d504

jkoritzinsky requested review from AaronRobinsonMSFT and jkotas May 22, 2025 02:01

jkotas reviewed May 22, 2025

View reviewed changes

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs Outdated Show resolved Hide resolved

jkotas reviewed May 22, 2025

View reviewed changes

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/DelegateHelpers.cs Show resolved Hide resolved

jkotas reviewed May 22, 2025

View reviewed changes

...aries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.InvokeNativeHandler.cs Show resolved Hide resolved

jkotas reviewed May 22, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/AppDomain.cs Outdated Show resolved Hide resolved

jkotas reviewed May 22, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 23, 2025

View reviewed changes

src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 23, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/AppDomain.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 23, 2025

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/AppDomain.cs Outdated Show resolved Hide resolved

jkoritzinsky added 2 commits May 23, 2025 14:35

PR feedback

e86536f

PR Feedback

9c474cd

jkoritzinsky requested review from AaronRobinsonMSFT and jkotas May 23, 2025 23:26

jkotas approved these changes May 24, 2025

View reviewed changes

AaronRobinsonMSFT approved these changes May 25, 2025

View reviewed changes

stephentoub reviewed May 28, 2025

View reviewed changes

stephentoub approved these changes May 28, 2025

View reviewed changes

-                            return ForceAllowDynamicCode();
+                            return ForceAllowDynamicCode(null);
+                            [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="ForceAllowDynamicCode")]
+                            static extern IDisposable ForceAllowDynamicCode(AssemblyBuilder? _);

-                return CreateObjectArrayDelegate(null, delegateType, handler);
+                return CreateObjectArrayDelegate(null, delegateType, handler);
+                [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name="CreateObjectArrayDelegate")]
+                static extern Delegate CreateObjectArrayDelegate(
+                    [UnsafeAccessorType("Internal.Runtime.Augments.DynamicDelegateAugments, System.Private.CoreLib")] object? _,
+                    Type delegateType,
+                    Func<object?[], object?> invoker);

Use UnsafeAccessorType in System.Private.CoreLib and the BCL #115583

Are you sure you want to change the base?

Use UnsafeAccessorType in System.Private.CoreLib and the BCL #115583

Conversation

jkoritzinsky commented May 14, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

teo-tsirpanis commented May 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!