Enroll System.IO.Compression and its tests in unsafe-v2 by EgorBo · Pull Request #136657 · dotnet/runtime

EgorBo · 2026-10-29T19:55:14Z

I chose System.IO.Compression (and its tests) as the first target because it's relatively small and interop-heavy.

This PR covers only the mechanical migration: there is no AI-driven work to fill in safety comments or decide which methods might need a caller-unsafe contract. This lets the review focus entirely on the code fixer output.

References

Unsafe-v2 spec: link
Order of operations: link (tracking issue)
Driver script (runs the fixers in the correct order): link

A prompt for reading all the relevant documentation and running the driver script:

Read https://gist.githubusercontent.com/EgorBo/271abeeac2bfb5a34013e4562507062e/raw/unsafe-v2-migration-agent-prompt.md in full
and follow it for **System.IO.Compression** and its tests. Run the driver once per project.

(I also tell it where to find a prebuilt dotnet/runtime in WSL so it can handle Linux targets there.)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Unmodified output of the migration driver across net12.0-windows, net12.0-unix, net12.0-browser, net12.0-wasi, and net12.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Unmodified output of the fixed migration driver across net12.0-windows, net12.0-unix, and net12.0-browser. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Enable the two affected consumer test projects to compile the migrated shared helper. Neither project is enrolled in unsafe-v2. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

azure-pipelines · 2026-10-29T19:56:04Z

Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

dotnet-policy-service · 2026-10-29T19:56:37Z

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

EgorBo · 2026-10-29T20:07:15Z

    internal static unsafe void GetCryptographicallySecureRandomBytes(byte* buffer, int length)
    {
-        if (Sys.GetCryptographicallySecureRandomBytes(buffer, length) != 0)
+        if (/* SAFETY: To be audited */ unsafe(Sys.GetCryptographicallySecureRandomBytes(buffer, length)) != 0)


hm.. this one probably should've been unsafe{}, will check.

Try all valid block candidates before unsafe expressions, including expression-body conversions. Keep expressions as a fallback and preserve the explicit expression-first action. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Retain unsafe-v2 enrollment and approved consumer settings. Restore only the previous automatic migration output before regenerating contexts with the updated fixer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Untouched output of a full driver replay from pre-migration sources across all five library TFMs. Control-flow headers and convertible expression bodies now prefer unsafe blocks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Untouched output of the fixed migration driver across all three test TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

agocke · 2026-10-29T21:13:53Z

            }
        }

-        public unsafe nuint SetPrefix(ReadOnlyMemory<byte> prefix)


A bit surprised about this change. I would have thought that we would leave unsafe on a method.

@agocke The code fixer is not supposed to infer whether internal unsafety should be propagated to callers. We make an exception for signatures containing unmanaged pointers, because experience shows that, in 99% of cases, methods accepting unmanaged pointers end up dereferencing them.

In general, though, this is the responsibility of a human or an AI agent:

Find all unsafe {} or unsafe(...) contexts marked with // SAFETY: To be audited.

Fill in the safety comment.

Decide whether the method's internal unsafety should be propagated to its callers (based on ...).

If it should, propagate it and add a /// <safety> comment.

Update all call sites (the code fixer can be rerun for this step).

So the code fixer, whether invoked by a human or an AI agent, handles 90% of the heavy lifting. AI then does the more nuanced work. This could all be done in one pass, but I'm not sure I want that for the libraries. I want to keep mechanical changes that don't need careful review separate from those that do.

NOTE: unsafe on a method is always kept by the code-fixer if the method:

Contains unmanaged pointers in the sig - purely speculation

Contains /// <safety> comment - it means it was already audited.

Otherwise we assumed unsafe is just a legacy unsafe-v1 "method-wide-unsafe-context" that we can remove.

The code fixer is not supposed to infer whether internal unsafety should be propagated to callers.

Isn't this assuming the opposite though? That any given unsafe method is actually only inner-unsafe?

I think this could work, but I want to understand the reasoning why we are pushing things down to inner unsafe. Just because it follows unsafe-v1 semantics?

Isn't this assuming the opposite though? That any given unsafe method is actually only inner-unsafe?

@agocke I am not sure it can be done the other way. Currently in v1 unsafe is used for inner-unsafe and we convert it into inner-unsafe in v2. If we convert unsafe into caller-unsafe, we'll get an enormous cascade of unsafe contexts - at this point the output of the migration tooling will be instantly reverted. Say unsafe void Dispose() are you suggesting it should propagated into IDisposable.Dispose interface? (same for any interface/virtual/overriden method. Also, a gazillion of classes marked as unsafe, does it mean all methods are unsafe in v2? or lack of unsafe on them means they're fully caller-safe?

Also, this effectively changes Public API and should go through Public API review, while with the current workflow it won't.

Currently, the only speculation we allow ourself is that method accepting unmanaged pointers should be marked as caller-unsafe (and they were effectively caller-unsafe even in unsafe-v1).

EgorBo · 2026-10-29T22:41:06Z

PTAL @agocke @jkotas @jjonescz. It contains a small fix (and tests) for UnsafeContext injector (to prioritize statements over expressions).

Tracking issue: #136664
Here is the explanation why this PR doesn't do the full migration and only the mechanical: #136657 (comment)

I don't expect a PR per assembly, I think many of them could be batched. (say up to 2000 LOC of changes or more).
PS: I recommend disabling whitespaces.

EgorBo · 2026-10-30T17:55:16Z

NOTE: there are no functional changes here, it's really about agreeing on the format.

rzikm · 2026-11-02T10:03:23Z

@EgorBo are we as IO.Compression owners expected to review this? If so, what should we be looking for?

EgorBo · 2026-11-02T16:02:48Z

@EgorBo are we as IO.Compression owners expected to review this? If so, what should we be looking for?

I'm not sure, @jkotas requested the review. I assume for correctness issues?

jkotas · 2026-11-02T16:57:02Z

I'm not sure, @jkotas requested the review. I assume for correctness issues?

I have requested the review for awareness - to make sure that the area owners are comfortable with assuming the ownership of these annotations.

rzikm · 2026-11-02T17:16:02Z

I see, and I assume once this is merged, it is up to us to fill in those security annotations? Is there some document/playbook summarizing the approach? I may have gaps in how unsafe-v2 is supposed to work so I could use some examples to work from.

EgorBo · 2026-11-02T17:54:34Z

I see, and I assume once this is merged, it is up to us to fill in those security annotations? Is there some document/playbook summarizing the approach? I may have gaps in how unsafe-v2 is supposed to work so I could use some examples to work from.

unsafe-v2 spec: https://github.com/dotnet/csharplang/blob/main/proposals/unsafe-evolution.md

The clean up work can be done by code owners or we'll just send an AI PR that you can review. The work is:

Inspect every inner unsafe{} (simple grep over // SAFETY: To be audited) whether the containing function has to be marked as caller-unsafe or not (propagation of unsafe).
Replace the To be audited with a proper explanation why it's safe to call an unsafe-API/unsafe operation here.
Replace unsafe with safe where explicit unsafe/safe is required, currently it is all extern/LibraryImport and fields in ExplicitLayout/ExtendedLayout (or/and Pack=1). E.g.

        // Compression level functions
        [LibraryImport(Libraries.CompressionNative)]
        internal static unsafe partial int ZSTD_minCLevel();

looks like a perfect candidate for unsafe to be replaced with safe here. In that case, a /// <safety>... comment must be added to explain why it's safe.

Optional: replace unsafe{} with a code that doesn't require it.

rzikm

LGTM from IO.Compression perspective, I will raise the follow-up PR with the required annotations.

EgorBo added 5 commits October 29, 2026 19:35

Enroll System.IO.Compression in unsafe-v2

b36ce68

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

Enroll System.IO.Compression tests in unsafe-v2

c6ffcba

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

github-actions Bot added the area-System.IO.Compression label Oct 29, 2026

dotnet-policy-service Bot assigned EgorBo Oct 29, 2026

EgorBo commented Oct 29, 2026

View reviewed changes

Comment thread src/libraries/Common/src/Interop/Interop.Zstd.cs Resolved

EgorBo added 4 commits October 29, 2026 21:38

Regenerate Compression test migration from baseline

9d14fd1

Untouched output of the fixed migration driver across all three test TFMs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5d9d8def-16fe-4105-9e0f-b8aad187abce

agocke reviewed Oct 29, 2026

View reviewed changes

EgorBo requested a review from sbomer as a code owner October 29, 2026 21:20

dotnet-policy-service Bot added linkable-framework Issues associated with delivering a linker friendly framework labels Oct 29, 2026

EgorBo mentioned this pull request Oct 29, 2026

Enroll libraries in unsafe-v2 #136664

Open

jkotas requested a review from a team October 29, 2026 23:35

build-analysis Bot mentioned this pull request Oct 30, 2026

[browser] Wasm.Build.Tests timeout - Timed out after 10s waiting for 'WASM EXIT' #116697

Open

EgorBo mentioned this pull request Oct 30, 2026

Enroll the Microsoft.* libraries and their tests in unsafe-v2 (net11.0+) #136720

Open

rzikm approved these changes Nov 3, 2026

View reviewed changes

Conversation

EgorBo commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Uh oh!

azure-pipelines Bot commented Oct 29, 2026

Uh oh!

dotnet-policy-service Bot commented Oct 29, 2026

Uh oh!

EgorBo Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

agocke Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

agocke Nov 2, 2026

Choose a reason for hiding this comment

Uh oh!

EgorBo Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EgorBo commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EgorBo commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzikm commented Nov 2, 2026

Uh oh!

EgorBo commented Nov 2, 2026

Uh oh!

jkotas commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzikm commented Nov 2, 2026

Uh oh!

EgorBo commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rzikm left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EgorBo commented Oct 29, 2026 •

edited

Loading

EgorBo Oct 29, 2026 •

edited

Loading

EgorBo Oct 29, 2026 •

edited

Loading

EgorBo Oct 29, 2026 •

edited

Loading

EgorBo Nov 2, 2026 •

edited

Loading

EgorBo commented Oct 29, 2026 •

edited

Loading

EgorBo commented Oct 30, 2026 •

edited

Loading

jkotas commented Nov 2, 2026 •

edited

Loading

EgorBo commented Nov 2, 2026 •

edited

Loading