Skip to content

Remove unsafe code from BigInteger.TryWriteBigEndian and TryWriteLittleEndian#124858

Draft
Copilot wants to merge 8 commits intomainfrom
copilot/remove-unsafe-code-big-integer
Draft

Remove unsafe code from BigInteger.TryWriteBigEndian and TryWriteLittleEndian#124858
Copilot wants to merge 8 commits intomainfrom
copilot/remove-unsafe-code-big-integer

Conversation

Copy link
Contributor

Copilot AI commented Feb 25, 2026

Replace Unsafe.WriteUnaligned/pointer-arithmetic patterns in BigInteger.TryWriteBigEndian and TryWriteLittleEndian with safe, idiomatic equivalents using BinaryPrimitives.

Description

bits is null path — replace manual endianness-conditional Unsafe.WriteUnaligned with BinaryPrimitives:

// Before
int value = BitConverter.IsLittleEndian ? BinaryPrimitives.ReverseEndianness(_sign) : _sign;
Unsafe.WriteUnaligned(ref MemoryMarshal.GetReference(destination), value);

// After
BinaryPrimitives.WriteInt32BigEndian(destination, _sign);

_bits positive path — replace ref byte pointer walking with BinaryPrimitives.WriteUInt32BigEndian/WriteUInt32LittleEndian using forward loops. For big endian, word i (LSW-first) is written at destination.Slice((bits.Length - 1 - i) * sizeof(uint)):

// Before
ref byte address = ref Unsafe.Add(ref MemoryMarshal.GetReference(destination), (bits.Length - 1) * sizeof(uint));
for (int i = 0; i < bits.Length; i++)
{
    Unsafe.WriteUnaligned(ref address, BinaryPrimitives.ReverseEndianness(bits[i]));
    address = ref Unsafe.Subtract(ref address, sizeof(uint));
}

// After (big endian)
for (int i = 0; i < bits.Length; i++)
{
    BinaryPrimitives.WriteUInt32BigEndian(destination.Slice((bits.Length - 1 - i) * sizeof(uint)), bits[i]);
}

_bits negative path — for little endian, a sliding Span<byte> advancing by sizeof(uint) each iteration is used. For big endian, the write offset is computed directly from the loop index as byteCount - (i + 1) * sizeof(uint), avoiding a decrementing offset variable. The original do-while/while carry propagation structure is preserved (avoids per-iteration carry branch after carry stops propagating):

// big endian negative path
BinaryPrimitives.WriteUInt32BigEndian(destination.Slice(byteCount - (i + 1) * sizeof(uint)), part);

Sign-extension detection — replace Unsafe.AreSame pointer comparison with a simple integer check:

// Before
if (Unsafe.AreSame(ref address, ref startAddress)) { Unsafe.WriteUnaligned(ref address, uint.MaxValue); }

// After
if (byteCount > bits.Length * sizeof(uint)) { BinaryPrimitives.WriteUInt32BigEndian(destination, uint.MaxValue); }

Structural improvements:

  • Early return pattern for insufficient destination buffer (reduces nesting)
  • Slice(offset) without explicit length to minimize redundant bounds checking (BinaryPrimitives validates length ≥ 4 internally)

Unsafe, MemoryMarshal.Cast, and BitConverter are no longer used in either method. All loops are forward loops. All existing tests (2647) pass.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@dotnet-policy-service
Copy link
Contributor

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

Copilot AI and others added 2 commits February 25, 2026 14:45
… BigInteger

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
…ndly loops

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove unsafe code from TryWrite methods in BigInteger Remove unsafe code from BigInteger.TryWriteBigEndian and TryWriteLittleEndian Feb 25, 2026
…/LittleEndian

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
…m index

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
@EgorBo
Copy link
Member

EgorBo commented Feb 25, 2026

@copilot please create microbenchmarks using the performance-benchmarks skill to validate performance of these methods (different values, lengths, etc).

This comment was marked as outdated.

Copy link
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 TryWriteBigEndian and TryWriteLittleEndian methods in BigInteger to replace unsafe code patterns (Unsafe, MemoryMarshal.Cast, BitConverter) with BinaryPrimitives APIs and use forward loops throughout instead of reversed loops.

Changes:

  • Replaced pointer-based writing with BinaryPrimitives.WriteInt32BigEndian/LittleEndian and BinaryPrimitives.WriteUInt32BigEndian/LittleEndian
  • Restructured control flow with early return pattern for insufficient destination buffer
  • Simplified loop structures to use forward iteration with offset calculations for reverse ordering where needed

Co-authored-by: EgorBo <523221+EgorBo@users.noreply.github.com>
@EgorBo

This comment was marked as resolved.

1 similar comment
@EgorBo

This comment was marked as resolved.

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants