From a04b96557d5387d8b01478f80c6bea09ac1cb10d Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Wed, 18 Mar 2020 13:03:29 -0700 Subject: [PATCH 1/3] Canonicalize arguments before loading instantiation when dropGenericArgumentLevel is TRUE --- src/coreclr/src/vm/siginfo.cpp | 7 +- .../Github/runtime_32848/runtime_32848.cs | 75 +++++++++++++++++++ .../Github/runtime_32848/runtime_32848.csproj | 23 ++++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.cs create mode 100644 src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj diff --git a/src/coreclr/src/vm/siginfo.cpp b/src/coreclr/src/vm/siginfo.cpp index 58b8d86f5b5c86..dc21a23eac1eb5 100644 --- a/src/coreclr/src/vm/siginfo.cpp +++ b/src/coreclr/src/vm/siginfo.cpp @@ -1356,7 +1356,7 @@ TypeHandle SigPointer::GetTypeHandleThrowing( if (tmpEType == ELEMENT_TYPE_CLASS) typeHnd = TypeHandle(g_pCanonMethodTableClass); } - else if ((elemType == (CorElementType) ELEMENT_TYPE_CANON_ZAPSIG) || + else if ((elemType == (CorElementType)ELEMENT_TYPE_CANON_ZAPSIG) || (CorTypeInfo::GetGCType_NoThrow(elemType) == TYPE_GC_REF)) { typeHnd = TypeHandle(g_pCanonMethodTableClass); @@ -1389,6 +1389,11 @@ TypeHandle SigPointer::GetTypeHandleThrowing( thisinst = NULL; break; } + + if (dropGenericArgumentLevel && level == CLASS_LOAD_APPROXPARENTS) + { + typeHnd = ClassLoader::CanonicalizeGenericArg(typeHnd); + } } thisinst[i] = typeHnd; IfFailThrowBF(psig.SkipExactlyOne(), BFA_BAD_SIGNATURE, pOrigModule); diff --git a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.cs b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.cs new file mode 100644 index 00000000000000..34d0dec01bf0c4 --- /dev/null +++ b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.cs @@ -0,0 +1,75 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +public struct MyStruct +{ + int _id; + public MyStruct(int id) { _id = id; } + public override string ToString() => this.GetType().ToString() + " = " + _id; +} + +public struct GenStruct { } + +public sealed class MyStructWrapper +{ + public MyStruct _field; + + public MyStructWrapper(MyStruct value) + { + _field = value; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public override string ToString() => _field.ToString(); +} + +public abstract class BaseStructCreator +{ + public abstract MyStructWrapper GetMyStructWrapper() where TRequest : class; +} + +public class StructCreator : BaseStructCreator +{ + public override MyStructWrapper GetMyStructWrapper() + { + return new MyStructWrapper(CreateCall()); + } + protected virtual MyStruct CreateCall() where TRequest : class + { + return new MyStruct(123); + } +} + +class DerivedCreator : StructCreator +{ + protected override MyStruct CreateCall() + { + return new MyStruct(456); + } +} + +public class Test +{ + [MethodImpl(MethodImplOptions.NoInlining)] + static string RunTest() + { + var creator = new DerivedCreator(); + var wrapper = creator.GetMyStructWrapper>(); + return wrapper.ToString(); + } + public static int Main() + { + Console.WriteLine("Expected: MyStruct`2[System.Exception,GenStruct`1[System.String]] = 456"); + + string result = RunTest(); + Console.WriteLine("Actual : " + result); + + string expected = "MyStruct`2[System.Exception,GenStruct`1[System.String]] = 456"; + return result == expected ? 100 : -1; + } +} diff --git a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj new file mode 100644 index 00000000000000..59773639818cf2 --- /dev/null +++ b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj @@ -0,0 +1,23 @@ + + + Exe + BuildAndRun + + + + + + + + + + + From a26ac4715d34a043f029487890b292b3877941a7 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Tue, 7 Apr 2020 12:19:26 -0700 Subject: [PATCH 2/3] Fix casing in COMPlus_GCStress env variable and remove COMPlus_gcServer Also changing a bunch of assert() calls to _ASSERTE. Usually when _ASSERTE fails in CI lab runs, we tend to get crash dumps associated with test results, unlike assert() which shows a GUI dialog that DHandler dismisses by clicking on the Abort button. --- src/coreclr/src/vm/gcenv.os.cpp | 30 +++++++++---------- .../Github/runtime_32848/runtime_32848.csproj | 6 ++-- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/coreclr/src/vm/gcenv.os.cpp b/src/coreclr/src/vm/gcenv.os.cpp index c0ed7d79733539..6b9e042f04e9b2 100644 --- a/src/coreclr/src/vm/gcenv.os.cpp +++ b/src/coreclr/src/vm/gcenv.os.cpp @@ -47,8 +47,8 @@ class GroupProcNo GroupProcNo(uint16_t group, uint16_t procIndex) : m_groupProc((group << 6) | procIndex) { // Making this the same as the # of NUMA node we support. - assert(group < 0x40); - assert(procIndex <= 0x3f); + _ASSERTE(group < 0x40); + _ASSERTE(procIndex <= 0x3f); } uint16_t GetGroup() { return m_groupProc >> 6; } @@ -120,7 +120,7 @@ bool GCToOSInterface::Initialize() uint32_t currentProcessCpuCount = PAL_GetLogicalCpuCountFromOS(); if (PAL_GetCurrentThreadAffinitySet(AffinitySet::BitsetDataSize, g_processAffinitySet.GetBitsetData())) { - assert(currentProcessCpuCount == g_processAffinitySet.Count()); + _ASSERTE(currentProcessCpuCount == g_processAffinitySet.Count()); } else { @@ -1322,7 +1322,7 @@ class GCEvent::Impl { WRAPPER_NO_CONTRACT; - assert(m_event.IsValid()); + _ASSERTE(m_event.IsValid()); m_event.CloseEvent(); } @@ -1330,7 +1330,7 @@ class GCEvent::Impl { WRAPPER_NO_CONTRACT; - assert(m_event.IsValid()); + _ASSERTE(m_event.IsValid()); m_event.Set(); } @@ -1338,7 +1338,7 @@ class GCEvent::Impl { WRAPPER_NO_CONTRACT; - assert(m_event.IsValid()); + _ASSERTE(m_event.IsValid()); m_event.Reset(); } @@ -1346,7 +1346,7 @@ class GCEvent::Impl { WRAPPER_NO_CONTRACT; - assert(m_event.IsValid()); + _ASSERTE(m_event.IsValid()); return m_event.Wait(timeout, alertable); } @@ -1400,7 +1400,7 @@ void GCEvent::CloseEvent() { WRAPPER_NO_CONTRACT; - assert(m_impl != nullptr); + _ASSERTE(m_impl != nullptr); m_impl->CloseEvent(); } @@ -1408,7 +1408,7 @@ void GCEvent::Set() { WRAPPER_NO_CONTRACT; - assert(m_impl != nullptr); + _ASSERTE(m_impl != nullptr); m_impl->Set(); } @@ -1416,7 +1416,7 @@ void GCEvent::Reset() { WRAPPER_NO_CONTRACT; - assert(m_impl != nullptr); + _ASSERTE(m_impl != nullptr); m_impl->Reset(); } @@ -1424,7 +1424,7 @@ uint32_t GCEvent::Wait(uint32_t timeout, bool alertable) { WRAPPER_NO_CONTRACT; - assert(m_impl != nullptr); + _ASSERTE(m_impl != nullptr); return m_impl->Wait(timeout, alertable); } @@ -1435,7 +1435,7 @@ bool GCEvent::CreateManualEventNoThrow(bool initialState) GC_NOTRIGGER; } CONTRACTL_END; - assert(m_impl == nullptr); + _ASSERTE(m_impl == nullptr); NewHolder event = new (nothrow) GCEvent::Impl(); if (!event) { @@ -1454,7 +1454,7 @@ bool GCEvent::CreateAutoEventNoThrow(bool initialState) GC_NOTRIGGER; } CONTRACTL_END; - assert(m_impl == nullptr); + _ASSERTE(m_impl == nullptr); NewHolder event = new (nothrow) GCEvent::Impl(); if (!event) { @@ -1473,7 +1473,7 @@ bool GCEvent::CreateOSAutoEventNoThrow(bool initialState) GC_NOTRIGGER; } CONTRACTL_END; - assert(m_impl == nullptr); + _ASSERTE(m_impl == nullptr); NewHolder event = new (nothrow) GCEvent::Impl(); if (!event) { @@ -1492,7 +1492,7 @@ bool GCEvent::CreateOSManualEventNoThrow(bool initialState) GC_NOTRIGGER; } CONTRACTL_END; - assert(m_impl == nullptr); + _ASSERTE(m_impl == nullptr); NewHolder event = new (nothrow) GCEvent::Impl(); if (!event) { diff --git a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj index 59773639818cf2..c78964310e4a5f 100644 --- a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj +++ b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj @@ -10,13 +10,11 @@ From 5853df3b145e673a8c1d987a46bf230416a687b2 Mon Sep 17 00:00:00 2001 From: Fadi Hanna Date: Fri, 10 Apr 2020 12:39:49 -0700 Subject: [PATCH 3/3] Remove gcstress switch. The test will run with gcstress as part of the gcstress lab runs --- .../Github/runtime_32848/runtime_32848.csproj | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj index c78964310e4a5f..9732b0b497875e 100644 --- a/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj +++ b/src/coreclr/tests/src/GC/Regressions/Github/runtime_32848/runtime_32848.csproj @@ -6,16 +6,4 @@ - - - - - -