Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove or document Unsafe.AsPointer uses in core libraries #99146

Merged
merged 14 commits into from
Mar 5, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -424,14 +424,7 @@ public void MultipleCallsToGetSpan()
Assert.True(span.Length >= 256);
Span<T> newSpan = output.GetSpan();
Assert.Equal(span.Length, newSpan.Length);

unsafe
{
void* pSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
void* pNewSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(newSpan));
Assert.Equal((IntPtr)pSpan, (IntPtr)pNewSpan);
}

Assert.Equal(0, Unsafe.ByteOffset(ref MemoryMarshal.GetReference(span), ref MemoryMarshal.GetReference(newSpan)));
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal(span.Length, output.GetSpan().Length);
}
finally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,16 @@ public static unsafe void CreateFromPinnedArrayVerifyPinning()
int[] pinnedArray = { 90, 91, 92, 93, 94, 95, 96, 97, 98 };
GCHandle pinnedGCHandle = GCHandle.Alloc(pinnedArray, GCHandleType.Pinned);

// Unsafe.AsPointer is used to ensure we catch if the GC moves the memory
Memory<int> pinnedMemory = MemoryMarshal.CreateFromPinnedArray(pinnedArray, 0, 2);
void* pinnedPtr = Unsafe.AsPointer(ref MemoryMarshal.GetReference(pinnedMemory.Span));
void* memoryHandlePinnedPtr = pinnedMemory.Pin().Pointer;

GC.Collect();
GC.Collect(2);

Assert.Equal((int)pinnedPtr, (int)Unsafe.AsPointer(ref MemoryMarshal.GetReference(pinnedMemory.Span)));
Assert.Equal((int)memoryHandlePinnedPtr, (int)pinnedGCHandle.AddrOfPinnedObject().ToPointer());
Assert.Equal((IntPtr)pinnedPtr, (IntPtr)Unsafe.AsPointer(ref MemoryMarshal.GetReference(pinnedMemory.Span)));
Assert.Equal((IntPtr)memoryHandlePinnedPtr, (IntPtr)pinnedGCHandle.AddrOfPinnedObject().ToPointer());
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved

pinnedGCHandle.Free();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static unsafe void GetArrayDataReference_EmptyInput_ReturnsRefToWhereFirs

ref int theRef = ref MemoryMarshal.GetArrayDataReference(theArray);

Assert.True(Unsafe.AsPointer(ref theRef) != null);
Assert.False(Unsafe.IsNullRef(ref theRef));
Assert.True(Unsafe.AreSame(ref theRef, ref MemoryMarshal.GetReference(theArray.AsSpan())));

ref int theMdArrayRef = ref Unsafe.As<byte, int>(ref MemoryMarshal.GetArrayDataReference((Array)theArray)); // szarray passed to generalized Array helper
Expand Down
9 changes: 4 additions & 5 deletions src/libraries/System.Memory/tests/MemoryPool/MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public static void MemoryPoolSpan()
{
unsafe
{
// Unsafe.AsPointer is safe here since it's pinned
void* pSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(sp));
Assert.Equal((IntPtr)newMemoryHandle.Pointer, (IntPtr)pSpan);
}
Expand All @@ -77,6 +78,7 @@ public static void MemoryPoolPin(int elementIndex)
{
unsafe
{
// Unsafe.AsPointer is safe here since it's pinned
void* pSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(sp.Slice(elementIndex)));
Assert.Equal((IntPtr)pSpan, ((IntPtr)newMemoryHandle.Pointer));
}
Expand Down Expand Up @@ -112,6 +114,7 @@ public static void MemoryPoolPinOffsetAtEnd()
{
unsafe
{
// Unsafe.AsPointer is safe here since it's pinned
void* pSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(sp.Slice(elementIndex)));
Assert.Equal((IntPtr)pSpan, ((IntPtr)newMemoryHandle.Pointer));
}
Expand Down Expand Up @@ -219,11 +222,7 @@ public static void MemoryPoolTryGetArray()
unsafe
{
Assert.True(MemoryMarshal.TryGetArray(memory, out arraySegment));
fixed (int* pArray = arraySegment.Array)
{
void* pSpan = Unsafe.AsPointer(ref MemoryMarshal.GetReference(memory.Span));
Assert.Equal((IntPtr)pSpan, (IntPtr)pArray);
}
Assert.Equal(0, Unsafe.ByteOffset(ref MemoryMarshal.GetArrayDataReference(arraySegment.Array), ref MemoryMarshal.GetReference(memory.Span)));
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/libraries/System.Memory/tests/ReadOnlySpan/AsSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ static unsafe void Validate(string text, int start, int length, ReadOnlySpan<cha
Assert.Equal(length, span.Length);
fixed (char* pText = text)
{
// Unsafe.AsPointer is safe here since it's pinned (since text and span should be the same string)
char* expected = pText + start;
void* actual = Unsafe.AsPointer(ref MemoryMarshal.GetReference(span));
Assert.Equal((IntPtr)expected, (IntPtr)actual);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Memory/tests/TestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public static unsafe void ValidateNonNullEmpty<T>(this Span<T> span)
Assert.True(span.IsEmpty);

// Validate that empty Span is not normalized to null
Assert.True(Unsafe.AsPointer(ref MemoryMarshal.GetReference(span)) != null);
Assert.False(Unsafe.IsNullRef(ref MemoryMarshal.GetReference(span)));
}

public delegate void AssertThrowsAction<T>(Span<T> span);
Expand Down Expand Up @@ -98,7 +98,7 @@ public static unsafe void ValidateNonNullEmpty<T>(this ReadOnlySpan<T> span)
Assert.True(span.IsEmpty);

// Validate that empty Span is not normalized to null
Assert.True(Unsafe.AsPointer(ref MemoryMarshal.GetReference(span)) != null);
Assert.False(Unsafe.IsNullRef(ref MemoryMarshal.GetReference(span)));
}

public delegate void AssertThrowsActionReadOnly<T>(ReadOnlySpan<T> span);
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/Memory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ public unsafe MemoryHandle Pin()
{
if (typeof(T) == typeof(char) && tmpObject is string s)
{
// Unsafe.AsPointer is safe since the handle pins it
GCHandle handle = GCHandle.Alloc(tmpObject, GCHandleType.Pinned);
ref char stringData = ref Unsafe.Add(ref s.GetRawStringData(), _index);
return new MemoryHandle(Unsafe.AsPointer(ref stringData), handle);
Expand All @@ -410,11 +411,13 @@ public unsafe MemoryHandle Pin()
// Array is already pre-pinned
if (_index < 0)
{
// Unsafe.AsPointer is safe since it's pinned
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref MemoryMarshal.GetArrayDataReference(Unsafe.As<T[]>(tmpObject))), _index & ReadOnlyMemory<T>.RemoveFlagsBitMask);
return new MemoryHandle(pointer);
}
else
{
// Unsafe.AsPointer is safe since the handle pins it
GCHandle handle = GCHandle.Alloc(tmpObject, GCHandleType.Pinned);
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref MemoryMarshal.GetArrayDataReference(Unsafe.As<T[]>(tmpObject))), _index);
return new MemoryHandle(pointer, handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ public unsafe MemoryHandle Pin()
{
if (typeof(T) == typeof(char) && tmpObject is string s)
{
// Unsafe.AsPointer is safe since the handle pins it
GCHandle handle = GCHandle.Alloc(tmpObject, GCHandleType.Pinned);
ref char stringData = ref Unsafe.Add(ref s.GetRawStringData(), _index);
return new MemoryHandle(Unsafe.AsPointer(ref stringData), handle);
Expand All @@ -325,11 +326,13 @@ public unsafe MemoryHandle Pin()
// Array is already pre-pinned
if (_index < 0)
{
// Unsafe.AsPointer is safe since it's pinned
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref MemoryMarshal.GetArrayDataReference(Unsafe.As<T[]>(tmpObject))), _index & RemoveFlagsBitMask);
return new MemoryHandle(pointer);
}
else
{
// Unsafe.AsPointer is safe since the handle pins it
GCHandle handle = GCHandle.Alloc(tmpObject, GCHandleType.Pinned);
void* pointer = Unsafe.Add<T>(Unsafe.AsPointer(ref MemoryMarshal.GetArrayDataReference(Unsafe.As<T[]>(tmpObject))), _index);
return new MemoryHandle(pointer, handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#pragma warning disable IDE0060 // implementations provided as intrinsics
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.Versioning;

Expand Down Expand Up @@ -907,5 +908,20 @@ public static ref T Unbox<T>(object box)
// unbox !!T
// ret
}


// Internal helper methods:

// Determines if the address is aligned at least to `alignment` bytes.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static bool IsOpportunisticallyAligned<T>(ref readonly T address, nuint alignment)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
{
// `alignment` is expected to be a power of 2 in bytes.
// We use Unsafe.AsPointer to convert to a pointer,
// GC will keep alignment when moving objects (up to sizeof(void*)),
// otherwise alignment should be considered a hint if not pinned.
Debug.Assert(nuint.IsPow2(alignment));
return ((nuint)AsPointer(ref AsRef(in address)) & (alignment - 1)) != 0;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal static partial class SpanHelpers
{
public static unsafe void ClearWithReferences(ref IntPtr ip, nuint pointerSizeLength)
{
Debug.Assert((int)Unsafe.AsPointer(ref ip) % sizeof(IntPtr) == 0, "Should've been aligned on natural word boundary.");
Debug.Assert(Unsafe.IsOpportunisticallyAligned(ref ip, (uint)sizeof(IntPtr)), "Should've been aligned on natural word boundary.");

// First write backward 8 natural words at a time.
// Writing backward allows us to get away with only simple modifications to the
Expand Down
Loading