Reflection API Incorrectly Boxes (U)IntPtr Constants as (U)Int32

[LEI-Hongfaan]

Description

The reflection API is expected to box constant literal fields of type (U)IntPtr as their correct types. However, when using reflection to retrieve the value of these constants, they are being boxed as (U)Int32 instead of (U)IntPtr.

Reproduction Steps

public enum E {

    V = 42,
}

internal class Program {

    public const UIntPtr A = 3;

    public const IntPtr B1 = -5;

    public const E C = E.V;

    public const long D = 1;

    static void Main(string[] args) {
        DumpConstLiteralByReflection("UIntPtr", nameof(A));
        DumpConstLiteralByReflection("IntPtr", nameof(B1));
        DumpConstLiteralByReflection("enum E", nameof(C));
        DumpConstLiteralByReflection("long", nameof(D));

        static void DumpConstLiteralByReflection(string label, string fieldName) {
            Console.WriteLine($@"{label}:");
            Console.WriteLine($@"  Field type: {typeof(Program).GetField(fieldName)!.FieldType}");
            var o = typeof(Program).GetField(fieldName)!.GetValue(null)!;
            Console.WriteLine($@"  Value type: {o.GetType()}");
            Console.WriteLine($@"  Value: {o:D}");
            Console.WriteLine();
        }
    }
}

Expected behavior

UIntPtr:
  Field type: System.UIntPtr
  Value type: System.UIntPtr
  Value: 3

IntPtr:
  Field type: System.IntPtr
  Value type: System.IntPtr
  Value: -5

enum E:
  Field type: E
  Value type: E
  Value: 42

long:
  Field type: System.Int64
  Value type: System.Int64
  Value: 1

Actual behavior

UIntPtr:
  Field type: System.UIntPtr
  Value type: System.UInt32
  Value: 3

IntPtr:
  Field type: System.IntPtr
  Value type: System.Int32
  Value: -5

enum E:
  Field type: E
  Value type: E
  Value: 42

long:
  Field type: System.Int64
  Value type: System.Int64
  Value: 1

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

This issue may present a theoretical security risk, although the practical threat is likely negligible.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

[colejohnson66]

Const n[u]int are limited to 32 bits in size, so a boxed constant n[u]int must be 32-bit, even on a 64-bit platform. The way to do such a thing is to actually box a [u]int.

[LEI-Hongfaan]

Const n[u]int are limited to 32 bits in size, so a boxed constant n[u]int must be 32-bit, even on a 64-bit platform. The way to do such a thing is to actually box a [u]int.

I also fail to see how this could be a security risk as it only affects constants.

The runtime is capable of loading an assembly containing a field defined as .field public static literal native uint B2 = int32(0Xffffffff). Moreover, the current behavior is counterintuitive and not what would be generally expected.

[LEI-Hongfaan]

I believe that the field initialization clause should merely serve as an expression of a literal value, rather than determining the field's type. To illustrate this with a thought experiment: even if the field initialization clause provided a string representing a C++ constant expression, and a tool calculated the field's value based on this C++ source code, I wouldn't find it strange.

[huoyaoyuan]

Note: according to ECMA-335 §II.16.2 Field init metadata:

Note that while both the type and the field initializer are stored in metadata there is no requirement that they match. (Any importing compiler is responsible for coercing the stored value to the target field type).

The original purpose was to allow conversion from underlying numeric types to enum types. However, it doesn't specify the desired behavior of other casts. It's not clear what's the expected behavior here.

[huoyaoyuan]

The runtime is capable of loading an assembly containing a field defined as .field public static literal native uint B2 = int32(0Xffffffff).

Also note that currently there's no compiler emitting this, and probably not understanding it.
For example, trying to use the constant in reference assembly.

[steveharter]

Verified repo; for the first field, the IL is:

.field public static literal native uint A = uint32(0x00000003)

so for constants yes it does change to a int\uint.

Also, for .NET Framework, this code fails compile with CS0283 The type 'IntPtr' cannot be declared const where that link does not call out the native pointer types.

This is not a reflection issue; changing area to runtime, although this is likely an ECMA issue and where we supported this to compile, unlike .NET Framework.

cc @jkotas @AaronRobinsonMSFT

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

[AaronRobinsonMSFT]

@steveharter Thanks for the investigation. I've also confirmed your findings using a variety of tools. The Constants table (0x0B) does encode this as uint32 so what we are returning is technically correct based on the data we have. We also have the actual field type (that is native uint) and can special case this behavior.

I personally don't like inconsistencies in this domain, makes for too much confusion. The fact that .NET Framework doesn't support this scenario lowers the concern about making it do what the OP expects.

The code to update is below. We could make a check to see if the field is native int / native uint and then determine if the CorElementType disagrees. Many options here. @jkotas or @MichalStrehovsky Any opinions?

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/MdConstant.cs

Lines 108 to 126 in 8432f0d

	return corElementType switch
	{
	CorElementType.ELEMENT_TYPE_VOID => DBNull.Value,
	CorElementType.ELEMENT_TYPE_CHAR => *(char*)&buffer,
	CorElementType.ELEMENT_TYPE_I1 => *(sbyte*)&buffer,
	CorElementType.ELEMENT_TYPE_U1 => *(byte*)&buffer,
	CorElementType.ELEMENT_TYPE_I2 => *(short*)&buffer,
	CorElementType.ELEMENT_TYPE_U2 => *(ushort*)&buffer,
	CorElementType.ELEMENT_TYPE_I4 => *(int*)&buffer,
	CorElementType.ELEMENT_TYPE_U4 => *(uint*)&buffer,
	CorElementType.ELEMENT_TYPE_I8 => buffer,
	CorElementType.ELEMENT_TYPE_U8 => (ulong)buffer,
	CorElementType.ELEMENT_TYPE_BOOLEAN => (*(int*)&buffer != 0),
	CorElementType.ELEMENT_TYPE_R4 => *(float*)&buffer,
	CorElementType.ELEMENT_TYPE_R8 => *(double*)&buffer,
	CorElementType.ELEMENT_TYPE_STRING => stringVal ?? string.Empty,
	CorElementType.ELEMENT_TYPE_CLASS => null,
	_ => throw new FormatException(SR.Arg_BadLiteralFormat),
	};

[davidwrighton]

I don't think we should change anything in the runtime here.

I took a slightly deeper look, and if you change the repro code to use nint and nuint for the const field types, and compile with a recent C# language version, you can compile and run the test case on the .NET Framework. So we have a legacy support burden that may be in place already. My advice is to write something into the reflection documentation about this weird case.

[jkotas]

Handling of constant and default values in reflection have seen number of fixes over the years in .NET Core. Some of the earlier fixes include:

• dotnet/coreclr@dbcfd2f
• dotnet/coreclr@6f0bb94
• 05b8bab

We have systemic problem where Roslyn encodes the constants and default values as it sees a fit, but reflection is not aware of all possible encodings produced by Roslyn and it may produce unexpected results for some of them.

We took number of fixes in this area and the corner case behavior deviated from .NET Framework, so I do not see a problem with fixing more corner cases. However, until somebody sits down and ensures that the reflection behavior is sensible for everything that's possible to write in C#, we are unlikely to be in 100% happy place.

Note that there is a fresh from the press proposal to allow encoding even more types as constants in C#: dotnet/csharplang#8257 . If this proposal is accepted, we will have a work to do here anyway.

This is not a reflection issue

Nit: I see this as Roslyn and reflection interaction issue. Nothing in the runtime outside of reflection cares about encoding of C# constants.