dotnet / dotnet/runtime

[Tracking] Unsafe-v2 migration tooling (analyzers and code fixers)

Open
#131,451 1 comment 0 reactions 1 assignee Claimed by @EgorBo View on GitHub
area-Tools-ILLink
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

Tracking issue for the analyzers and code fixers that help migrate a code base to the [updated memory safety rules](https://github.com/dotnet/csharplang/blob/main/proposals/unsafe-evolution.md) (unsafe-v2).

Migration workflow: https://gist.github.com/EgorBo/1ed6752b3430a181240dcee7dace7070

Design goals carried over from #131002:
- Tooling must be **idempotent**, since migration happens incrementally.
- Code fixers should lean on **existing Roslyn diagnostics** wherever possible, and only add an `ILxxxx` diagnostic where the language deliberately does not require something.
- Everything here is currently **non-shipping** (`#if DEBUG`) and disabled by default.
- Source-Generators shoud not break compilation for unsafe-v1

Legend: 🔍 analyzer, 🔧 code fixer, ⚙️ source generator.

## 1. Declaring contracts on members

- [x] 🔧 **`AddUnsafeToExternCodeFixProvider`**: fixes `CS9389` by marking an `extern` member `unsafe`. (#131002)
- [x] 🔧 **`AddUnsafeToFieldCodeFixProvider`**: fixes `CS9392` for fields, field-backed properties and field-like events in explicit/extended-layout types. (#131002)
- [x] 🔍 **`PointerSignatureRequiresUnsafeAnalyzer` (`IL5006`)**
- [x] 🔧 **`AddUnsafeToPointerSignatureCodeFixProvider`**: pointer and function-pointer signatures that lost the caller-unsafe contract they had under unsafe-v1. (#131002)
- [x] 🔍 **`LibraryImportRequiresExplicitSafetyAnalyzer` (`IL5007`)**
- [x] 🔧 **`AddUnsafeToLibraryImportCodeFixProvider`**: `[LibraryImport]` methods with no explicit contract. (#131245)

## 2. Unsafe contexts at call sites

Highest volume by far, since every *consumer* of a newly-`unsafe` API breaks.

- [x] 🔧 **`IntroduceUnsafeContextCodeFixProvider`**: fixes `CS9360`, `CS9361`, `CS9362`, `CS9363` and `CS9376`. Prefers `unsafe { }` around the statement, splits a local declaration into `T x; unsafe { x = init; }` so the local stays visible, and falls back to `unsafe(...)` where a block would be invalid syntax or would shorten a scope. A body that would need three or more separate regions is wrapped whole when every use site is fixed at once. (#131581)

## 3. Contract consistency

- [x] 🔧 **`SynchronizeUnsafeContractCodeFixProvider`**: fixes `CS9364`, `CS9365` and `CS9366` (derived is `unsafe`, base is safe) with two actions: mark the base member `unsafe` when it is in source, or drop `unsafe` from the derived member. https://github.com/dotnet/runtime/pull/131454
- [x] 🔧 **`MatchPartialSafetyModifierCodeFixProvider`**: fixes `CS0764` and `CS9390` by copying the safety modifier to the other part. Relevant because source generators author one half of those partials. https://github.com/dotnet/runtime/pull/131454
- [ ] 🔍 [TBD] **`UnsafeContractNotPropagatedAnalyzer` (`IL5008`)**: an override or interface implementation left unannotated while the member it overrides or implements is `unsafe`. Needs its own diagnostic because narrowing to safe is *legal*, so the compiler is silent and migration silently drops the obligation.
- [ ] 🔧 [TBD] **`PropagateUnsafeContractCodeFixProvider`**: fixes `IL5008` by adding `unsafe`.

## 4. Cleaning up legacy `unsafe`

- [x] 🔧 **`RemoveInvalidUnsafeCodeFixProvider`**: fixes `CS9377` and unsafe-specific `CS0106` by removing an `unsafe` modifier that is now meaningless or invalid, for example on a type declaration. (#131002)
- [x] 🔍 **`UnsafeMemberMissingSafetyDocumentationAnalyzer` (`IL5005`)**
- [x] 🔧 **`RemoveUndocumentedUnsafeCodeFixProvider`**: removes an undocumented `unsafe` modifier when it was an unsafe-v1 lexical scope rather than an intentional caller-unsafe contract. (#131002)

## 5. Safety documentation

`IL5005` covers signatures, but nothing covers bodies and nothing covers `safe`. The speclet recommends Rust-style `// SAFETY` comments, and [LDM 2026-05-27](https://github.com/dotnet/csharplang/blob/main/meetings/2026/LDM-2026-05-27.md#safety-comments) left "compiler or analyzer?" open.

- [x] 🔍 **`UnsafeBlockMissingSafetyCommentAnalyzer` (`IL5009`)**: an `unsafe { }` block or `unsafe(...)` expression with no `// SAFETY:` comment. https://github.com/dotnet/runtime/pull/131484
- [x] 🔧 **`AddSafetyCommentCodeFixProvider`**: fixes `IL5009` by inserting a `// SAFETY: TODO` stub, matching the `/* SAFETY: Audit */` convention already used by the other fixers. https://github.com/dotnet/runtime/pull/131484
- [x] 🔍 **`SafeModifierMissingJustificationAnalyzer` (`IL5010`)**: an explicit `safe` modifier with no `` documentation. This is the symmetric hole to `IL5005`, since `safe` is a hand-written assertion the compiler cannot verify. It applies wherever `safe` can appear: `extern` members and `[LibraryImport]` methods, fields and field-backed members in `[StructLayout(LayoutKind.Explicit)]` and `[ExtendedLayout]` types (where `safe` asserts the overlap cannot be used to type-pun, for example aliasing a reference with an integer), and any other declaration once [dotnet/roslyn#84602](https://github.com/dotnet/roslyn/pull/84602) allows `safe` as a no-op. No fixer, a human must write the justification. https://github.com/dotnet/runtime/pull/131484

## 6. Source generator output

The common root cause for the unfixed generators below is that every generated body relied on an `unsafe` modifier stamped onto the generated **type** declaration. Under unsafe-v2 a type modifier establishes no unsafe context for the members inside it, so generated bodies that dereference pointers need explicit `unsafe` blocks instead. (The type modifier itself is only a suppressed warning, `CS9377`, and has to stay: under unsafe-v1 it is what makes a pointer legal to name in a stub whose modifiers are copied from a user declaration carrying `unsafe` on the type rather than on the member.)

- [x] ⚙️ **`LibraryImportGenerator`**: requires an explicit contract on the user-facing method (`SYSLIB1064`), keeps the hidden `__PInvoke` caller-unsafe, and wraps generated bodies in explicit `unsafe` blocks instead of relying on a type-level modifier. (#131245)
- [x] ⚙️ **`ComInterfaceGenerator` and `ComClassGenerator`**: generated bodies now open their own `unsafe` blocks, and a shadowing member forwarding to a caller-unsafe base member is emitted `unsafe` with a block body so it does not hit `CS9366`/`CS9362`. `[GeneratedComInterface]` and `[GeneratedComClass]` are public API shipping since .NET 8, so this was real user impact. The open design question is untouched: whether `[GeneratedComInterface]` methods should also require an explicit `safe`/`unsafe` contract the way `[LibraryImport]` methods now do. (#131701)
- [x] ⚙️ **`JSImportGenerator` and `JSExportGenerator`**: generated bodies now open their own `unsafe` blocks, and the generated `__GeneratedInitializer` class drops its `unsafe` modifier since none of its members names a pointer type. The `[JSImport]` bodies name no pointer type today but are wrapped anyway, because they call `Unsafe.SkipInit`, which #126956 makes caller-unsafe. (#131701)
- [x] ⚙️ **`VtableIndexStubGenerator`**: `PopulateUnmanagedVirtualMethodTable` wraps its body. `[VirtualMethodIndex]` is a test asset rather than public API, so there is no external impact. (#131701)

Verified clean under `updated-memory-safety-rules`, no work needed: `RegexGenerator` (emits `[SkipLocalsInit]` but no `stackalloc`, so the tightened `stackalloc` rule does not apply), `JsonSourceGenerator`, and the `Microsoft.Extensions.*` and `System.Private.CoreLib` generators.

## Blocked / external

- [x] [dotnet/roslyn#84602](https://github.com/dotnet/roslyn/pull/84602): allow `safe` on non-`extern` members. Until it merges, `safe` cannot be spelled on a `[LibraryImport]` whose generated implementation is a wrapper.
- [ ] [dotnet/roslyn#84616](https://github.com/dotnet/roslyn/pull/84616): allow `await` in an `unsafe` context. Gates how aggressively the call-site fixer can use `unsafe` blocks rather than `unsafe(...)` expressions in async code.
- [ ] [dotnet/roslyn#82546](https://github.com/dotnet/roslyn/issues/82546): public API for the memory-safety-rules version. Until then, tooling detects the opt-in through the `updated-memory-safety-rules` feature flag.
- [ ] **GenAPI does not round-trip the safety contract.** It infers `unsafe` in reference source from pointer types in the signature rather than from the declaration, so a hand-removed `unsafe` or an added `safe` is lost on the next regeneration. Harmless before the opt-in, since `unsafe` has no metadata representation and nothing ApiCompat compares can disagree. Once an assembly opts in, `unsafe` becomes `RequiresUnsafeAttribute` in metadata and the reference assembly has to agree with the implementation about which members are caller-unsafe, so GenAPI needs to emit the modifier from metadata and ApiCompat needs to compare it. GenAPI lives in [dotnet/dotnet](https://github.com/dotnet/dotnet).
- [ ] **StyleCop SA1206 does not know the `safe` contextual keyword** and asks for it ahead of the accessibility modifier, which is not how it orders `unsafe`. Disabled repo-wide in `eng/CodeAnalysis.src.globalconfig` as a stopgap (#131719); needs a fix in [DotNetAnalyzers/StyleCopAnalyzers](https://github.com/DotNetAnalyzers/StyleCopAnalyzers) before the rule can be turned back on.
- [ ] **ApiCompat does not compare the safety contract.** A reference assembly compiled with the updated rules carries `MemorySafetyRulesAttribute` and emits `RequiresUnsafeAttribute` for its `unsafe` members, while an implementation assembly still on the legacy rules emits neither, so the two disagree in metadata about which members are caller-unsafe. Whether ApiCompat currently objects to that divergence is unverified. It should compare the contract, so that a reference assembly cannot silently promise a contract the implementation does not keep, or drop one it does.

## Diagnostic ID map

| ID | Owner | Meaning |
|---|---|---|
| `CS9360`-`CS9363`, `CS9376` | Roslyn | An unsafe context is required at this use site |
| `CS9364`-`CS9366` | Roslyn | An `unsafe` member cannot override or implement a safe member |
| `CS9377`, `CS0106` | Roslyn | `unsafe` is meaningless or invalid here |
| `CS9388`, `CS9389`, `CS9392` | Roslyn | Explicit `safe`/`unsafe` required, or `safe` used in an invalid position |
| `CS0764`, `CS9390` | Roslyn | Partial declarations disagree on `unsafe` or `safe` |
| `SYSLIB1064` | This effort | `[LibraryImport]` method must be marked `safe` or `unsafe` |
| `IL5005` | This effort | `unsafe` member has no `` documentation |
| `IL5006` | This effort | Pointer signature is missing `unsafe` |
| `IL5007` | This effort | `[LibraryImport]` has no explicit contract |
| `IL5008` | *proposed* | Unsafe contract not propagated to override or implementation |
| `IL5009` | *proposed* | `unsafe` block has no `// SAFETY:` comment |
| `IL5010` | *proposed* | Explicit `safe` modifier has no justification |

Note on testing: the interop generator test harnesses compile with `LanguageVersion.Preview`, which already relaxes the pointer-declaration rule *without* the feature flag. A generator change that breaks unsafe-v1 still passes the entire existing suite, so output has to be verified out-of-band with `LangVersion=latest`.

> [!NOTE]
> The GenAPI, StyleCop, and ApiCompat items under "Blocked / external" were added with GitHub Copilot.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.