Skip to content

Add analyzer+codefix for unsafe methods missing [RequiresUnsafe]#125196

Draft
Copilot wants to merge 18 commits intomainfrom
copilot/duplicate-pr-content-125195
Draft

Add analyzer+codefix for unsafe methods missing [RequiresUnsafe]#125196
Copilot wants to merge 18 commits intomainfrom
copilot/duplicate-pr-content-125195

Conversation

Copy link
Contributor

Copilot AI commented Mar 4, 2026

Description

Cherry-pick of PR #125195 (agocke/runtime). Adds a Roslyn analyzer and code fixer that warns on methods with the unsafe modifier but missing [RequiresUnsafe], and applies [RequiresUnsafe] to all unsafe methods in System.Private.CoreLib.

  • Analyzer (UnsafeMethodMissingRequiresUnsafeAnalyzer): Reports IL2900 on unsafe methods lacking [RequiresUnsafe]
  • Code fixer (UnsafeMethodMissingRequiresUnsafeCodeFixProvider): Adds [RequiresUnsafe] attribute automatically
  • Attribute application: 276 files across CoreCLR and shared CoreLib annotated with [RequiresUnsafe]
  • Merge conflict resolution: MdImport.cs — kept current LibraryImport/partial signature (diverged from fork's MethodImpl/extern), added [RequiresUnsafe]
  • Corner case exclusions:
    • Reverted [RequiresUnsafe] from IntPtr.cs and UIntPtr.cs — pointers in those files do not imply RequiresUnsafe
    • Removed [RequiresUnsafe] from Add<T>(void*, int) and Subtract<T>(void*, int) in Unsafe.cs — these methods do not dereference the pointer
    • Added [RequiresUnsafe] to all Add, Subtract, AddByteOffset, and SubtractByteOffset methods returning ref T in Unsafe.cs — these perform pointer arithmetic on references
    • Removed [RequiresUnsafe] from Alloc and AllocZeroed in NativeMemory.cs and NativeMemory.Unix.cs — returning a pointer is not inherently unsafe, only dereferencing it would be

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Add a new DiagnosticAnalyzer (UnsafeMethodMissingRequiresUnsafeAnalyzer)
that warns when a method, constructor, or local function has the 'unsafe'
modifier but is not annotated with [RequiresUnsafe].

Add a matching CodeFixProvider that adds the [RequiresUnsafe] attribute
to the flagged declaration.

Both are #if DEBUG guarded and enabled via the existing
EnableUnsafeAnalyzer MSBuild property.

New diagnostic: IL5004 (UnsafeMethodMissingRequiresUnsafe)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI changed the title [WIP] Add exact contents from PR 125195 Add analyzer+codefix for unsafe methods missing [RequiresUnsafe] Mar 4, 2026
@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Mar 4, 2026
agocke added 2 commits March 5, 2026 14:44
Now based on methods with pointer types, rather than methods with .
agocke and others added 4 commits March 5, 2026 16:10
…r-content-125195

# Conflicts:
#	src/libraries/Common/src/Interop/Unix/System.Native/Interop.Futex.cs
#	src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelFutex.Unix.cs
- Remove lambda/anonymous method break in RequiresUnsafeAnalyzer so
  unsafe context flows through nested lambdas (matching C# semantics)
- Add FieldDeclarationSyntax to IsInRequiresScope for unsafe field
  initializers
- Remove [RequiresUnsafe] from files compiled outside CoreLib (Common/,
  nativeaot/Runtime.Base/, Resources/) where the attribute is unavailable
- Add tests for lambda-in-unsafe-method, anonymous delegate, and field
  initializer scenarios

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the DiagnosticSeverity.Info overload so IL5004 shows as a
suggestion rather than a warning/error in builds. Update tests to
expect Info severity.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@agocke agocke force-pushed the copilot/duplicate-pr-content-125195 branch from c08c9a1 to a6490f4 Compare March 6, 2026 18:35
agocke added 3 commits March 6, 2026 10:50
…r-content-125195

# Conflicts:
#	src/coreclr/System.Private.CoreLib/src/System/AppContext.CoreCLR.cs
#	src/coreclr/System.Private.CoreLib/src/System/StartupHookProvider.CoreCLR.cs
Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…nsafe.cs

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: agocke <515774+agocke@users.noreply.github.com>
…returning ref T

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
@agocke
Copy link
Member

agocke commented Mar 9, 2026

Found some more over-attributions. And I used copilot to scan the remaining and it believes there are another 60 methods that should be cleaned up. I'm still reviewing but this is interesting data. It suggests ~3% of all pointer methods are actually safe.

That's not a lot, but it does mean that in a set of hundreds of methods, you will find a dozen or so that are safe.

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

Labels

area-Infrastructure-libraries linkable-framework Issues associated with delivering a linker friendly framework

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants