Use a custom ReadOnlySpan marshaller to prevent null pointers when marshaling #115001
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,107 @@ | ||||||
| // Licensed to the .NET Foundation under one or more agreements. | ||||||
| // The .NET Foundation licenses this file to you under the MIT license. | ||||||
|
|
||||||
| using System; | ||||||
| using System.Collections.Generic; | ||||||
| using System.Runtime.CompilerServices; | ||||||
| using System.Text; | ||||||
|
|
||||||
| namespace System.Runtime.InteropServices.Marshalling | ||||||
| { | ||||||
| [CustomMarshaller(typeof(ReadOnlySpan<>), MarshalMode.ManagedToUnmanagedIn, typeof(NotNullReadOnlySpanMarshaller<,>.ManagedToUnmanagedIn))] | ||||||
| [CustomMarshaller(typeof(ReadOnlySpan<>), MarshalMode.ManagedToUnmanagedOut, typeof(NotNullReadOnlySpanMarshaller<,>.ManagedToUnmanagedOut))] | ||||||
| [ContiguousCollectionMarshaller] | ||||||
| internal static unsafe class NotNullReadOnlySpanMarshaller<T, TUnmanagedElement> | ||||||
| where TUnmanagedElement : unmanaged | ||||||
| { | ||||||
| public ref struct ManagedToUnmanagedIn | ||||||
| { | ||||||
| /// <summary> | ||||||
| /// Gets the size of the caller-allocated buffer to allocate. | ||||||
| /// </summary> | ||||||
| // We'll keep the buffer size at a maximum of 512 bytes to avoid overflowing the stack. | ||||||
| public static int BufferSize => 0x200 / sizeof(TUnmanagedElement); | ||||||
|
|
||||||
| private ReadOnlySpan<T> _managedArray; | ||||||
| private TUnmanagedElement* _allocatedMemory; | ||||||
| private Span<TUnmanagedElement> _span; | ||||||
|
|
||||||
| public void FromManaged(ReadOnlySpan<T> managed, Span<TUnmanagedElement> buffer) | ||||||
| { | ||||||
| _managedArray = managed; | ||||||
|
|
||||||
| if (managed.Length <= buffer.Length) | ||||||
| { | ||||||
| _span = buffer[0..managed.Length]; | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| int bufferSize = checked(managed.Length * sizeof(TUnmanagedElement)); | ||||||
| _allocatedMemory = (TUnmanagedElement*)NativeMemory.Alloc((nuint)bufferSize); | ||||||
| _span = new Span<TUnmanagedElement>(_allocatedMemory, managed.Length); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public ReadOnlySpan<T> GetManagedValuesSource() => _managedArray; | ||||||
|
|
||||||
| public Span<TUnmanagedElement> GetUnmanagedValuesDestination() => _span; | ||||||
|
|
||||||
| public ref TUnmanagedElement GetPinnableReference() => ref MemoryMarshal.GetReference(_span); | ||||||
|
|
||||||
| public TUnmanagedElement* ToUnmanaged() | ||||||
| { | ||||||
| // Unsafe.AsPointer is safe since buffer must be pinned | ||||||
| return (TUnmanagedElement*)Unsafe.AsPointer(ref GetPinnableReference()); | ||||||
| } | ||||||
|
|
||||||
| public void Free() | ||||||
| { | ||||||
| NativeMemory.Free(_allocatedMemory); | ||||||
| } | ||||||
|
|
||||||
| public static ref T GetPinnableReference(ReadOnlySpan<T> managed) | ||||||
| { | ||||||
vcsjones marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| if (Unsafe.IsNullRef(ref MemoryMarshal.GetReference(managed)) && managed.IsEmpty) | ||||||
|
There was a problem hiding this comment.
Suggested change
This can be simpler There was a problem hiding this comment. I had the same thought as Jan, then wondered if it was valueable to have the zero-length slice of some other array report a different pointer than the empty array, and didn't leave a comment. (His comment has "forced" me to). I can't think of anywhere in the crypto use cases where the pointer is used as a lookup (and if it was we'd have to do a lot more pinning), so normalizing all emptys to the same non-null is probably fine. |
||||||
| { | ||||||
| return ref MemoryMarshal.GetArrayDataReference(Array.Empty<T>()); | ||||||
| } | ||||||
| else | ||||||
| { | ||||||
| return ref MemoryMarshal.GetReference(managed); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| public struct ManagedToUnmanagedOut | ||||||
| { | ||||||
| private TUnmanagedElement* _unmanagedArray; | ||||||
| private T[]? _managedValues; | ||||||
|
|
||||||
| public void FromUnmanaged(TUnmanagedElement* unmanaged) | ||||||
| { | ||||||
| _unmanagedArray = unmanaged; | ||||||
| } | ||||||
|
|
||||||
| public ReadOnlySpan<T> ToManaged() | ||||||
| { | ||||||
| return new ReadOnlySpan<T>(_managedValues!); | ||||||
| } | ||||||
|
|
||||||
| public ReadOnlySpan<TUnmanagedElement> GetUnmanagedValuesSource(int numElements) | ||||||
| { | ||||||
| return new ReadOnlySpan<TUnmanagedElement>(_unmanagedArray, numElements); | ||||||
| } | ||||||
|
|
||||||
| public Span<T> GetManagedValuesDestination(int numElements) | ||||||
| { | ||||||
| _managedValues = new T[numElements]; | ||||||
| return _managedValues; | ||||||
| } | ||||||
|
|
||||||
| public void Free() | ||||||
| { | ||||||
| Marshal.FreeCoTaskMem((IntPtr)_unmanagedArray); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt that you will ever use (or want to use) the
ManagedToUnmanagedOutvariant in libraries. You can certainly add it, but it is effectively dead code.