Correctly handle gc desc for explicit layout type which inherits from another type...

- Fix memory corruption issue when an explicit layout class derived from a class with a gc pointer in it

diff --git a/src/coreclr/src/vm/methodtablebuilder.cpp b/src/coreclr/src/vm/methodtablebuilder.cpp
index c50c02d..fc14d39 100644 (file)
--- a/src/coreclr/src/vm/methodtablebuilder.cpp
+++ b/src/coreclr/src/vm/methodtablebuilder.cpp
@@ -8764,8 +8764,10 @@ MethodTableBuilder::HandleGCForExplicitLayout()
         if (bmtParent->NumParentPointerSeries != 0)
         {
             size_t ParentGCSize = CGCDesc::ComputeSize(bmtParent->NumParentPointerSeries);
-            memcpy( (PVOID) (((BYTE*) pMT) - ParentGCSize),  (PVOID) (((BYTE*) GetParentMethodTable()) - ParentGCSize), ParentGCSize - sizeof(UINT) );
-
+            memcpy( (PVOID) (((BYTE*) pMT) - ParentGCSize),
+                    (PVOID) (((BYTE*) GetParentMethodTable()) - ParentGCSize),
+                    ParentGCSize - sizeof(size_t)   // sizeof(size_t) is the NumSeries count
+                  );
         }
 
         UINT32 dwInstanceSliceOffset = AlignUp(HasParent() ? GetParentMethodTable()->GetNumInstanceFieldBytes() : 0, TARGET_POINTER_SIZE);
@@ -8780,6 +8782,16 @@ MethodTableBuilder::HandleGCForExplicitLayout()
             pSeries->SetSeriesOffset(bmtGCSeries->pSeries[i].offset + OBJECT_SIZE + dwInstanceSliceOffset);
             pSeries++;
         }
+
+        // Adjust the inherited series - since the base size has increased by "# new field instance bytes", we need to
+        // subtract that from all the series (since the series always has BaseSize subtracted for it - see gcdesc.h)
+        CGCDescSeries *pHighest = CGCDesc::GetCGCDescFromMT(pMT)->GetHighestSeries();
+        while (pSeries <= pHighest)
+        {
+            CONSISTENCY_CHECK(CheckPointer(GetParentMethodTable()));
+            pSeries->SetSeriesSize( pSeries->GetSeriesSize() - ((size_t) pMT->GetBaseSize() - (size_t) GetParentMethodTable()->GetBaseSize()) );
+            pSeries++;
+        }
     }
 
     delete [] bmtGCSeries->pSeries;

diff --git a/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.cs b/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.cs
new file mode 100644 (file)
index 0000000..183c286
--- /dev/null
+++ b/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.cs
@@ -0,0 +1,56 @@
+using System;
+using System.Reflection;
+using System.Runtime.InteropServices;
+
+namespace ClrIssueRepro
+{
+    // If you remove '[StructLayout(LayoutKind.Sequential)]', you get:
+    // Unhandled exception. System.TypeLoadException: 
+    // Could not load type 'ClrIssueRepro.GenInt' from assembly '...' 
+    //    because the format is invalid.
+    //  at ClrIssueRepro.Program.Main(String[] args)
+    [StructLayout(LayoutKind.Sequential)]
+    public class GenBase<T>
+    {
+        public string _string0 = "string0";
+        public string _string1 = "string1";
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    public class GenInt : GenBase<int>
+    {
+        // Commenting out either one of these fields fixes things!?
+        [FieldOffset(0)] public string _sstring0 = "string0";
+        [FieldOffset(16)] public string _sstring1 = "string1";
+    }
+
+    // This works! (it's GenInt with [StructLayout(LayoutKind.Explicit)] and [FieldOffset(..)] removed)
+    public class GenIntNormal : GenBase<int>
+    {
+        public string _sstring0 = "string0";
+        public string _sstring1 = "string1";
+    }
+
+    class Program
+    {
+        static int Main(string[] args)
+        {
+            // If you comment this line out, you get
+            //    Unhandled exception. System.TypeLoadException: 
+            //    Could not load type 'ClrIssueRepro.GenInt' from assembly '...'
+            //    at ClrIssueRepro.Program.Main(String[] args)
+            // in Debug and Release builds?!
+            Type type = typeof(GenInt);
+
+            object instance = new GenInt();
+
+            // GenIntNormal has the same fields as GenInt, but has
+            // [StructLayout(LayoutKind.Explicit)] and [FieldOffset(..)] REMOVED
+            //object instance = new GenIntNormal(); // works fine!!
+
+            string instType = instance.GetType().ToString();
+            Console.WriteLine(instType);
+            return "ClrIssueRepro.GenInt" == instType ? 100 : 0;
+        }
+    }
+}
\ No newline at end of file

diff --git a/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.csproj b/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.csproj
new file mode 100644 (file)
index 0000000..0a3e92e
--- /dev/null
+++ b/src/tests/Loader/classloader/explicitlayout/Regressions/13362/Github13362.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="Github13362.cs" />
+  </ItemGroup>
+</Project>