From b69e03e592e913c65c3f7264a143c0bc85d68f1e Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 13 Jan 2023 16:43:00 -0800 Subject: [PATCH 1/6] Fix nested non-byref like VC with no pointer fields --- src/coreclr/vm/methodtablebuilder.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index fae0605d0b0bd5..f7846628f5a3e4 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -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; From 21bf6f53e2281c74be212e8bc6ebcac1acdd6c7e Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 13 Jan 2023 17:27:43 -0800 Subject: [PATCH 2/6] Add tests --- .../objrefandnonobjrefoverlap/case13.cs | 95 +++++++++++++++++++ .../objrefandnonobjrefoverlap/case13.csproj | 9 ++ 2 files changed, 104 insertions(+) create mode 100644 src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs create mode 100644 src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs new file mode 100644 index 00000000000000..d21e240adf290c --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs @@ -0,0 +1,95 @@ +// 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 WithByORefs + { + 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 WithByORefs Field2; + } + } + + [Fact] + public static void Validate_Explicit1() + { + Load1(); + // that's enough. if we didn't throw a TypeLoadException, the test case will fail. + + [MethodImpl(MethodImplOptions.NoInlining)] + static string Load1() + { + return typeof(Explicit1).ToString(); + } + } + + [Fact] + public static void Validate_Explicit2() + { + Load2(); + // that's enough. if we didn't throw a TypeLoadException, the test case will fail. + + [MethodImpl(MethodImplOptions.NoInlining)] + static string Load2() + { + return typeof(Explicit2).ToString(); + } + } + + [Fact] + public static void Validate_Explicit3() + { + Load3(); + // that's enough. if we didn't throw a TypeLoadException, the test case will fail. + + [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 index 00000000000000..6f45afef5e5fa8 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj @@ -0,0 +1,9 @@ + + + true + Exe + + + + + From 068797743fc6ca4f9d0d1489cbc9a61815f9625e Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 13 Jan 2023 17:29:23 -0800 Subject: [PATCH 3/6] Fix test typo --- .../explicitlayout/objrefandnonobjrefoverlap/case13.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs index d21e240adf290c..919e3380fb3b9c 100644 --- a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs +++ b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs @@ -19,7 +19,7 @@ public struct WithNoORefs public int F; } - public ref struct WithByORefs + public ref struct WithByRefs { public ref int F; } @@ -50,7 +50,7 @@ public ref struct Explicit3 [FieldOffset(0)] public Inner3 Field1; public ref struct Inner3 { - public WithByORefs Field2; + public WithByRefs Field2; } } From 057748406d2d47b27896cb90c47c7caf63c60b0e Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Sun, 15 Jan 2023 12:10:42 -0800 Subject: [PATCH 4/6] Disable new test on llvmaot --- src/tests/issues.targets | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index dadea0c4b59e30..33498c4803c557 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2976,10 +2976,10 @@ https://github.com/dotnet/runtime/issues/57350 - + https://github.com/dotnet/runtime/issues/57350 - + https://github.com/dotnet/runtime/issues/57350 @@ -2988,10 +2988,13 @@ https://github.com/dotnet/runtime/issues/57350 - + + https://github.com/dotnet/runtime/issues/57350 + + https://github.com/dotnet/runtime/issues/57350 - + https://github.com/dotnet/runtime/issues/57350 @@ -3179,7 +3182,7 @@ - + https://github.com/dotnet/runtime/issues/57350 From 09365ed118e64ebd0ca1198080e4d16af06e8f0d Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Sun, 15 Jan 2023 12:48:48 -0800 Subject: [PATCH 5/6] Apply suggestions from code review --- .../explicitlayout/objrefandnonobjrefoverlap/case13.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs index 919e3380fb3b9c..99ca59fdd47e7a 100644 --- a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs +++ b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.cs @@ -58,7 +58,6 @@ public ref struct Inner3 public static void Validate_Explicit1() { Load1(); - // that's enough. if we didn't throw a TypeLoadException, the test case will fail. [MethodImpl(MethodImplOptions.NoInlining)] static string Load1() @@ -71,7 +70,6 @@ static string Load1() public static void Validate_Explicit2() { Load2(); - // that's enough. if we didn't throw a TypeLoadException, the test case will fail. [MethodImpl(MethodImplOptions.NoInlining)] static string Load2() @@ -84,7 +82,6 @@ static string Load2() public static void Validate_Explicit3() { Load3(); - // that's enough. if we didn't throw a TypeLoadException, the test case will fail. [MethodImpl(MethodImplOptions.NoInlining)] static string Load3() From 142862a5fe51119dc28532b21233defb883da94b Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 17 Jan 2023 15:42:14 -0800 Subject: [PATCH 6/6] Fully enable new test for MonoAOT --- .../explicitlayout/objrefandnonobjrefoverlap/case13.csproj | 1 + src/tests/issues.targets | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj index 6f45afef5e5fa8..0de2ecf447373d 100644 --- a/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj +++ b/src/tests/Loader/classloader/explicitlayout/objrefandnonobjrefoverlap/case13.csproj @@ -2,6 +2,7 @@ true Exe + false diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 33498c4803c557..cd2f7e35636b99 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -2994,9 +2994,6 @@ https://github.com/dotnet/runtime/issues/57350 - - https://github.com/dotnet/runtime/issues/57350 - llvmfullaot: EH problem