Fix nested non-byref like VC with no pointer fields (#80642)
authorAaron Robinson <arobins@microsoft.com>
Thu, 19 Jan 2023 19:05:25 +0000 (11:05 -0800)
committerGitHub <noreply@github.com>
Thu, 19 Jan 2023 19:05:25 +0000 (11:05 -0800)
* Fix nested non-byref like VC with no pointer fields

src/coreclr/vm/methodtablebuilder.cpp
src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs [new file with mode: 0644]
src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj [new file with mode: 0644]
src/tests/issues.targets

index fae0605..f784662 100644 (file)
@@ -8732,29 +8732,30 @@ MethodTableBuilder::HandleExplicitLayout(
     if (pMT->IsByRefLike())
         return CheckByRefLikeValueClassLayout(pMT, pFieldLayout);
 
-    // This method assumes there is a GC desc associated with the MethodTable.
-    _ASSERTE(pMT->ContainsPointers());
-
     // Build a layout of the value class (vc). Don't know the sizes of all the fields easily, but
     // do know (a) vc is already consistent so don't need to check it's overlaps and
     // (b) size and location of all objectrefs. So build it by setting all non-oref
-    // then fill in the orefs later
+    // then fill in the orefs later if present.
     UINT fieldSize = pMT->GetNumInstanceFieldBytes();
 
     CQuickBytes qb;
     bmtFieldLayoutTag *vcLayout = (bmtFieldLayoutTag*) qb.AllocThrows(fieldSize * sizeof(bmtFieldLayoutTag));
     memset((void*)vcLayout, nonoref, fieldSize);
 
-    // use pointer series to locate the orefs
-    CGCDesc* map = CGCDesc::GetCGCDescFromMT(pMT);
-    CGCDescSeries *pSeries = map->GetLowestSeries();
-
-    for (SIZE_T j = 0; j < map->GetNumSeries(); j++)
+    // If the type contains pointers fill it out from the GC data
+    if (pMT->ContainsPointers())
     {
-        CONSISTENCY_CHECK(pSeries <= map->GetHighestSeries());
+        // use pointer series to locate the orefs
+        CGCDesc* map = CGCDesc::GetCGCDescFromMT(pMT);
+        CGCDescSeries *pSeries = map->GetLowestSeries();
 
-        memset((void*)&vcLayout[pSeries->GetSeriesOffset() - OBJECT_SIZE], oref, pSeries->GetSeriesSize() + pMT->GetBaseSize());
-        pSeries++;
+        for (SIZE_T j = 0; j < map->GetNumSeries(); j++)
+        {
+            CONSISTENCY_CHECK(pSeries <= map->GetHighestSeries());
+
+            memset((void*)&vcLayout[pSeries->GetSeriesOffset() - OBJECT_SIZE], oref, pSeries->GetSeriesSize() + pMT->GetBaseSize());
+            pSeries++;
+        }
     }
 
     ExplicitClassTrust explicitClassTrust;
diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs
new file mode 100644 (file)
index 0000000..99ca59f
--- /dev/null
@@ -0,0 +1,92 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+using Xunit;
+
+public class Test
+{
+    public struct WithORefs
+    {
+        public object F;
+    }
+
+    public struct WithNoORefs
+    {
+        public int F;
+    }
+
+    public ref struct WithByRefs
+    {
+        public ref int F;
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    public ref struct Explicit1
+    {
+        [FieldOffset(0)] public Inner1 Field1;
+        public ref struct Inner1
+        {
+            public WithORefs Field2;
+        }
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    public ref struct Explicit2
+    {
+        [FieldOffset(0)] public Inner2 Field1;
+        public ref struct Inner2
+        {
+            public WithNoORefs Field2;
+        }
+    }
+
+    [StructLayout(LayoutKind.Explicit)]
+    public ref struct Explicit3
+    {
+        [FieldOffset(0)] public Inner3 Field1;
+        public ref struct Inner3
+        {
+            public WithByRefs Field2;
+        }
+    }
+
+    [Fact]
+    public static void Validate_Explicit1()
+    {
+        Load1();
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static string Load1()
+        {
+            return typeof(Explicit1).ToString();
+        }
+    }
+
+    [Fact]
+    public static void Validate_Explicit2()
+    {
+        Load2();
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static string Load2()
+        {
+            return typeof(Explicit2).ToString();
+        }
+    }
+
+    [Fact]
+    public static void Validate_Explicit3()
+    {
+        Load3();
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static string Load3()
+        {
+            return typeof(Explicit3).ToString();
+        }
+    }
+}
diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj
new file mode 100644 (file)
index 0000000..0de2ecf
--- /dev/null
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <OutputType>Exe</OutputType>
+    <MonoAotIncompatible>false</MonoAotIncompatible>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="case13.cs" />
+  </ItemGroup>
+</Project>
index 75485f5..c790ff9 100644 (file)
         <ExcludeList Include = "$(XunitTestBinBase)/JIT/Performance/CodeQuality/Serialization/Serialize/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
-        <ExcludeList Include = "$(XunitTestBinBase)/JIT/Performance/CodeQuality/Serialization/Deserialize/**">
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Performance/CodeQuality/Serialization/Deserialize/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
-        <ExcludeList Include = "$(XunitTestBinBase)/JIT/Intrinsics/TypeIntrinsics_r*/**">
+        <ExcludeList Include="$(XunitTestBinBase)/JIT/Intrinsics/TypeIntrinsics_r*/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/varargs/varargsupport/*">
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/varargs/varargsupport_r/*">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
-        <ExcludeList Include = "$(XunitTestBinBase)/baseservices/TieredCompilation/TieredVtableMethodTests/**">
+        <ExcludeList Include="$(XunitTestBinBase)/baseservices/TieredCompilation/TieredVtableMethodTests/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
-        <ExcludeList Include = "$(XunitTestBinBase)/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case2/**">
+        <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case2/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
         <ExcludeList Include = "$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_80731/Runtime_80731/**">
     </ItemGroup>
 
     <ItemGroup Condition="'$(RuntimeFlavor)' == 'mono' and ('$(RuntimeVariant)' == 'llvmfullaot' or '$(RuntimeVariant)' == 'llvmaot') and '$(TargetArchitecture)' == 'arm64'">
-        <ExcludeList Include = "$(XunitTestBinBase)/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case2/**">
+        <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case2/**">
             <Issue>https://github.com/dotnet/runtime/issues/57350</Issue>
         </ExcludeList>
     </ItemGroup>