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 @@ -24,7 +24,7 @@ private struct SigPointer
private int _remainingArgs; // # of remaining args.

#if TARGET_WINDOWS // Native Varargs are not supported on Unix
// ArgIterator is a ref struct. It does not require pinning.
// ArgIterator is a ref struct. It does not require pinning, therefore Unsafe.AsPointer is safe.
// This method null checks the this pointer as a side-effect.
private ArgIterator* ThisPtr => (ArgIterator*)Unsafe.AsPointer(ref _argCookie);

Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,9 @@ public static unsafe IReadOnlyDictionary<string, object> GetConfigurationVariabl
Configurations = new Dictionary<string, object>()
};

_EnumerateConfigurationValues(Unsafe.AsPointer(ref context), &ConfigCallback);
#pragma warning disable CS8500 // takes address of managed type
_EnumerateConfigurationValues(&context, &ConfigCallback);
#pragma warning restore CS8500
return context.Configurations!;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,12 @@ private static unsafe object[] InitializeStatics(IntPtr gcStaticRegionStart, int
nint blockAddr = MethodTable.SupportsRelativePointers ? (nint)ReadRelPtr32(pBlock) : *pBlock;
if ((blockAddr & GCStaticRegionConstants.Uninitialized) == GCStaticRegionConstants.Uninitialized)
{
#pragma warning disable CS8500 // takes address of managed type
object? obj = null;
RuntimeImports.RhAllocateNewObject(
new IntPtr(blockAddr & ~GCStaticRegionConstants.Mask),
(uint)GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP,
Unsafe.AsPointer(ref obj));
&obj);
if (obj == null)
{
RuntimeExceptionHelpers.FailFast("Failed allocating GC static bases");
Expand All @@ -232,7 +233,8 @@ private static unsafe object[] InitializeStatics(IntPtr gcStaticRegionStart, int
Unsafe.Add(ref rawSpineData, currentBase) = obj;

// Update the base pointer to point to the pinned object
*pBlock = *(IntPtr*)Unsafe.AsPointer(ref obj);
*pBlock = *(IntPtr*)&obj;
#pragma warning restore CS8500
}

currentBase++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,9 @@ public static unsafe IReadOnlyDictionary<string, object> GetConfigurationVariabl
Configurations = new Dictionary<string, object>()
};

RuntimeImports.RhEnumerateConfigurationValues(Unsafe.AsPointer(ref context), &ConfigCallback);
#pragma warning disable CS8500 // takes address of managed type
RuntimeImports.RhEnumerateConfigurationValues(&context, &ConfigCallback);
#pragma warning restore CS8500
return context.Configurations!;
}

Expand Down Expand Up @@ -830,7 +832,9 @@ static T[] AllocateNewUninitializedArray(int length, bool pinned)
throw new OverflowException();

T[]? array = null;
RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
#pragma warning disable CS8500 // takes address of managed type
RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, &array);
#pragma warning restore CS8500
if (array == null)
throw new OutOfMemoryException();

Expand All @@ -857,7 +861,9 @@ public static unsafe T[] AllocateArray<T>(int length, bool pinned = false)
throw new OverflowException();

T[]? array = null;
RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, Unsafe.AsPointer(ref array));
#pragma warning disable CS8500 // takes address of managed type
RuntimeImports.RhAllocateNewArray(MethodTable.Of<T[]>(), (uint)length, (uint)flags, &array);
#pragma warning restore CS8500
if (array == null)
throw new OutOfMemoryException();

Expand Down
4 changes: 3 additions & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,11 @@ private CompilationResult CompileMethodInternal(IMethodNode methodCodeNodeNeedin
IntPtr exception;
IntPtr nativeEntry;
uint codeSize;
#pragma warning disable CS8500 // takes address of managed type
var result = JitCompileMethod(out exception,
_jit, (IntPtr)Unsafe.AsPointer(ref _this), _unmanagedCallbacks,
_jit, (IntPtr)(&_this), _unmanagedCallbacks,
ref methodInfo, (uint)CorJitFlag.CORJIT_FLAG_CALL_GETJITFLAGS, out nativeEntry, out codeSize);
#pragma warning restore CS8500
if (exception != IntPtr.Zero)
{
if (_lastException != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ private static void Test_get_runtime_property(string[] args)

static string GetProperty(string name, host_runtime_contract contract)
{
Span<byte> nameSpan = stackalloc byte[Encoding.UTF8.GetMaxByteCount(name.Length)];
byte* namePtr = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(nameSpan));
int nameSize = Encoding.UTF8.GetMaxByteCount(name.Length);
byte* namePtr = stackalloc byte[nameSize];
Span<byte> nameSpan = new Span<byte>(namePtr, nameSize);
int nameLen = Encoding.UTF8.GetBytes(name, nameSpan);
nameSpan[nameLen] = 0;

Expand Down Expand Up @@ -86,8 +87,9 @@ public static void Test_bundle_probe(string[] args)

unsafe static void Probe(host_runtime_contract contract, string path)
{
Span<byte> pathSpan = stackalloc byte[Encoding.UTF8.GetMaxByteCount(path.Length)];
byte* pathPtr = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(pathSpan));
int pathSize = Encoding.UTF8.GetMaxByteCount(path.Length);
byte* pathPtr = stackalloc byte[pathSize];
Span<byte> pathSpan = new Span<byte>(pathPtr, pathSize);
int pathLen = Encoding.UTF8.GetBytes(path, pathSpan);
pathSpan[pathLen] = 0;

Expand Down
14 changes: 5 additions & 9 deletions src/libraries/Common/src/System/Number.NumberBuffer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ internal unsafe ref struct NumberBuffer
public bool IsNegative;
public bool HasNonZeroTail;
public NumberBufferKind Kind;
public Span<byte> Digits;
public byte* DigitsPtr;
public int DigitsLength;
public readonly Span<byte> Digits => new Span<byte>(DigitsPtr, DigitsLength);

public NumberBuffer(NumberBufferKind kind, byte* digits, int digitsLength) : this(kind, new Span<byte>(digits, digitsLength))
{
Expand All @@ -48,7 +50,8 @@ public NumberBuffer(NumberBufferKind kind, Span<byte> digits)
IsNegative = false;
HasNonZeroTail = false;
Kind = kind;
Digits = digits;
DigitsPtr = (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(digits)); // Safe since memory must be fixed
DigitsLength = digits.Length;
#if DEBUG
Digits.Fill(0xCC);
#endif
Expand Down Expand Up @@ -83,13 +86,6 @@ public void CheckConsistency()
}
#pragma warning restore CA1822

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public byte* GetDigitsPointer()
{
// This is safe to do since we are a ref struct
return (byte*)Unsafe.AsPointer(ref MemoryMarshal.GetReference(Digits));
}

//
// Code coverage note: This only exists so that Number displays nicely in the VS watch window. So yes, I know it works.
//
Expand Down
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, pinnedGCHandle.AddrOfPinnedObject());

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
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,14 @@ protected unsafe void WriteEvent(int eventId, long arg1, byte[]? arg2)
}
}

// Returns the object as a IntPtr - safe when only used for logging
internal static unsafe nint ObjectIDForEvents(object? o)
{
#pragma warning disable CS8500 // takes address of managed type
return *(nint*)&o;
#pragma warning restore CS8500
}

#pragma warning restore 1591

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ private void ContentionLockCreated(nint LockID, nint AssociatedObjectID, ushort

[NonEvent]
[MethodImpl(MethodImplOptions.NoInlining)]
public void ContentionLockCreated(Lock lockObj) => ContentionLockCreated(lockObj.LockIdForEvents, lockObj.ObjectIdForEvents);
public void ContentionLockCreated(Lock lockObj) => ContentionLockCreated(lockObj.LockIdForEvents, ObjectIDForEvents(lockObj));

[Event(81, Level = EventLevel.Informational, Message = Messages.ContentionStart, Task = Tasks.Contention, Opcode = EventOpcode.Start, Version = 2, Keywords = Keywords.ContentionKeyword)]
private void ContentionStart(
Expand All @@ -115,7 +115,7 @@ public void ContentionStart(Lock lockObj) =>
ContentionFlagsMap.Managed,
DefaultClrInstanceId,
lockObj.LockIdForEvents,
lockObj.ObjectIdForEvents,
ObjectIDForEvents(lockObj),
lockObj.OwningThreadId);

[Event(91, Level = EventLevel.Informational, Message = Messages.ContentionStop, Task = Tasks.Contention, Opcode = EventOpcode.Stop, Version = 1, Keywords = Keywords.ContentionKeyword)]
Expand Down Expand Up @@ -360,7 +360,7 @@ private void WaitHandleWaitStart(
public unsafe void WaitHandleWaitStart(
WaitHandleWaitSourceMap waitSource = WaitHandleWaitSourceMap.Unknown,
object? associatedObject = null) =>
WaitHandleWaitStart(waitSource, *(nint*)Unsafe.AsPointer(ref associatedObject));
WaitHandleWaitStart(waitSource, ObjectIDForEvents(associatedObject));

[Event(302, Level = EventLevel.Verbose, Message = Messages.WaitHandleWaitStop, Task = Tasks.WaitHandleWait, Opcode = EventOpcode.Stop, Version = 0, Keywords = Keywords.WaitHandleKeyword)]
public void WaitHandleWaitStop(ushort ClrInstanceID = DefaultClrInstanceId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ private unsafe void ContentionLockCreated(nint LockID, nint AssociatedObjectID,

[NonEvent]
[MethodImpl(MethodImplOptions.NoInlining)]
public void ContentionLockCreated(Lock lockObj) => ContentionLockCreated(lockObj.LockIdForEvents, lockObj.ObjectIdForEvents);
public void ContentionLockCreated(Lock lockObj) => ContentionLockCreated(lockObj.LockIdForEvents, ObjectIDForEvents(lockObj));

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern", Justification = "Parameters to this method are primitive and are trimmer safe")]
[Event(81, Level = EventLevel.Informational, Message = Messages.ContentionStart, Task = Tasks.Contention, Opcode = EventOpcode.Start, Version = 2, Keywords = Keywords.ContentionKeyword)]
Expand Down Expand Up @@ -146,7 +146,7 @@ public void ContentionStart(Lock lockObj) =>
ContentionFlagsMap.Managed,
DefaultClrInstanceId,
lockObj.LockIdForEvents,
lockObj.ObjectIdForEvents,
ObjectIDForEvents(lockObj),
lockObj.OwningThreadId);

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern", Justification = "Parameters to this method are primitive and are trimmer safe")]
Expand Down Expand Up @@ -557,7 +557,7 @@ private unsafe void WaitHandleWaitStart(
public unsafe void WaitHandleWaitStart(
WaitHandleWaitSourceMap waitSource = WaitHandleWaitSourceMap.Unknown,
object? associatedObject = null) =>
WaitHandleWaitStart(waitSource, *(nint*)Unsafe.AsPointer(ref associatedObject));
WaitHandleWaitStart(waitSource, ObjectIDForEvents(associatedObject));

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern", Justification = "Parameters to this method are primitive and are trimmer safe")]
[Event(302, Level = EventLevel.Verbose, Message = Messages.WaitHandleWaitStop, Task = Tasks.WaitHandleWait, Opcode = EventOpcode.Stop, Version = 0, Keywords = Keywords.WaitHandleKeyword)]
Expand Down
Loading
Loading