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 @@ -206,12 +206,13 @@ 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
object? obj = null;
#pragma warning disable CS8500
RuntimeImports.RhAllocateNewObject(
new IntPtr(blockAddr & ~GCStaticRegionConstants.Mask),
(uint)GC_ALLOC_FLAGS.GC_ALLOC_PINNED_OBJECT_HEAP,
&obj);
#pragma warning restore CS8500
if (obj == null)
{
RuntimeExceptionHelpers.FailFast("Failed allocating GC static bases");
Expand All @@ -233,8 +234,7 @@ 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*)&obj;
#pragma warning restore CS8500
*pBlock = Unsafe.GetPinnedObjectPointer(obj);
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}

currentBase++;
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, Unsafe.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,
Unsafe.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 @@ -355,14 +355,12 @@ private void WaitHandleWaitStart(
LogWaitHandleWaitStart(WaitSource, AssociatedObjectID, ClrInstanceID);
}

#pragma warning disable CS8500
[NonEvent]
[MethodImpl(MethodImplOptions.NoInlining)]
public unsafe void WaitHandleWaitStart(
WaitHandleWaitSourceMap waitSource = WaitHandleWaitSourceMap.Unknown,
object? associatedObject = null) =>
WaitHandleWaitStart(waitSource, *(nint*)&associatedObject);
#pragma warning restore CS8500
WaitHandleWaitStart(waitSource, Unsafe.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, Unsafe.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,
Unsafe.ObjectIDForEvents(lockObj),
lockObj.OwningThreadId);

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:UnrecognizedReflectionPattern", Justification = "Parameters to this method are primitive and are trimmer safe")]
Expand Down Expand Up @@ -552,14 +552,12 @@ private unsafe void WaitHandleWaitStart(
WriteEventCore(301, 3, data);
}

#pragma warning disable CS8500
[NonEvent]
[MethodImpl(MethodImplOptions.NoInlining)]
public unsafe void WaitHandleWaitStart(
WaitHandleWaitSourceMap waitSource = WaitHandleWaitSourceMap.Unknown,
object? associatedObject = null) =>
WaitHandleWaitStart(waitSource, *(nint*)&associatedObject);
#pragma warning restore CS8500
WaitHandleWaitStart(waitSource, Unsafe.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
Original file line number Diff line number Diff line change
Expand Up @@ -935,5 +935,10 @@ internal static nuint OpportunisticMisalignment<T>(ref readonly T address, nuint
Debug.Assert(nuint.IsPow2(alignment));
return (nuint)AsPointer(ref AsRef(in address)) & (alignment - 1);
}

// Returns the object as a IntPtr - safe when object is pinned, or when only used for logging;
// The second method is also defined so that safe usage is always obvious by name.
internal static nint GetPinnedObjectPointer(object o) => *(nint*)&o;
internal static nint ObjectIDForEvents(object o) => GetPinnedObjectPointer(o);
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
}
}
11 changes: 0 additions & 11 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -636,17 +636,6 @@ internal nint LockIdForEvents
}
}

internal unsafe nint ObjectIdForEvents
{
get
{
Lock lockObj = this;
#pragma warning disable CS8500
return *(nint*)&lockObj;
#pragma warning restore CS8500
}
}

internal ulong OwningThreadId => _owningThreadId;

private static short DetermineMaxSpinCount() =>
Expand Down
Loading