[API Proposal]: Mark MemoryMarshal APIs as caller-unsafe

[EgorBo]

Background and motivation

Follow-up to #126956. This proposal lists the APIs in System.Runtime.InteropServices.MemoryMarshal and marks them as caller-unsafe.

I'm using a diff format where + indicates that it's now caller-unsafe (gains the unsafe modifier on the method) and - indicates the unsafe modifier has been removed (a non-breaking change).

The explicit [RequiresUnsafe] attribute is being removed in favor of the language-level unsafe modifier on members

API Proposal

public static partial class MemoryMarshal
{
+   public static unsafe ReadOnlySpan<byte> AsBytes<T>(ReadOnlySpan<T> span) where T : struct { throw null; }
+   public static unsafe Span<byte> AsBytes<T>(Span<T> span) where T : struct { throw null; }
+   public static unsafe Memory<T> AsMemory<T>(ReadOnlyMemory<T> memory) { throw null; }
+   public static unsafe ref readonly T AsRef<T>(ReadOnlySpan<byte> span) where T : struct { throw null; }
+   public static unsafe ref T AsRef<T>(Span<byte> span) where T : struct { throw null; }
+   public static unsafe ReadOnlySpan<TTo> Cast<TFrom, TTo>(ReadOnlySpan<TFrom> span) where TFrom : struct where TTo : struct { throw null; }
+   public static unsafe Span<TTo> Cast<TFrom, TTo>(Span<TFrom> span) where TFrom : struct where TTo : struct { throw null; }
+   public static unsafe Memory<T> CreateFromPinnedArray<T>(T[]? array, int start, int length) { throw null; }
    public unsafe static ReadOnlySpan<byte> CreateReadOnlySpanFromNullTerminated(byte* value) { throw null; }
    public unsafe static ReadOnlySpan<char> CreateReadOnlySpanFromNullTerminated(char* value) { throw null; }
+   public static unsafe ReadOnlySpan<T> CreateReadOnlySpan<T>(scoped ref readonly T reference, int length) { throw null; }
+   public static unsafe Span<T> CreateSpan<T>(scoped ref T reference, int length) { throw null; }
+   public static unsafe ref byte GetArrayDataReference(Array array) { throw null; }
+   public static unsafe ref T GetArrayDataReference<T>(T[] array) { throw null; }
+   public static unsafe ref T GetReference<T>(ReadOnlySpan<T> span) { throw null; }
+   public static unsafe ref T GetReference<T>(Span<T> span) { throw null; }
+   public static unsafe T Read<T>(ReadOnlySpan<byte> source) where T : struct { throw null; }
    public static IEnumerable<T> ToEnumerable<T>(ReadOnlyMemory<T> memory) { throw null; }
+   public static unsafe bool TryGetArray<T>(ReadOnlyMemory<T> memory, out ArraySegment<T> segment) { throw null; }
+   public static unsafe bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, [NotNullWhen(true)] out TManager? manager) where TManager : MemoryManager<T> { throw null; }
+   public static unsafe bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, [NotNullWhen(true)] out TManager? manager, out int start, out int length) where TManager : MemoryManager<T> { throw null; }
    public static bool TryGetString(ReadOnlyMemory<char> memory, [NotNullWhen(true)] out string? text, out int start, out int length) { throw null; }
+   public static unsafe bool TryRead<T>(ReadOnlySpan<byte> source, out T value) where T : struct { throw null; }
+   public static unsafe bool TryWrite<T>(Span<byte> destination, in T value) where T : struct { throw null; }
+   public static unsafe void Write<T>(Span<byte> destination, in T value) where T : struct { }
}

API Usage

The annotated APIs will require unsafe { } context at their call site once the language-level unsafe requirement is enforced.

Alternative Designs

No response

Risks

No response

[dotnet-policy-service]

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

[tannergooding]

public static Memory<T> CreateFromPinnedArray<T>(T[]? array, int start, int length) { throw null; }

I believe this one should be unsafe for the reasons we discussed internally. It violates an invariant and can potentially allow for an AV.

While the actual dereference will occur by the user through a pointer (indirecting MemoryHandle.Pointer), the actual point of unsafety is in the creation from an array that we claim is pinned if it actually isn't. In safe code MemoryHandle would otherwise guarantee that there are Memory<T>.Length elements of type T accessible from the pointer, while we cannot maintain such a claim for an invalid CreateFromPinnedArray<T> call.

public static T Read<T>(ReadOnlySpan<byte> source) where T : struct { throw null; }
public static bool TryRead<T>(ReadOnlySpan<byte> source, out T value) where T : struct { throw null; }

Similar here, this is unsafe because it may produce a T that has an unsafe U _field;

public static unsafe bool TryGetArray<T>(ReadOnlyMemory<T> memory, out ArraySegment<T> segment) { throw null; }
public static unsafe bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, [NotNullWhen(true)] out TManager? manager) where TManager : MemoryManager<T> { throw null; }
public static unsafe bool TryGetMemoryManager<T, TManager>(ReadOnlyMemory<T> memory, [NotNullWhen(true)] out TManager? manager, out int start, out int length) where TManager : MemoryManager<T> { throw null; }
public static bool TryGetString(ReadOnlyMemory<char> memory, [NotNullWhen(true)] out string? text, out int start, out int length) { throw null; }

It isn't clear why these would be unsafe. They aren't violating any invariants of ROMemory<T> and only return the backing storage if it could be recovered. That may create an alias to the backing storage, but no more than just holding on to the original may have already done.

Essentially beware dragons and potential data races, but no potential for AVs or violating things.

---

The other annotations and addition of unsafe look correct.

[agocke]

I think we need comments for each of these -- to explain what's unsafe and what's needed by the caller to make the call sound.