Skip to content

Add analyzer+codefix for applying RequiresUnsafe to pointer methods#125196

Merged
agocke merged 24 commits intomainfrom
copilot/duplicate-pr-content-125195
Mar 19, 2026
Merged

Add analyzer+codefix for applying RequiresUnsafe to pointer methods#125196
agocke merged 24 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]
    • Sve.PlatformNotSupported.cs — resolved conflicts from SVE method signature changes (Vector<int>/Vector<uint>Vector<long>/Vector<ulong>), ensured new unsafe pointer methods have [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
    • 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
  • Cleanup: Reverted 43 files that had only unnecessary using System.Diagnostics.CodeAnalysis; additions without any [RequiresUnsafe] usage (leftover from code fixer iterations), and restored UTF-8 BOMs in 20 files that were inadvertently stripped by the fork's editor
  • Scope note: [RequiresUnsafe] on Add, Subtract, AddByteOffset, and SubtractByteOffset methods returning ref T in Unsafe.cs was reverted to keep this PR focused on pointer-based methods; non-pointer RequiresUnsafe changes will be grouped into a separate PR

🔒 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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
@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
@jkotas
Copy link
Member

jkotas commented Mar 14, 2026

The annotations look fine to me.

@agocke
Copy link
Member

agocke commented Mar 17, 2026

@jkotas If you approve this change I'll check in the analyzer/codefixer and use them on the other libraries (after we make RequiresUnsafe match the API review approved this morning)

@jkotas
Copy link
Member

jkotas commented Mar 17, 2026

(I have not reviewed the analyzer and fixer in detail.)

@agocke
Copy link
Member

agocke commented Mar 19, 2026

Merging. Note: analyzer and fixer are currently non-shipping so skipping them is not a problem.

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: Done

Development

Successfully merging this pull request may close these issues.

5 participants