Assertion failed 'fieldCorType == CORINFO_TYPE_VALUECLASS' during 'Optimize Valnum CSEs'

[MichalStrehovsky]

NGenDump leading to the assert: ngendump.txt

Compile the below with:

• csc /noconfig /nostdlib cse.cs /r:System.Private.CoreLib.dll /O /unsafe /target:library
• crossgen cse.dll

using System.Runtime.InteropServices;
using System;
using System.Diagnostics;

        [StructLayout(LayoutKind.Sequential)]
        struct DynamicFieldHandleInfo
        {
            public IntPtr DeclaringType;
            public IntPtr FieldName;
        }

class Foo
{
        public string GetStringFromMemoryInNativeFormat(IntPtr p) => null;

        private unsafe bool TryGetDynamicRuntimeFieldHandleComponents(RuntimeFieldHandle runtimeFieldHandle, out RuntimeTypeHandle declaringTypeHandle, out string fieldName)
        {
            IntPtr runtimeFieldHandleValue = *(IntPtr*)&runtimeFieldHandle;

            // Special flag in the handle value to indicate it was dynamically allocated
            Debug.Assert((runtimeFieldHandleValue.ToInt64() & 0x1) == 0x1);
            runtimeFieldHandleValue = runtimeFieldHandleValue - 1;

            DynamicFieldHandleInfo* fieldData = (DynamicFieldHandleInfo*)runtimeFieldHandleValue.ToPointer();
            declaringTypeHandle = *(RuntimeTypeHandle*)&(fieldData->DeclaringType);

            // FieldName points to the field name in NativeLayout format, so we parse it using a NativeParser
            IntPtr fieldNamePtr = fieldData->FieldName;
            fieldName = GetStringFromMemoryInNativeFormat(fieldNamePtr);

            return true;
        }
}

[BruceForstall]

Assert failure(PID 33540 [0x00008304], Thread: 6592 [0x19c0]): Assertion failed 'fieldCorType == CORINFO_TYPE_VALUECLASS' in 'Foo:TryGetDynamicRuntimeFieldHandleComponents(System.RuntimeFieldHandle,byref,byref):bool:this' during 'Optimize Valnum CSEs' (IL size 57)

    File: C:\git\runtime\src\coreclr\src\jit\gentree.cpp Line: 17312
    Image: C:\git\runtime\artifacts\bin\coreclr\Windows_NT.x64.Debug\crossgen.exe

[BruceForstall]

@dotnet/jit-contrib

[briansull]

This assert was added with #33225
It doesn't hold when unsafe code uses type casts to force conversions between types.

[briansull]

Fix is to remove the assert and allow a nullptr to be returned for the structHnd.
In this case the CSE code will not consider the node to be a CSE candidate.

line 3521 of optcse.cpp:

    // If this is a struct type (including SIMD*), we can only consider it for CSE-ing
    // if we can get its handle, so that we can create a temp.
    if (varTypeIsStruct(type) && (gtGetStructHandleIfPresent(tree) == NO_CLASS_HANDLE))
    {
        return false;
    }

[sandreenko]

That is interesting, I agree with your analysis, except that we had that assert before the change, the PR just moved it down a bit.
Also, in the repro from Michal it is not clear for me why ZapInfo::getFieldType returns CORINFO_TYPE_NATIVEINT when asking about DynamicFieldHandleInfo.DeclaringType that is IntPtr that is declared as struct.

However, it is possible to repro it with a primitive without crossgen:

    // Make both structs with >4 fields to prevent struct promoting and force byref passing.
    struct CastFromStruct
    {
        public int primField;
        int b;
        int c;
        int d;
        int e;
    }

    struct CastToStruct
    {
        public bool a;
        public bool b;
        public bool c;
        public bool d;
    }

    // our src local var has to be an implicit byref, otherwise we won't try to recover struct handle from it.
    [MethodImpl(MethodImplOptions.NoInlining)]
    static CastToStruct UnsafeCastFromAPrimitiveStructFieldToStruct(CastFromStruct impByRefStruct)
    {
        CastToStruct to = new CastToStruct();
        // Force `IND struct(LCL_VAR   byref V00 arg0 Zero Fseq[primField])` where `primField` is a primitive type.
        to = Unsafe.As<int, CastToStruct>(ref impByRefStruct.primField);
        return to;
    }
    public static int Main()
    {
        CastFromStruct s = new CastFromStruct();
        UnsafeCastFromAPrimitiveStructFieldToStruct(s);
        return 100;
    }

but it led me to a bigger problem: we lose struct size on IND node when we have an unsafe cast, so if we create a CSE var out of it it could have a wrong size and lead to incorrect bytes being read, but it a separate issue, I will try to create a non-stress repro and open it.

stress repro

class TestStructs
{

    struct A
    {
        public int a;
    }

    // Make both structs with >4 fields to prevent struct promoting and force byref passing.
    struct CastFromStruct
    {
        public A primField;
        int b;
        int c;
        int d;
        int e;
    }

    struct CastToStruct
    {
        public bool a;
        public bool b;
        public bool c;
        public bool d;
        public bool e;
    }

    struct CastToStruct2
    {
        public bool a;
        public bool b;
        public bool c;
        public bool d;
        public bool e;
        public bool f;
    }

    struct CastToStruct3
    {
        public bool a;
        public bool b;
        public bool c;
        public bool d;
        public bool e;
        public bool f;
    }

    // our src local var has to be an implicit byref, otherwise we won't try to recover struct handle from it.
    [MethodImpl(MethodImplOptions.NoInlining)]
    static bool UnsafeCastFromAPrimitiveStructFieldToStruct(CastFromStruct impByRefStruct, int b)
    {
        CastToStruct to1 = new CastToStruct();
        // Force `IND struct(LCL_VAR   byref V00 arg0 Zero Fseq[primField])` where `primField` is a primitive type.
        to1 = Unsafe.As<A, CastToStruct>(ref impByRefStruct.primField);
        CastToStruct to4 = new CastToStruct();
        to4 = Unsafe.As<A, CastToStruct>(ref impByRefStruct.primField);
        CastToStruct to5 = new CastToStruct();
        to5 = Unsafe.As<A, CastToStruct>(ref impByRefStruct.primField);

        CastToStruct2 to2 = new CastToStruct2();
        to2 = Unsafe.As<A, CastToStruct2>(ref impByRefStruct.primField);

        CastToStruct3 to3 = new CastToStruct3();
        to3 = Unsafe.As<A, CastToStruct3>(ref impByRefStruct.primField);


        if (b == 0)
        {
            bool r = to1.a;
            r &= to2.a;
            r &= to3.a;
            r &= to4.a;
            r &= to5.a;
            return r;
        }
        return false;
    }
    public static int Main()
    {
        CastFromStruct s = new CastFromStruct();
        UnsafeCastFromAPrimitiveStructFieldToStruct(s, 1);
        return 100;
    }
}

will produce IR (with aggressive CSE):

***** BB01
STMT00002 (IL   ???...  ???)
N008 ( 14, 10) [000013] -A--G---R---              *  ASG       struct (copy) $VN.Void
N007 (  3,  2) [000011] D------N----              +--*  LCL_VAR   struct<CastToStruct, 5> V02 loc0         d:2
N006 ( 10,  7) [000146] -A--G-------              \--*  COMMA     struct <l:$140, c:$180>
N004 (  7,  5) [000144] -A--G---R---                 +--*  ASG       struct (copy) $VN.Void
N003 (  3,  2) [000142] D------N----                 |  +--*  LCL_VAR   struct<A, 4> V09 cse0         d:1 <l:$140, c:$180>
N002 (  3,  2) [000010] n---G--N----                 |  \--*  IND       struct <l:$140, c:$180>
N001 (  1,  1) [000114] -------N----                 |     \--*  LCL_VAR   byref  V00 arg0         u:1 Zero Fseq[primField] $80
N005 (  3,  2) [000145] ------------                 \--*  LCL_VAR   struct<A, 4> V09 cse0         u:1 <l:$140, c:$180>

***** BB01
STMT00005 (IL   ???...  ???)
N003 (  7,  5) [000027] -A--G---R---              *  ASG       struct (copy) $VN.Void
N002 (  3,  2) [000025] D------N----              +--*  LCL_VAR   struct<CastToStruct, 5> V03 loc1         d:2
N001 (  3,  2) [000147] ------------              \--*  LCL_VAR   struct<A, 4> V09 cse0         u:1 <l:$140, c:$180>

***** BB01
STMT00008 (IL   ???...  ???)
N003 (  7,  5) [000041] -A--G---R---              *  ASG       struct (copy) $VN.Void
N002 (  3,  2) [000039] D------N----              +--*  LCL_VAR   struct<CastToStruct, 5> V04 loc2         d:2
N001 (  3,  2) [000148] ------------              \--*  LCL_VAR   struct<A, 4> V09 cse0         u:1 <l:$140, c:$180>

***** BB01
STMT00011 (IL   ???...  ???)
N003 (  7,  5) [000055] -A--G---R---              *  ASG       struct (copy) $VN.Void
N002 (  3,  2) [000053] D------N----              +--*  LCL_VAR   struct<CastToStruct2, 6> V05 loc3         d:2
N001 (  3,  2) [000149] ------------              \--*  LCL_VAR   struct<A, 4> V09 cse0         u:1 <l:$140, c:$180>

***** BB01
STMT00014 (IL   ???...  ???)
N003 (  7,  5) [000069] -A--G---R---              *  ASG       struct (copy) $VN.Void
N002 (  3,  2) [000067] D------N----              +--*  LCL_VAR   struct<CastToStruct3, 6> V06 loc4         d:2
N001 (  3,  2) [000150] ------------              \--*  LCL_VAR   struct<A, 4> V09 cse0         u:1 <l:$140, c:$180>

***** BB01
STMT00015 (IL 0x083...0x084)
N004 (  5,  5) [000073] ------------              *  JTRUE     void
N003 (  3,  3) [000072] J------N----              \--*  NE        int    $1c0
N001 (  1,  1) [000070] ------------                 +--*  LCL_VAR   int    V01 arg1         u:1 (last use) $c0
N002 (  1,  1) [000071] ------------                 \--*  CNS_INT   int    0 $40

that is not correct, we are reading 5/6 bytes from a 4 bytes CSE copy.

[CarolEidt]

it is not clear for me why ZapInfo::getFieldType returns CORINFO_TYPE_NATIVEINT when asking about DynamicFieldHandleInfo.DeclaringType that is IntPtr that is declared as struct.

I believe that this is because IntPtr is special-cased as it specifically is CORINFO_TYPE_NATIVEINT