Type verification in tests (#40186)

- Enable type layout verification in crossgen2 test passes

Product bugs found/fixed
- Fix type blittability issue found during enabling this test.
  - HFA encoding was not correct for Arm64, and did not have R2R defined constants

- Fix type layout verification infrastructure issues
  - Do not generate type layout checks or verification if the offset is 24 bits or larger
  - Only load approx enclosing type when verifying, instead of enclosing type (this avoids asserts)

diff --git a/src/coreclr/src/inc/readytorun.h b/src/coreclr/src/inc/readytorun.h
index b883f4b558e9b5a0ebbef0a70705612a8364d347..e1719a2843eb74d4312f894f971b72bc424d747e 100644 (file)
--- a/src/coreclr/src/inc/readytorun.h
+++ b/src/coreclr/src/inc/readytorun.h
@@ -395,4 +395,13 @@ enum ReadyToRunRuntimeConstants : DWORD
     READYTORUN_ReversePInvokeTransitionFrameSizeInPointerUnits = 2
 };
 
+enum ReadyToRunHFAElemType : DWORD
+{
+    READYTORUN_HFA_ELEMTYPE_None = 0,
+    READYTORUN_HFA_ELEMTYPE_Float32 = 1,
+    READYTORUN_HFA_ELEMTYPE_Float64 = 2,
+    READYTORUN_HFA_ELEMTYPE_Vector64 = 3,
+    READYTORUN_HFA_ELEMTYPE_Vector128 = 4,
+};
+
 #endif // __READYTORUN_H__

diff --git a/src/coreclr/src/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/src/tools/Common/Internal/Runtime/ReadyToRunConstants.cs
index ce6de5de37cac427bddf413d3d5c2fe86c1bddeb..6d92e7b5d1227c6d4d48506bd232310319c32f1a 100644 (file)
--- a/src/coreclr/src/tools/Common/Internal/Runtime/ReadyToRunConstants.cs
+++ b/src/coreclr/src/tools/Common/Internal/Runtime/ReadyToRunConstants.cs
@@ -314,6 +314,17 @@ namespace Internal.ReadyToRunConstants
         TypeHandleToRuntimeTypeHandle,
     }
 
+    // Enum used for HFA type recognition.
+    // Supported across architectures, so that it can be used in altjits and cross-compilation.
+    public enum ReadyToRunHFAElemType
+    {
+        None = 0,
+        Float32 = 1,
+        Float64 = 2,
+        Vector64 = 3,
+        Vector128 = 4,
+    }
+
     public static class ReadyToRunRuntimeConstants
     {
         public const int READYTORUN_PInvokeTransitionFrameSizeInPointerUnits = 11;

diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs
index ebeb537f5f152f1d71b0ab871b1861e6da6c3fa5..eb5431f29e529a2056aca3132d72ce8b13b2f1b9 100644 (file)
--- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs
+++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/FieldFixupSignature.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics;
 
 using Internal.JitInterface;
 using Internal.Text;
@@ -13,17 +14,19 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
     public class FieldFixupSignature : Signature
     {
+        public const int MaxCheckableOffset = 0x1FFFFFFF;
         private readonly ReadyToRunFixupKind _fixupKind;
 
         private readonly FieldDesc _fieldDesc;
 
-        public FieldFixupSignature(ReadyToRunFixupKind fixupKind, FieldDesc fieldDesc)
+        public FieldFixupSignature(ReadyToRunFixupKind fixupKind, FieldDesc fieldDesc, NodeFactory factory)
         {
             _fixupKind = fixupKind;
             _fieldDesc = fieldDesc;
 
             // Ensure types in signature are loadable and resolvable, otherwise we'll fail later while emitting the signature
             ((CompilerTypeSystemContext)fieldDesc.Context).EnsureLoadableType(fieldDesc.OwningType);
+            Debug.Assert(factory.SignatureContext.GetTargetModule(_fieldDesc) != null);
         }
 
         public override int ClassCode => 271828182;

diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
index 0bc86c7794cbf98b25b3ac0c489a1ca9c09c329b..ea21b730330b9e5480f598e855ccc4a9e5f72dd6 100644 (file)
--- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
+++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/TypeFixupSignature.cs
@@ -81,16 +81,16 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
             if (defType.IsHomogeneousAggregate)
             {
-                CorElementType elementType = (defType.ValueTypeShapeCharacteristics & ValueTypeShapeCharacteristics.AggregateMask) switch
+                ReadyToRunHFAElemType hfaElementType = (defType.ValueTypeShapeCharacteristics & ValueTypeShapeCharacteristics.AggregateMask) switch
                 {
-                    ValueTypeShapeCharacteristics.Float32Aggregate => CorElementType.ELEMENT_TYPE_R4,
-                    ValueTypeShapeCharacteristics.Float64Aggregate => CorElementType.ELEMENT_TYPE_R8,
-                    ValueTypeShapeCharacteristics.Vector64Aggregate => CorElementType.ELEMENT_TYPE_R8,
+                    ValueTypeShapeCharacteristics.Float32Aggregate => ReadyToRunHFAElemType.Float32,
+                    ValueTypeShapeCharacteristics.Float64Aggregate => ReadyToRunHFAElemType.Float64,
+                    ValueTypeShapeCharacteristics.Vector64Aggregate => ReadyToRunHFAElemType.Vector64,
                     // See MethodTable::GetHFAType
-                    ValueTypeShapeCharacteristics.Vector128Aggregate => CorElementType.ELEMENT_TYPE_VALUETYPE,
-                    _ => CorElementType.Invalid
+                    ValueTypeShapeCharacteristics.Vector128Aggregate => ReadyToRunHFAElemType.Vector128,
+                    _ => throw new NotSupportedException()
                 };
-                dataBuilder.EmitUInt((uint)elementType);
+                dataBuilder.EmitUInt((uint)hfaElementType);
             }
             
             if (alignment != pointerSize)

diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
index 7ae68230d8a692637a9b2c793ae51ad7c29a0e4e..1178d4372c33a080fcb0eedabd75206129ae093a 100644 (file)
--- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
+++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRunSymbolNodeFactory.cs
@@ -70,7 +70,7 @@ namespace ILCompiler.DependencyAnalysis
                     _codegenNodeFactory,
                     _codegenNodeFactory.HelperImports,
                     ReadyToRunHelper.DelayLoad_Helper,
-                    new FieldFixupSignature(ReadyToRunFixupKind.FieldAddress, key)
+                    new FieldFixupSignature(ReadyToRunFixupKind.FieldAddress, key, _codegenNodeFactory)
                 );
             });
 
@@ -78,7 +78,7 @@ namespace ILCompiler.DependencyAnalysis
             {
                 return new PrecodeHelperImport(
                     _codegenNodeFactory,
-                    new FieldFixupSignature(ReadyToRunFixupKind.FieldOffset, key)
+                    new FieldFixupSignature(ReadyToRunFixupKind.FieldOffset, key, _codegenNodeFactory)
                 );
             });
 
@@ -94,7 +94,7 @@ namespace ILCompiler.DependencyAnalysis
             {
                 return new PrecodeHelperImport(
                     _codegenNodeFactory,
-                    new FieldFixupSignature(_verifyTypeAndFieldLayout ? ReadyToRunFixupKind.Verify_FieldOffset : ReadyToRunFixupKind.Check_FieldOffset, key)
+                    new FieldFixupSignature(_verifyTypeAndFieldLayout ? ReadyToRunFixupKind.Verify_FieldOffset : ReadyToRunFixupKind.Check_FieldOffset, key, _codegenNodeFactory)
                 );
             });
 
@@ -356,7 +356,7 @@ namespace ILCompiler.DependencyAnalysis
         {
             return new PrecodeHelperImport(
                 _codegenNodeFactory,
-                new FieldFixupSignature(ReadyToRunFixupKind.FieldHandle, field));
+                new FieldFixupSignature(ReadyToRunFixupKind.FieldHandle, field, _codegenNodeFactory));
         }
 
         private ISymbolNode CreateCctorTrigger(TypeDesc type)

diff --git a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
index 2c2085635f3482d7862e6edca08c2bc7a870e75f..57c4b738d690c179f0dd150ddfc7f37b34299bf5 100644 (file)
--- a/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
+++ b/src/coreclr/src/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs
@@ -1012,7 +1012,7 @@ namespace Internal.JitInterface
                             CorInfoHelpFunc.CORINFO_HELP_GETGENERICS_NONGCSTATIC_BASE);
                     }
 
-                    if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
+                    if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && (fieldOffset <= FieldFixupSignature.MaxCheckableOffset))
                     {
                         // ENCODE_CHECK_FIELD_OFFSET
                         _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));
@@ -1066,7 +1066,7 @@ namespace Internal.JitInterface
                     else
                     if (helperId != ReadyToRunHelperId.Invalid)
                     {
-                        if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
+                        if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && (fieldOffset <= FieldFixupSignature.MaxCheckableOffset))
                         {
                             // ENCODE_CHECK_FIELD_OFFSET
                             _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));
@@ -1922,7 +1922,7 @@ namespace Internal.JitInterface
             if (!type.IsValueType)
                 return false;
 
-            return !_compilation.IsLayoutFixedInCurrentVersionBubble(type) || _compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout;
+            return !_compilation.IsLayoutFixedInCurrentVersionBubble(type) || (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && !((MetadataType)type).IsNonVersionable());
         }
 
         private bool HasLayoutMetadata(TypeDesc type)
@@ -1963,6 +1963,9 @@ namespace Internal.JitInterface
                 if (pMT.IsValueType)
                 {
                     // ENCODE_CHECK_FIELD_OFFSET
+                    if (pResult->offset > FieldFixupSignature.MaxCheckableOffset)
+                        throw new RequiresRuntimeJitException(callerMethod.ToString() + " -> " + field.ToString());
+
                     _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));
                     // No-op other than generating the check field offset fixup
                 }
@@ -1978,7 +1981,7 @@ namespace Internal.JitInterface
             }
             else if (pMT.IsValueType)
             {
-                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
+                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && !callerMethod.IsNonVersionable() && (pResult->offset <= FieldFixupSignature.MaxCheckableOffset))
                 {
                     // ENCODE_CHECK_FIELD_OFFSET
                     _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));
@@ -1987,7 +1990,7 @@ namespace Internal.JitInterface
             }
             else if (_compilation.IsInheritanceChainLayoutFixedInCurrentVersionBubble(pMT.BaseType))
             {
-                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
+                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && !callerMethod.IsNonVersionable() && (pResult->offset <= FieldFixupSignature.MaxCheckableOffset))
                 {
                     // ENCODE_CHECK_FIELD_OFFSET
                     _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));
@@ -2009,7 +2012,7 @@ namespace Internal.JitInterface
             {
                 PreventRecursiveFieldInlinesOutsideVersionBubble(field, callerMethod);
 
-                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout)
+                if (_compilation.SymbolNodeFactory.VerifyTypeAndFieldLayout && !callerMethod.IsNonVersionable() && (pResult->offset <= FieldFixupSignature.MaxCheckableOffset))
                 {
                     // ENCODE_CHECK_FIELD_OFFSET
                     _methodCodeNode.Fixups.Add(_compilation.SymbolNodeFactory.CheckFieldOffset(field));

diff --git a/src/coreclr/src/vm/fieldmarshaler.cpp b/src/coreclr/src/vm/fieldmarshaler.cpp
index 4b1b7abc39caf160ecb56a6a8af0e67d2d595bd0..47a0f2693f14b0480fd43f19fd24977951bab485 100644 (file)
--- a/src/coreclr/src/vm/fieldmarshaler.cpp
+++ b/src/coreclr/src/vm/fieldmarshaler.cpp
@@ -277,7 +277,7 @@ bool IsFieldBlittable(
             }
             else
             {
-                isBlittable = !valueTypeHandle.GetMethodTable()->HasInstantiation() && valueTypeHandle.GetMethodTable()->IsBlittable();
+                isBlittable = valueTypeHandle.GetMethodTable()->IsBlittable();
             }
             break;
         default:

diff --git a/src/coreclr/src/vm/jitinterface.cpp b/src/coreclr/src/vm/jitinterface.cpp
index 9e3c9180ef0bed26ed56bbdbe78de56006740603..9dc409db9ad5d6c4fccc1d7a56e69aaec1ebe046 100644 (file)
--- a/src/coreclr/src/vm/jitinterface.cpp
+++ b/src/coreclr/src/vm/jitinterface.cpp
@@ -13840,7 +13840,8 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
             DWORD baseOffset = CorSigUncompressData(pBlob);
             DWORD fieldOffset = CorSigUncompressData(pBlob);
             FieldDesc* pField = ZapSig::DecodeField(currentModule, pInfoModule, pBlob);
-            pField->GetEnclosingMethodTable()->CheckRestore();
+            MethodTable *pEnclosingMT = pField->GetApproxEnclosingMethodTable();
+            pEnclosingMT->CheckRestore();
             DWORD actualFieldOffset = pField->GetOffset();
             if (!pField->IsStatic() && !pField->IsFieldOfValueType())
             {
@@ -13849,10 +13850,10 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
 
             DWORD actualBaseOffset = 0;
             if (!pField->IsStatic() && 
-                pField->GetEnclosingMethodTable()->GetParentMethodTable() != NULL &&
-                !pField->GetEnclosingMethodTable()->IsValueType())
+                pEnclosingMT->GetParentMethodTable() != NULL &&
+                !pEnclosingMT->IsValueType())
             {
-                actualBaseOffset = ReadyToRunInfo::GetFieldBaseOffset(pField->GetEnclosingMethodTable());
+                actualBaseOffset = ReadyToRunInfo::GetFieldBaseOffset(pEnclosingMT);
             }
 
             if ((fieldOffset != actualFieldOffset) || (baseOffset != actualBaseOffset))
@@ -13863,7 +13864,7 @@ BOOL LoadDynamicInfoEntry(Module *currentModule,
 
                 SString fatalErrorString;
                 fatalErrorString.Printf(W("Verify_FieldOffset '%s.%s' %d!=%d || %d!=%d"), 
-                    GetFullyQualifiedNameForClassW(pField->GetEnclosingMethodTable()),
+                    GetFullyQualifiedNameForClassW(pEnclosingMT),
                     ssFieldName.GetUnicode(),
                     fieldOffset,
                     actualFieldOffset,

diff --git a/src/coreclr/src/zap/zapreadytorun.cpp b/src/coreclr/src/zap/zapreadytorun.cpp
index e36d4aee6eafcc700e83628cb1517bf8986f414c..d5bb25ab1b6ab2976291363a4a4be13f7c2eeff7 100644 (file)
--- a/src/coreclr/src/zap/zapreadytorun.cpp
+++ b/src/coreclr/src/zap/zapreadytorun.cpp
@@ -851,3 +851,13 @@ static_assert_no_msg((int)READYTORUN_FIXUP_Verify_TypeLayout          == (int)EN
 //
 static_assert_no_msg(sizeof(READYTORUN_EXCEPTION_LOOKUP_TABLE_ENTRY) == sizeof(CORCOMPILE_EXCEPTION_LOOKUP_TABLE_ENTRY));
 static_assert_no_msg(sizeof(READYTORUN_EXCEPTION_CLAUSE) == sizeof(CORCOMPILE_EXCEPTION_CLAUSE));
+
+//
+// ReadyToRunHFAElemType
+//
+static_assert_no_msg((int)READYTORUN_HFA_ELEMTYPE_None      == (int)CORINFO_HFA_ELEM_NONE);
+static_assert_no_msg((int)READYTORUN_HFA_ELEMTYPE_Float32   == (int)CORINFO_HFA_ELEM_FLOAT);
+static_assert_no_msg((int)READYTORUN_HFA_ELEMTYPE_Float64   == (int)CORINFO_HFA_ELEM_DOUBLE);
+static_assert_no_msg((int)READYTORUN_HFA_ELEMTYPE_Vector64  == (int)CORINFO_HFA_ELEM_VECTOR64);
+static_assert_no_msg((int)READYTORUN_HFA_ELEMTYPE_Vector128 == (int)CORINFO_HFA_ELEM_VECTOR128);
+

diff --git a/src/coreclr/tests/src/CLRTest.CrossGen.targets b/src/coreclr/tests/src/CLRTest.CrossGen.targets
index 3460ed5f6f7ada4eed9e07faabfcd9a8be507bdf..3fd4b4a530c9185287aa0ab5dfde64efad792826 100644 (file)
--- a/src/coreclr/tests/src/CLRTest.CrossGen.targets
+++ b/src/coreclr/tests/src/CLRTest.CrossGen.targets
@@ -111,6 +111,7 @@ if [ ! -z ${RunCrossGen2+x} ]%3B then
       echo -r:$CORE_ROOT/System.*.dll>>$__ResponseFile
       echo -r:$CORE_ROOT/Microsoft.*.dll>>$__ResponseFile
       echo -r:$CORE_ROOT/mscorlib.dll>>$__ResponseFile
+      echo --verify-type-and-field-layout>>$__ResponseFile
       echo --targetarch:$(TargetArchitecture)>>$__ResponseFile
       echo -O>>$__ResponseFile
 
@@ -249,6 +250,7 @@ if defined RunCrossGen2 (
     echo !__InputFile!>>!__ResponseFile!
     echo -o:!__OutputFile!>>!__ResponseFile!
     echo --targetarch:$(TargetArchitecture)>>!__ResponseFile!
+    echo --verify-type-and-field-layout>>!__ResponseFile!
     echo -O>>!__ResponseFile!
     echo -r:!CORE_ROOT!\System.*.dll>>!__ResponseFile!
     echo -r:!CORE_ROOT!\Microsoft.*.dll>>!__ResponseFile!

diff --git a/src/tests/readytorun/crossgen2/Program.cs b/src/tests/readytorun/crossgen2/Program.cs
index e2da7433754608021678ad29fa515f049f780052..16ab7df575a90772c9ccf8cd2d327d1f94bc0bcb 100644 (file)
--- a/src/tests/readytorun/crossgen2/Program.cs
+++ b/src/tests/readytorun/crossgen2/Program.cs
@@ -1765,6 +1765,61 @@ internal class Program
         return true;
     }
 
+    [StructLayout(LayoutKind.Explicit)]
+    struct ExplicitLayoutStruct16
+    {
+        [FieldOffset(0)]
+        public int x;
+        [FieldOffset(4)]
+        public int y;
+        [FieldOffset(8)]
+        public int z;
+        [FieldOffset(12)]
+        public int w;
+        public override string ToString() { return $"{x}{y}{z}{w}"; }
+    }
+    struct BlittableStruct<T>
+    {
+       public ExplicitLayoutStruct16 _explict;
+        public override string ToString() { return $"{_explict}"; }
+    }
+
+    struct StructWithGenericBlittableStruct
+    {
+        public BlittableStruct<short> _blittableGeneric;
+        public int _int;
+        public override string ToString() { return $"{_blittableGeneric}{_int}"; }
+    }
+
+    [MethodImpl(MethodImplOptions.AggressiveOptimization)]    
+    private static bool TestWithStructureNonBlittableFieldDueToGenerics_StringCompare(ref StructWithGenericBlittableStruct input)
+    {
+        StructWithGenericBlittableStruct s = new StructWithGenericBlittableStruct();
+        s._blittableGeneric._explict.x = 1;
+        s._blittableGeneric._explict.y = 2;
+        s._blittableGeneric._explict.z = 3;
+        s._blittableGeneric._explict.w = 4;
+        s._int = 5;
+
+        Console.WriteLine(input);
+        Console.WriteLine(s);
+
+        return s.Equals(input) && input.ToString() == "12345";
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static bool TestWithStructureNonBlittableFieldDueToGenerics()
+    {
+        StructWithGenericBlittableStruct s = new StructWithGenericBlittableStruct();
+        s._blittableGeneric._explict.x = 1;
+        s._blittableGeneric._explict.y = 2;
+        s._blittableGeneric._explict.z = 3;
+        s._blittableGeneric._explict.w = 4;
+        s._int = 5;
+
+        return TestWithStructureNonBlittableFieldDueToGenerics_StringCompare(ref s);
+    }
+
     public static int Main(string[] args)
     {
         _passedTests = new List<string>();
@@ -1832,6 +1887,7 @@ internal class Program
         RunTest("GenericLdtokenTest", GenericLdtokenTest());
         RunTest("ArrayLdtokenTests", ArrayLdtokenTests());
         RunTest("TestGenericMDArrayBehavior", TestGenericMDArrayBehavior());
+        RunTest("TestWithStructureNonBlittableFieldDueToGenerics", TestWithStructureNonBlittableFieldDueToGenerics());
 
         File.Delete(TextFileName);