Avoid assuming BCL private field layouts via Unsafe.As (List<T>, Stack<T>, BitArray, ImmutableArray<T>)
@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.cs — https://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
- No contract. The field layout of
List<T>,Stack<T>,BitArray, andImmutableArray<T>is an implementation detail. Nothing prevents the BCL from reordering, renaming, or adding fields, andBitArray'sm_*names date back to .NET Framework — there is no guarantee they stay. - Writing makes the failure mode worse.
view._size = lengthandview.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. - It fails silently.
Unsafe.Asperforms 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. - It is not confined to legacy targets.
CollectionsMarshalExis#if NET7_0_OR_GREATER, so this ships on modern .NET, not just in a shim for Unity or .NET Framework. - 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.SetCounthas been available since .NET 8 and does exactly whatCreateSpandoes, with runtime support. Since .NET 7 is out of support (EOL 2024-05-14), raising thenet7.0TFM tonet8.0would let theList<T>part be deleted outright.Stack<T>: there is no public equivalent, so this is the one case that needs a design decision. Serializing viastack.ToArray()/ reconstructing via theIEnumerable<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 sameint[]words thatm_arrayholds, andnew BitArray(int[] values)followed by settingLengthreconstructs it exactly. That removesBitArrayViewwith no format change.ImmutableArray<T>:ImmutableCollectionsMarshal.AsImmutableArrayis also available onnetstandard2.0through theSystem.Collections.Immutablepackage (8.0+), so the#elsebranch 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.