Fixing bug 445667: AV in universal transition thunk.

     The problem was that dynamically created MD arrays were getting the "HasSealedVtable" flag without really having a sealed vtable. The flag was coming from the template type. MD arrays should really not have a sealed vtable, and the runtime should use the dispatchmap/sealedvtable of the System.Array basetype instead.
     These changes fix the binder to not emit dispatch maps and sealed vtables for md arrays.

     Cleanup: Remove unused IsDynamicTypeWithSealedVTableEntriesFlag rare flag.

[tfs-changeset: 1661447]

5 files changed, 40 insertions, 54 deletions

diff --git a/src/Common/src/Internal/Runtime/EEType.Constants.cs b/src/Common/src/Internal/Runtime/EEType.Constants.cs
index 63839949b..85309ed17 100644
--- a/src/Common/src/Internal/Runtime/EEType.Constants.cs
+++ b/src/Common/src/Internal/Runtime/EEType.Constants.cs
@@ -141,11 +141,9 @@ namespace Internal.Runtime
         HasCctorFlag = 0x0000020,
 
         /// <summary>
-        /// This EEType has sealed vtable entries (note that this flag is only used for
-        /// dynamically created types because they always have an optional field (hence the
-        /// very explicit flag name).
+        /// Old unused flag
         /// </summary>
-        IsDynamicTypeWithSealedVTableEntriesFlag = 0x00000040,
+        UNUSED = 0x00000040,
 
         /// <summary>
         /// This EEType was constructed from a universal canonical template, and has
@@ -160,8 +158,6 @@ namespace Internal.Runtime
 
         /// <summary>
         /// This EEType has sealed vtable entries
-        /// This is for statically generated types - we need two different flags because
-        /// the sealed vtable entries are reached in different ways in the static and dynamic case
         /// </summary>
         HasSealedVTableEntriesFlag = 0x00000200,
 
diff --git a/src/Common/src/Internal/Runtime/EEType.cs b/src/Common/src/Internal/Runtime/EEType.cs
index 8fa5cd502..a8956b6a1 100644
--- a/src/Common/src/Internal/Runtime/EEType.cs
+++ b/src/Common/src/Internal/Runtime/EEType.cs
@@ -1005,6 +1005,7 @@ namespace Internal.Runtime
         internal IntPtr GetSealedVirtualSlot(UInt16 slotNumber)
         {
             Debug.Assert(!IsNullable);
+            Debug.Assert((RareFlags & EETypeRareFlags.HasSealedVTableEntriesFlag) != 0);
 
             fixed (EEType* pThis = &this)
             {
@@ -1315,19 +1316,21 @@ namespace Internal.Runtime
             if (eField == EETypeField.ETF_SealedVirtualSlots)
                 return cbOffset;
 
-            if (IsNullable || (RareFlags & EETypeRareFlags.IsDynamicTypeWithSealedVTableEntriesFlag) != 0)
+            if (IsNullable)
                 cbOffset += (UInt32)IntPtr.Size;
 
+            EETypeRareFlags rareFlags = RareFlags;
+
             // in the case of sealed vtable entries on static types, we have a UInt sized relative pointer
-            if ((RareFlags & EETypeRareFlags.HasSealedVTableEntriesFlag) != 0)
-                cbOffset += 4;
+            if ((rareFlags & EETypeRareFlags.HasSealedVTableEntriesFlag) != 0)
+                cbOffset += (IsDynamicType ? (UInt32)IntPtr.Size : 4);
 
             if (eField == EETypeField.ETF_DynamicDispatchMap)
             {
                 Debug.Assert(IsDynamicType);
                 return cbOffset;
             }
-            if ((RareFlags & EETypeRareFlags.HasDynamicallyAllocatedDispatchMapFlag) != 0)
+            if ((rareFlags & EETypeRareFlags.HasDynamicallyAllocatedDispatchMapFlag) != 0)
                 cbOffset += (UInt32)IntPtr.Size;
 
             if (eField == EETypeField.ETF_GenericDefinition)
@@ -1351,7 +1354,7 @@ namespace Internal.Runtime
                 return cbOffset;
             }
 
-            if ((RareFlags & EETypeRareFlags.HasDynamicModuleFlag) != 0)
+            if ((rareFlags & EETypeRareFlags.HasDynamicModuleFlag) != 0)
                 cbOffset += (UInt32)IntPtr.Size;
 
             if (eField == EETypeField.ETF_DynamicTemplateType)
@@ -1364,26 +1367,26 @@ namespace Internal.Runtime
 
             if (eField == EETypeField.ETF_DynamicGcStatics)
             {
-                Debug.Assert((RareFlags & EETypeRareFlags.IsDynamicTypeWithGcStatics) != 0);
+                Debug.Assert((rareFlags & EETypeRareFlags.IsDynamicTypeWithGcStatics) != 0);
                 return cbOffset;
             }
-            if ((RareFlags & EETypeRareFlags.IsDynamicTypeWithGcStatics) != 0)
+            if ((rareFlags & EETypeRareFlags.IsDynamicTypeWithGcStatics) != 0)
                 cbOffset += (UInt32)IntPtr.Size;
 
             if (eField == EETypeField.ETF_DynamicNonGcStatics)
             {
-                Debug.Assert((RareFlags & EETypeRareFlags.IsDynamicTypeWithNonGcStatics) != 0);
+                Debug.Assert((rareFlags & EETypeRareFlags.IsDynamicTypeWithNonGcStatics) != 0);
                 return cbOffset;
             }
-            if ((RareFlags & EETypeRareFlags.IsDynamicTypeWithNonGcStatics) != 0)
+            if ((rareFlags & EETypeRareFlags.IsDynamicTypeWithNonGcStatics) != 0)
                 cbOffset += (UInt32)IntPtr.Size;
 
             if (eField == EETypeField.ETF_DynamicThreadStaticOffset)
             {
-                Debug.Assert((RareFlags & EETypeRareFlags.IsDynamicTypeWithThreadStatics) != 0);
+                Debug.Assert((rareFlags & EETypeRareFlags.IsDynamicTypeWithThreadStatics) != 0);
                 return cbOffset;
             }
-            if ((RareFlags & EETypeRareFlags.IsDynamicTypeWithThreadStatics) != 0)
+            if ((rareFlags & EETypeRareFlags.IsDynamicTypeWithThreadStatics) != 0)
                 cbOffset += 4;
 
             Debug.Assert(false, "Unknown EEType field type");
diff --git a/src/Native/Runtime/inc/eetype.h b/src/Native/Runtime/inc/eetype.h
index 409b1f28b..65d082f9a 100644
--- a/src/Native/Runtime/inc/eetype.h
+++ b/src/Native/Runtime/inc/eetype.h
@@ -302,10 +302,8 @@ public:
         // This EEType has a Class Constructor
         HasCctorFlag            = 0x0000020,
 
-        // This EEType has sealed vtable entries (note that this flag is only used for
-        // dynamically created types because they always have an optional field (hence the
-        // very explicit flag name).
-        IsDynamicTypeWithSealedVTableEntriesFlag    = 0x00000040,
+        // Old unused flag
+        UNUSED                  = 0x00000040,
 
         // This EEType was constructed from a universal canonical template, and has
         // its own dynamically created DispatchMap (does not use the DispatchMap of its template type)
@@ -315,8 +313,6 @@ public:
         IsHFAFlag                           = 0x00000100,
 
         // This EEType has sealed vtable entries
-        // This is for statically generated types - we need two different flags because
-        // the sealed vtable entries are reached in different ways in the static and dynamic case
         HasSealedVTableEntriesFlag          = 0x00000200,
 
         // This dynamically created type has gc statics
diff --git a/src/Native/Runtime/inc/eetype.inl b/src/Native/Runtime/inc/eetype.inl
index 0b4908f07..e286d6234 100644
--- a/src/Native/Runtime/inc/eetype.inl
+++ b/src/Native/Runtime/inc/eetype.inl
@@ -46,30 +46,21 @@ inline PTR_UInt8 FollowRelativePointer(const Int32 *pDist)
 inline PTR_Code EEType::get_SealedVirtualSlot(UInt16 slotNumber)
 {
     ASSERT(!IsNullable());
+    ASSERT((get_RareFlags() & HasSealedVTableEntriesFlag) != 0);
 
     if (IsDynamicType())
     {
-        if ((get_RareFlags() & IsDynamicTypeWithSealedVTableEntriesFlag) != 0)
-        {
-            UInt32 cbSealedVirtualSlotsTypeOffset = GetFieldOffset(ETF_SealedVirtualSlots);
-
-            PTR_PTR_Code pSealedVirtualsSlotTable = *(PTR_PTR_Code*)((PTR_UInt8)this + cbSealedVirtualSlotsTypeOffset);
-
-            return pSealedVirtualsSlotTable[slotNumber];
-        }
-        else
-        {
-            return get_DynamicTemplateType()->get_SealedVirtualSlot(slotNumber);
-        }
+        UInt32 cbSealedVirtualSlotsTypeOffset = GetFieldOffset(ETF_SealedVirtualSlots);
+        PTR_PTR_Code pSealedVirtualsSlotTable = *(PTR_PTR_Code*)((PTR_UInt8)this + cbSealedVirtualSlotsTypeOffset);
+        return pSealedVirtualsSlotTable[slotNumber];
+    }
+    else
+    {
+        UInt32 cbSealedVirtualSlotsTypeOffset = GetFieldOffset(ETF_SealedVirtualSlots);
+        PTR_Int32 pSealedVirtualsSlotTable = (PTR_Int32)FollowRelativePointer((PTR_Int32)((PTR_UInt8)this + cbSealedVirtualSlotsTypeOffset));
+        PTR_Code result = FollowRelativePointer(&pSealedVirtualsSlotTable[slotNumber]);
+        return result;
     }
-
-    UInt32 cbSealedVirtualSlotsTypeOffset = GetFieldOffset(ETF_SealedVirtualSlots);
-
-    PTR_Int32 pSealedVirtualsSlotTable = (PTR_Int32)FollowRelativePointer((PTR_Int32)((PTR_UInt8)this + cbSealedVirtualSlotsTypeOffset));
-
-    PTR_Code result = FollowRelativePointer(&pSealedVirtualsSlotTable[slotNumber]);
-
-    return result;
 }
 #endif // !BINDER && !DACCESS_COMPILE
 
@@ -565,7 +556,9 @@ inline DynamicModule * EEType::get_DynamicModule()
         ((ArrayClass*)pMT->GetClass())->GetApproxArrayElementTypeHandle().AsMethodTable() :
         NULL;
 
-    bool fHasSealedVirtuals = pMT->GetNumVirtuals() < (pMT->GetNumVtableSlots() + pMT->GetNumAdditionalVtableSlots());
+    bool isMdArray = pMT->IsArray() && ((ArrayClass*)pMT->GetClass())->GetRank() > 0;
+    bool fHasSealedVirtuals = !isMdArray && (pMT->GetNumVirtuals() < (pMT->GetNumVtableSlots() + pMT->GetNumAdditionalVtableSlots()));
+
     return
         // Do we need a padding size for value types or unsealed classes? that could be unboxed?
         (!pMT->IsArray() && 
@@ -581,7 +574,7 @@ inline DynamicModule * EEType::get_DynamicModule()
         (pMT->IsHFA()) ||
 #endif
         // Do we need a DispatchMap?
-        (pMT->GetDispatchMap() != NULL && !pMT->GetDispatchMap()->IsEmpty()) ||
+        (!isMdArray && pMT->GetDispatchMap() != NULL && !pMT->GetDispatchMap()->IsEmpty()) ||
         // Do we need to cache ICastable method vtable slots?
         (pMT->IsICastable()) ||
         // Is the class a Nullable<T> instantiation (need to store the flag and possibly a field offset)?
@@ -714,13 +707,14 @@ __forceinline UInt32 EEType::GetFieldOffset(EETypeField eField)
 
     // Binder does not use DynamicTemplateType
 #ifndef BINDER
-    UInt32 rareFlags = get_RareFlags();
-    if (IsNullable() || ((rareFlags & IsDynamicTypeWithSealedVTableEntriesFlag) != 0))
+    if (IsNullable())
         cbOffset += sizeof(UIntTarget);
 
+    UInt32 rareFlags = get_RareFlags();
+
     // in the case of sealed vtable entries on static types, we have a UInt sized relative pointer
     if (rareFlags & HasSealedVTableEntriesFlag)
-        cbOffset += sizeof(UInt32);
+        cbOffset += (IsDynamicType() ? sizeof(UIntTarget) : sizeof(UInt32));
 
     if (eField == ETF_DynamicDispatchMap)
     {
@@ -748,11 +742,11 @@ __forceinline UInt32 EEType::GetFieldOffset(EETypeField eField)
 
     if (eField == ETF_DynamicModule)
     {
-        ASSERT((get_RareFlags() & HasDynamicModuleFlag) != 0);
+        ASSERT((rareFlags & HasDynamicModuleFlag) != 0);
         return cbOffset;
     }
 
-    if ((get_RareFlags() & HasDynamicModuleFlag) != 0)
+    if ((rareFlags & HasDynamicModuleFlag) != 0)
         cbOffset += sizeof(UIntTarget);
 
     if (eField == ETF_DynamicTemplateType)
diff --git a/src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs b/src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
index 5ca8d36b0..e7b4b31dc 100644
--- a/src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
+++ b/src/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
@@ -296,12 +296,9 @@ namespace Internal.Runtime.TypeLoader
                     UInt32 rareFlags = optionalFields.GetFieldValue(EETypeOptionalFieldTag.RareFlags, 0);
                     rareFlags |= (uint)EETypeRareFlags.IsDynamicTypeFlag;          // Set the IsDynamicTypeFlag
                     rareFlags &= ~(uint)EETypeRareFlags.NullableTypeViaIATFlag;    // Remove the NullableTypeViaIATFlag flag
-                    rareFlags &= ~(uint)EETypeRareFlags.HasSealedVTableEntriesFlag;// Remove the HasSealedVTableEntriesFlag
-                                                                                   // we'll set IsDynamicTypeWithSealedVTableEntriesFlag instead
 
-                    // Set the IsDynamicTypeWithSealedVTableEntriesFlag if needed
                     if (state.NumSealedVTableEntries > 0)
-                        rareFlags |= (uint)EETypeRareFlags.IsDynamicTypeWithSealedVTableEntriesFlag;
+                        rareFlags |= (uint)EETypeRareFlags.HasSealedVTableEntriesFlag;
 
                     if (requiresDynamicDispatchMap)
                         rareFlags |= (uint)EETypeRareFlags.HasDynamicallyAllocatedDispatchMapFlag;