Cysharp / Cysharp/MemoryPack

Avoid assuming BCL private field layouts via Unsafe.As (List<T>, Stack<T>, BitArray, ImmutableArray<T>)

Open
#460 0 comments 0 reactions 1 assignee View on GitHub

@aetos382 is already working on this.

Since Sep 17, 2026.

Dominant language
C#
Stars
4.7k
Forks
313
PR merge metrics
No merged PRs in 30d

Description

Summary

Several places in MemoryPack.Core read and write private fields of BCL types by declaring view types with an assumed field layout and reinterpreting instances with Unsafe.As. Unlike the equivalent code in other Cysharp repositories, some of this is not a polyfill for old targets — it is on the modern .NET path. I would like to propose moving away from this technique.

Affected code

1. Internal/CollectionsMarshalEx.cs (#if NET7_0_OR_GREATER) — https://github.com/Cysharp/MemoryPack/blob/main/src/MemoryPack.Core/Internal/CollectionsMarshalEx.cs

ref var view = ref Unsafe.As<List<T?>, ListView<T?>>(ref list);
view._size = length;
return view._items.AsSpan(0, length);
ref var view = ref Unsafe.As<Stack<T?>, StackView<T?>>(ref stack);
view._size = length;

The file already acknowledges the coupling:

// NOTE: These structure depndent on .NET 7, if changed, require to keep same structure.

2. Formatters/BitArrayFormatter.cshttps://github.com/Cysharp/MemoryPack/blob/main/src/MemoryPack.Core/Formatters/BitArrayFormatter.cs

ref var view = ref Unsafe.As<BitArray, BitArrayView>(ref value);
writer.WriteUnmanagedWithObjectHeader(2, view.m_length);
writer.WriteUnmanagedArray(view.m_array);

BitArrayView assumes m_array, m_length, _version in that order — and deserialization writes back into view.m_array.

3. Formatters/ImmutableCollectionFormatters.cs — the pre-.NET 8 path:

#if NET8_0_OR_GREATER
    value = ImmutableCollectionsMarshal.AsImmutableArray(array);
#else
    // create Empty and replace inner T[] field(avoid defensive copy of Create)
    value = ImmutableArray.Create<T?>();
    ref var view = ref Unsafe.As<ImmutableArray<T?>, ImmutableArrayView<T?>>(ref value);
    view.array = array;
#endif

This one is the good precedent: once ImmutableCollectionsMarshal shipped, the hack was replaced by the official API on the newer target. I would like to see the same done for the rest.

Why this is risky

  1. No contract. The field layout of List<T>, Stack<T>, BitArray, and ImmutableArray<T> is an implementation detail. Nothing prevents the BCL from reordering, renaming, or adding fields, and BitArray's m_* names date back to .NET Framework — there is no guarantee they stay.
  2. Writing makes the failure mode worse. view._size = length and view.m_array = ... mutate live BCL objects at guessed offsets. A mismatch does not read garbage, it writes garbage, and the damage surfaces far from here.
  3. It fails silently. Unsafe.As performs no type check. A layout change produces heap corruption or a GC crash rather than a clean exception — the worst possible failure mode for a serializer, since it can also turn malformed input into memory corruption.
  4. It is not confined to legacy targets. CollectionsMarshalEx is #if NET7_0_OR_GREATER, so this ships on modern .NET, not just in a shim for Unity or .NET Framework.
  5. IL2CPP is not bound by CoreCLR's layout rules. ZLinq's equivalent code carries the comment // Unsafe.As<>._size is failed in Unity so don't use it., which suggests the technique has already broken in practice on Unity.

Suggested direction

  • List<T>: CollectionsMarshal.SetCount has been available since .NET 8 and does exactly what CreateSpan does, with runtime support. Since .NET 7 is out of support (EOL 2024-05-14), raising the net7.0 TFM to net8.0 would let the List<T> part be deleted outright.
  • Stack<T>: there is no public equivalent, so this is the one case that needs a design decision. Serializing via stack.ToArray() / reconstructing via the IEnumerable<T> constructor is correct but allocates; an internally owned stack type would avoid both the allocation and the layout assumption.
  • BitArray: this can be done entirely with public API. BitArray.CopyTo(int[], 0) fills the same int[] words that m_array holds, and new BitArray(int[] values) followed by setting Length reconstructs it exactly. That removes BitArrayView with no format change.
  • ImmutableArray<T>: ImmutableCollectionsMarshal.AsImmutableArray is also available on netstandard2.0 through the System.Collections.Immutable package (8.0+), so the #else branch may be removable by raising the package floor rather than keeping the view type.
  • If any of these must stay for now, adding a layout self-check at static initialization (resolve the field by name via reflection, confirm its type, fall back to a safe path otherwise) would at least turn a future layout change into a slow path rather than silent corruption.

Note

I found this while auditing R3, and the same or similar pattern exists in several other Cysharp repositories (R3, ZLogger, ZLinq, ObservableCollections, ZString). I am filing one issue per repository rather than a single cross-repo issue, since the right fix differs per project.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.