Skip to content

Add [RequiresUnsafe] to all extern and LibraryImport (CoreLib)#125880

Closed
EgorBo wants to merge 5 commits intodotnet:mainfrom
EgorBo:requires-unsafe-pinvokes-opus
Closed

Add [RequiresUnsafe] to all extern and LibraryImport (CoreLib)#125880
EgorBo wants to merge 5 commits intodotnet:mainfrom
EgorBo:requires-unsafe-pinvokes-opus

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 21, 2026

This PR does 4 things:

  1. Issue a new warning IL5005 for [LibraryImport] partial methods and extern methods without [RequiresUnsafe] on them. (IL5004 already exists for pointer-type signatures.)
  2. Extends the code-fixer to add [RequiresUnsafe] when it's missing on [LibraryImport]/extern methods
  3. Disable the "api with [RequiresUnsafe] is invoked in a safe context" warning. We'll enable them once we're ready to run the fixer that will handle all the callsites (going to be a huge change).
  4. Updates the LibraryImport source generator to propagate [RequiresUnsafe] to the generated __PInvoke local extern methods.

Not done:

  1. Non-CoreLib (because the analyzers are only enabled for CoreLib currently in Add analyzer+codefix for applying RequiresUnsafe to pointer methods #125196)
  2. MethodImplOptions.InternalCall is ignored with a TODO despite the extern - there are way too many safe APIs with InternalCall e.g. all Math.* APIs.
  3. Maybe some of the pinvokes can be marked as [RequiresUnsafe(false)] (in case if we decide to introduce that overload)?
  4. Are there any source-generators that could emit LibraryImports? If so - those must be fixed too.

Copilot AI review requested due to automatic review settings March 21, 2026 01:13
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Mar 21, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the ILLink RequiresUnsafe analyzer/code-fix to also cover P/Invoke-like entrypoints declared via [LibraryImport]/extern, and bulk-applies [RequiresUnsafe] annotations to CoreLib interop entrypoints while temporarily suppressing related warnings in CoreLib builds.

Changes:

  • Add a new RequiresUnsafe-related diagnostic (IL5005) for methods with LibraryImportAttribute missing [RequiresUnsafe], plus code-fix support and tests.
  • Annotate numerous CoreLib [LibraryImport] declarations with [RequiresUnsafe] (and add required using System.Diagnostics.CodeAnalysis where needed).
  • Suppress IL5000–IL5005 warnings in System.Private.CoreLib.csproj (Debug/Checked) pending callsite-fixer rollout.

Reviewed changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/tools/illink/test/ILLink.RoslynAnalyzer.Tests/UnsafeMethodMissingRequiresUnsafeTests.cs Adds test coverage for the new LibraryImport/extern-related diagnostic and code fix.
src/tools/illink/src/ILLink.Shared/SharedStrings.resx Adds title/message resources for the new diagnostic.
src/tools/illink/src/ILLink.Shared/DiagnosticId.cs Introduces ExternLibraryImportMissingRequiresUnsafe = 5005.
src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMethodMissingRequiresUnsafeAnalyzer.cs Extends analyzer to report on [LibraryImport] methods missing [RequiresUnsafe].
src/tools/illink/src/ILLink.CodeFix/UnsafeMethodMissingRequiresUnsafeCodeFixProvider.cs Extends code fix provider to handle the new diagnostic id.
src/libraries/System.Private.CoreLib/src/System/Threading/TimerQueue.Browser.cs Adds [RequiresUnsafe] to QCall LibraryImport timer scheduling entrypoint.
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Browser.cs Adds [RequiresUnsafe] to QCall LibraryImport background job scheduling entrypoint.
src/libraries/System.Private.CoreLib/src/System/Threading/Interlocked.cs Adds [RequiresUnsafe] to a QCall LibraryImport entrypoint.
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/TypeMapLazyDictionary.cs Adds [RequiresUnsafe] to QCall LibraryImport typemap lookup entrypoints.
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.Browser.cs Adds [RequiresUnsafe] to Browser QCall LibraryImport entrypoints and required using.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/TraceLogging/XplatEventLogger.cs Adds [RequiresUnsafe] to QCall LibraryImport logging entrypoints.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.Threading.NativeSinks.Internal.cs Adds [RequiresUnsafe] to multiple QCall LibraryImport native sink entrypoints and required using.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/NativeRuntimeEventSource.Exceptions.NativeSinks.Internal.cs Adds [RequiresUnsafe] to QCall LibraryImport exception sink entrypoint and required using.
src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.Internal.cs Adds [RequiresUnsafe] to QCall LibraryImport EventPipe entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Variant.cs Adds [RequiresUnsafe] to QCall LibraryImport COM marshalling entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/TypeLoadException.CoreCLR.cs Adds [RequiresUnsafe] to a QCall LibraryImport message helper and required using.
src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs Adds [RequiresUnsafe] to numerous QCall LibraryImport Thread entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs Adds [RequiresUnsafe] to a QCall LibraryImport monitor entrypoint and required using.
src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs Adds [RequiresUnsafe] to many QCall LibraryImport stub-helper entrypoints.
src/coreclr/System.Private.CoreLib/src/System/String.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport string intern entrypoints.
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport reflection/interop entrypoints.
src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs Adds [RequiresUnsafe] to many QCall LibraryImport runtime handle helpers.
src/coreclr/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyLoadContext.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport ALC entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/TrackerObjectManager.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport tracker entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ObjectiveCMarshal.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport ObjC marshal entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/NativeLibrary.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport NativeLibrary entrypoint and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs Adds [RequiresUnsafe] to many QCall/interop LibraryImport entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Java/JavaMarshal.CoreCLR.cs Adds [RequiresUnsafe] to a QCall LibraryImport Java marshal entrypoint.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport GCHandle entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport ComWrappers entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Runtime/DependentHandle.cs Adds [RequiresUnsafe] to QCall LibraryImport DependentHandle entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/ControlledExecution.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport thread abort/reset entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/VirtualDispatchHelpers.cs Adds [RequiresUnsafe] to QCall LibraryImport virtual dispatch resolver and required using.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/StaticsHelpers.cs Adds [RequiresUnsafe] to a QCall LibraryImport statics helper.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs Adds [RequiresUnsafe] to several QCall LibraryImport RuntimeHelpers entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/GenericsHelpers.cs Adds [RequiresUnsafe] to QCall LibraryImport generic handle helper.
src/coreclr/System.Private.CoreLib/src/System/Reflection/TypeNameResolver.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport resolver entrypoint.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeModule.cs Adds [RequiresUnsafe] to QCall LibraryImport RuntimeModule entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs Adds [RequiresUnsafe] to QCall LibraryImport custom attribute entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs Adds [RequiresUnsafe] to many QCall LibraryImport RuntimeAssembly entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs Adds [RequiresUnsafe] to a QCall LibraryImport current-method helper.
src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs Adds [RequiresUnsafe] to QCall LibraryImport metadata update support query.
src/coreclr/System.Private.CoreLib/src/System/Reflection/LoaderAllocator.cs Adds [RequiresUnsafe] to QCall LibraryImport loader allocator destroy helper.
src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeTypeBuilder.cs Adds [RequiresUnsafe] to numerous QCall LibraryImport Reflection.Emit entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/RuntimeModuleBuilder.cs Adds [RequiresUnsafe] to QCall LibraryImport module builder entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Reflection/Assembly.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport Assembly entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/IO/FileLoadException.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport message helpers and required using.
src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs Adds [RequiresUnsafe] to many QCall LibraryImport GC entrypoints and required using.
src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport exception helpers.
src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport Environment entrypoints.
src/coreclr/System.Private.CoreLib/src/System/Enum.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport enum helper and required using.
src/coreclr/System.Private.CoreLib/src/System/Diagnostics/StackTrace.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport stack trace helper and required using.
src/coreclr/System.Private.CoreLib/src/System/Diagnostics/StackFrame.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport stack frame helper and required using.
src/coreclr/System.Private.CoreLib/src/System/Diagnostics/Debugger.cs Adds [RequiresUnsafe] to QCall LibraryImport debugger helpers and required using.
src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs Adds [RequiresUnsafe] to multiple QCall LibraryImport delegate helpers and required using.
src/coreclr/System.Private.CoreLib/src/System/ComAwareWeakReference.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport COM weak reference helpers and required using.
src/coreclr/System.Private.CoreLib/src/System/CLRConfig.cs Adds [RequiresUnsafe] to QCall LibraryImport config helper and required using.
src/coreclr/System.Private.CoreLib/src/System/ArgIterator.cs Adds [RequiresUnsafe] to QCall LibraryImport ArgIterator entrypoints.
src/coreclr/System.Private.CoreLib/src/System/AppContext.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport handler hook.
src/coreclr/System.Private.CoreLib/src/Internal/VersionResilientHashCode.CoreCLR.cs Adds [RequiresUnsafe] to QCall LibraryImport hashing helper and required using.
src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj Suppresses IL5000–IL5005 in Debug/Checked builds during adoption period.

Copilot AI review requested due to automatic review settings March 21, 2026 02:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 64 out of 64 changed files in this pull request and generated 4 comments.

Copilot AI review requested due to automatic review settings March 21, 2026 02:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 88 out of 88 changed files in this pull request and generated 2 comments.

Copilot AI review requested due to automatic review settings March 21, 2026 02:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comments suppressed due to low confidence (2)

src/tools/illink/src/ILLink.RoslynAnalyzer/UnsafeMethodMissingRequiresUnsafeAnalyzer.cs:1

  • System.Linq is imported but not used in this file. Please remove the unused using to avoid unnecessary dependencies and keep the file tidy.
    src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs:1
  • The generator now forwards [RequiresUnsafe] onto generated stubs. Please add a source-generator unit test that asserts the generated __PInvoke local extern method includes [global::System.Diagnostics.CodeAnalysis.RequiresUnsafe] when the user-authored LibraryImport method is annotated.

Copilot AI review requested due to automatic review settings March 21, 2026 03:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 69 out of 69 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings March 21, 2026 04:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 69 out of 69 changed files in this pull request and generated 2 comments.

@jkoritzinsky
Copy link
Member

Source generators cannot currently emit LibraryImport methods as source generators cannot consume source created by other source generators (at least until dotnet/roslyn#82680 or something like it is approved).

@stephentoub
Copy link
Member

nit: we should re-consider the diagnostic code. An "IL" prefix does not seem correct for this. It's an implementation detail that it's reusing illink infrastructure.

Copilot AI review requested due to automatic review settings March 21, 2026 18:08
@EgorBo EgorBo force-pushed the requires-unsafe-pinvokes-opus branch from e567e8e to 3a517f6 Compare March 21, 2026 18:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 173 out of 173 changed files in this pull request and generated 2 comments.

@EgorBo EgorBo force-pushed the requires-unsafe-pinvokes-opus branch from 3a517f6 to fa8ec81 Compare March 21, 2026 18:39
Copilot AI review requested due to automatic review settings March 21, 2026 18:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 126 out of 127 changed files in this pull request and generated 2 comments.

@EgorBo EgorBo force-pushed the requires-unsafe-pinvokes-opus branch from 4c58afc to 32d502c Compare March 21, 2026 19:13
Copilot AI review requested due to automatic review settings March 21, 2026 19:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 118 out of 119 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings March 21, 2026 20:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 118 out of 119 changed files in this pull request and generated 2 comments.

Comment on lines +85 to +90
foreach (AttributeData? attr in method.GetAttributes ()) {
if (attr.AttributeClass?.HasName ("System.Runtime.CompilerServices.MethodImplAttribute") == true) {
foreach (TypedConstant arg in attr.ConstructorArguments) {
if (arg.Value is int intVal && (intVal & (int)MethodImplOptions.InternalCall) != 0)
return false;
}
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The InternalCall exemption checks only TypedConstant.Value is int. Roslyn can surface MethodImplAttribute ctor arguments as short (legacy ctor) or as the enum type; in those cases this will fail to recognize InternalCall and will incorrectly report IL5005 on InternalCall externs. Consider normalizing the constant to an int (handling short/enum) before doing the bitmask test.

Copilot uses AI. Check for mistakes.
Comment on lines +16 to 20
private static readonly DiagnosticDescriptor s_pointerRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.UnsafeMethodMissingRequiresUnsafe, diagnosticSeverity: DiagnosticSeverity.Info);
private static readonly DiagnosticDescriptor s_externLibraryImportRule = DiagnosticDescriptors.GetDiagnosticDescriptor (DiagnosticId.ExternMethodMissingRequiresUnsafe, diagnosticSeverity: DiagnosticSeverity.Info);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_rule);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => [s_pointerRule, s_externLibraryImportRule];

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces a new diagnostic (IL5005) and expands the code fix to cover it, but there are no analyzer/code-fix tests covering the new extern/LibraryImport scenarios. Please add test cases similar to the existing UnsafeMethodMissingRequiresUnsafeTests to validate both diagnostics and the fixer behavior for [LibraryImport] partial methods and extern methods (including the InternalCall exemption).

Copilot uses AI. Check for mistakes.
@EgorBo
Copy link
Member Author

EgorBo commented Mar 21, 2026

Moved to #125894

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Reflection linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants