Skip to content

Use safe writes in Xoshiro NextBytes#127410

Draft
EgorBo wants to merge 1 commit intodotnet:mainfrom
EgorBo:safe-span-random-xoshiro
Draft

Use safe writes in Xoshiro NextBytes#127410
EgorBo wants to merge 1 commit intodotnet:mainfrom
EgorBo:safe-span-random-xoshiro

Conversation

@EgorBo
Copy link
Copy Markdown
Member

@EgorBo EgorBo commented Apr 24, 2026

Note

This PR description was generated by GitHub Copilot.

Summary

  • Rewrite xoshiro Random.NextBytes bulk writes to use BitConverter.TryWriteBytes
  • Replace the unsafe partial-tail byte-copy path with a collection-literal scratch span and BitConverter.TryWriteBytes
  • Remove the now-unneeded unsafe method declarations and MemoryMarshal using directives

Validation

  • build.cmd clr.corelib+clr.nativecorelib+libs.pretest -rc checked
  • dotnet.cmd build /t:test src\libraries\System.Runtime\tests\System.Runtime.Extensions.Tests\System.Runtime.Extensions.Tests.csproj -p:XUnitClassName=System.Tests.RandomTests -p:TargetFramework=net11.0-windows -v:minimal (121 tests, 0 failed)

Rewrite the bulk and tail byte writes in the xoshiro Random implementations to use BitConverter and span copies instead of unsafe pointer access or MemoryMarshal.Write.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
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

This PR refactors the Xoshiro-based Random.NextBytes implementations in System.Private.CoreLib to avoid unsafe code by switching bulk and tail writes to BitConverter.TryWriteBytes.

Changes:

  • Replace MemoryMarshal.Write bulk writes with BitConverter.TryWriteBytes and add Debug.Assert for the expected success path.
  • Replace unsafe tail byte-copy logic with a scratch span plus TryWriteBytes, removing the need for unsafe.
  • Remove the System.Runtime.InteropServices using directives made unnecessary by the refactor.

Reviewed changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro256StarStarImpl.cs Refactors NextBytes(Span<byte>) to use TryWriteBytes for bulk and tail writes, removing unsafe pointer usage.
src/libraries/System.Private.CoreLib/src/System/Random.Xoshiro128StarStarImpl.cs Same refactor pattern for the 32-bit Xoshiro implementation’s NextBytes(Span<byte>).

@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented Apr 24, 2026

@MihuBot

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants