Conflicting "unsafe code best practices" information

[colejohnson66]

Microsoft's own "unsafe code best practices" says "In CoreCLR, the object header cannot be accessed safely without pinning the object first." Yet, in various low-level regions of the runtime library, you can see the MethodTable accessed at-will, with only a GC.KeepAlive guarding them.

For example:

runtime/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

Lines 393 to 440 in 0cbfea4

	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	internal static unsafe nuint GetRawObjectDataSize(object obj)
	{
	MethodTable* pMT = GetMethodTable(obj);
	// See comment on RawArrayData for details
	nuint rawSize = pMT->BaseSize - (nuint)(2 * sizeof(IntPtr));
	if (pMT->HasComponentSize)
	rawSize += (uint)Unsafe.As<RawArrayData>(obj).Length * (nuint)pMT->ComponentSize;
	GC.KeepAlive(obj); // Keep MethodTable alive
	return rawSize;
	}
	// Returns array element size.
	// Callers are required to keep obj alive
	internal static unsafe ushort GetElementSize(this Array array)
	{
	Debug.Assert(ObjectHasComponentSize(array));
	return GetMethodTable(array)->ComponentSize;
	}
	// Returns pointer to the multi-dimensional array bounds.
	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	internal static ref int GetMultiDimensionalArrayBounds(this Array array)
	{
	Debug.Assert(GetMultiDimensionalArrayRank(array) > 0);
	// See comment on RawArrayData for details
	return ref Unsafe.As<byte, int>(ref Unsafe.As<RawArrayData>(array).Data);
	}
	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	internal static unsafe int GetMultiDimensionalArrayRank(this Array array)
	{
	int rank = GetMethodTable(array)->MultiDimensionalArrayRank;
	GC.KeepAlive(array); // Keep MethodTable alive
	return rank;
	}
	// Returns true iff the object has a component size;
	// i.e., is variable length like System.String or Array.
	// Callers are required to keep obj alive
	[MethodImpl(MethodImplOptions.AggressiveInlining)]
	internal static unsafe bool ObjectHasComponentSize(object obj)
	{
	return GetMethodTable(obj)->HasComponentSize;
	}

And usage sites that don't "pin" the object:

runtime/src/libraries/System.Private.CoreLib/src/System/Array.cs

Lines 718 to 761 in 0cbfea4

	private unsafe nint GetFlattenedIndex(int rawIndex)
	{
	// Checked by the caller
	Debug.Assert(Rank == 1);
	if (!RuntimeHelpers.GetMethodTable(this)->IsSzArray)
	{
	ref int bounds = ref this.GetMultiDimensionalArrayBounds();
	rawIndex -= Unsafe.Add(ref bounds, 1);
	}
	if ((uint)rawIndex >= NativeLength)
	ThrowHelper.ThrowIndexOutOfRangeException();
	return rawIndex;
	}
	internal unsafe nint GetFlattenedIndex(ReadOnlySpan<int> indices)
	{
	// Checked by the caller
	Debug.Assert(indices.Length == Rank);
	if (!RuntimeHelpers.GetMethodTable(this)->IsSzArray)
	{
	ref int bounds = ref this.GetMultiDimensionalArrayBounds();
	nint flattenedIndex = 0;
	for (int i = 0; i < indices.Length; i++)
	{
	int index = indices[i] - Unsafe.Add(ref bounds, indices.Length + i);
	int length = Unsafe.Add(ref bounds, i);
	if ((uint)index >= (uint)length)
	ThrowHelper.ThrowIndexOutOfRangeException();
	flattenedIndex = (length * flattenedIndex) + index;
	}
	Debug.Assert((nuint)flattenedIndex < NativeLength);
	return flattenedIndex;
	}
	else
	{
	int index = indices[0];
	if ((uint)index >= NativeLength)
	ThrowHelper.ThrowIndexOutOfRangeException();
	return index;
	}
	}

So which is it?

[EgorBo]

I guess it depends on the terminology. All objects in CoreCLR/NativeAOT have the following layout:

| header | pMethodTable | other fields/data |
         |
         ^ object starts here

So the pointer to the MethodTable is accessible at the offset 0 and is part of the object, while the header (it's where we store hashcode/thinlock/syncblocIndex, some system bits (and 4bytes empty padding on 64bit)) is accessible at the negative offset and technically becomes an exterior pointer. From GC's perspective it kind of belongs to the previous object roughly saying.

[tannergooding]

What is disallowed/unsafe for user code is not the same as what is disallowed/unsafe for the runtime itself.

User code is written, versioned, and shipped independently of the runtime. It can "roll forward" onto newer runtimes, won't be specially handled by the runtime, and so there are more general limitations to it.

The core library (currently System.Private.Corelib), however, is "built-in" and ships with the runtime. It is only ever supported with the exact version of the runtime that it was built for and it cannot "roll forward". It, by virtue of being the foundational thing, also provides many specialized types and scenarios which are specially handled by the runtime (VM, JIT/AOT, etc).

Accordingly, one cannot simply look at code in the core libraries and go "they're doing it, so I guess I can in my library as well".

[tannergooding]

Put another way, an implementation can rely on its own implementation details. It can access and change it's own private state. It can special case scenarios, bypass checks, and do other things because its only doing things to itself.

An implementation cannot safely depend on something else's implementation details and cannot safely touch the private state of something else.

This is especially true for the "primitive" types which fundamentally must have specialized handling to even function, as they are by spec "special" and require specialized semantics.

[colejohnson66]

Oh for sure. I'm not asking if it's safe for me to do so, but just about the disconnect. Because the documentation says I can't (without pinning), not that I shouldn't. The way I read that is that the GC doesn't track interior references to MethodTable*, but @EgorBo corrected me.

[MichalPetryka]

Also, it's worth noting that the MethodTable* there does NOT point to a part of the object, the only part accessing object header there is the GetMethodTable call (which is now an intrinsic too).

The MethodTable itself that it points to is stored in separate memory and shared between all instances of the type (the KeepAlive there is only for preventing the type from unloading).