From 4cd3e780c469e2416b6ebaba903dbea7c411a5b8 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 29 Jul 2022 23:29:50 +0200 Subject: [PATCH] Add EH filters for generic catch(T) blocks (#72721) Co-authored-by: Jan Kotas --- docs/design/coreclr/botr/readytorun-format.md | 1 + src/coreclr/inc/corinfo.h | 2 + src/coreclr/inc/jiteeversionguid.h | 10 +- src/coreclr/inc/jithelpers.h | 2 + src/coreclr/inc/readytorun.h | 3 +- src/coreclr/inc/readytorunhelpers.h | 1 + src/coreclr/jit/compiler.h | 2 + src/coreclr/jit/flowgraph.cpp | 4 + src/coreclr/jit/jiteh.cpp | 87 ++++++++++++ .../src/System/Runtime/ExceptionHandling.cs | 2 +- .../Runtime.Base/src/System/Runtime/TypeCast.cs | 37 +++++ src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h | 2 +- .../src/System/Runtime/RuntimeImports.cs | 4 + .../tools/Common/Internal/Runtime/ModuleHeaders.cs | 2 +- .../Common/Internal/Runtime/ReadyToRunConstants.cs | 1 + .../tools/Common/JitInterface/CorInfoHelpFunc.cs | 2 + .../tools/Common/JitInterface/CorInfoImpl.cs | 18 --- .../aot/ILCompiler.Compiler/Compiler/JitHelper.cs | 3 + .../ILCompiler.Compiler/IL/ILImporter.Scanner.cs | 10 +- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 3 + .../ReadyToRunSignature.cs | 4 + .../JitInterface/CorInfoImpl.RyuJit.cs | 10 +- src/coreclr/vm/codeman.cpp | 61 --------- src/coreclr/vm/jithelpers.cpp | 11 +- src/coreclr/vm/jitinterface.cpp | 42 ------ src/coreclr/vm/siginfo.cpp | 115 ---------------- src/coreclr/vm/siginfo.hpp | 28 ---- .../Exceptions/GenericCatchInterfaceProgram.il | 151 +++++++++++++++++++++ .../Exceptions/GenericCatchInterfaceProgram.ilproj | 9 ++ src/tests/issues.targets | 21 +-- .../nativeaot/SmokeTests/Exceptions/Exceptions.cs | 58 ++++++-- 31 files changed, 389 insertions(+), 317 deletions(-) create mode 100644 src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.il create mode 100644 src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.ilproj diff --git a/docs/design/coreclr/botr/readytorun-format.md b/docs/design/coreclr/botr/readytorun-format.md index 24dd445..fa6bb18 100644 --- a/docs/design/coreclr/botr/readytorun-format.md +++ b/docs/design/coreclr/botr/readytorun-format.md @@ -797,6 +797,7 @@ enum ReadyToRunHelper READYTORUN_HELPER_GenericGcTlsBase = 0x66, READYTORUN_HELPER_GenericNonGcTlsBase = 0x67, READYTORUN_HELPER_VirtualFuncPtr = 0x68, + READYTORUN_HELPER_IsInstanceOfException = 0x69, // Long mul/div/shift ops READYTORUN_HELPER_LMul = 0xC0, diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index d3ccc3b..80ef9c1 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -438,6 +438,8 @@ enum CorInfoHelpFunc CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check + CORINFO_HELP_ISINSTANCEOF_EXCEPTION, + CORINFO_HELP_BOX, // Fast box helper. Only possible exception is OutOfMemory CORINFO_HELP_BOX_NULLABLE, // special form of boxing for Nullable CORINFO_HELP_UNBOX, diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 2a9fc7d..dedd71f 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 0d853657-7a01-421f-b1b0-d22a8e691441 */ - 0x0d853657, - 0x7a01, - 0x421f, - {0xb1, 0xb0, 0xd2, 0x2a, 0x8e, 0x69, 0x14, 0x41} +constexpr GUID JITEEVersionIdentifier = { /* 4efa8fe2-8489-4b61-aac9-b4df74af15b7 */ + 0x4efa8fe2, + 0x8489, + 0x4b61, + {0xaa, 0xc9, 0xb4, 0xdf, 0x74, 0xaf, 0x15, 0xb7} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index a500c29..e897d59 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -99,6 +99,8 @@ DYNAMICJITHELPER(CORINFO_HELP_CHKCASTANY, NULL, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_CHKCASTCLASS_SPECIAL, NULL, CORINFO_HELP_SIG_REG_ONLY) + JITHELPER(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, JIT_IsInstanceOfException, CORINFO_HELP_SIG_REG_ONLY) + DYNAMICJITHELPER(CORINFO_HELP_BOX, JIT_Box, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_BOX_NULLABLE, JIT_Box, CORINFO_HELP_SIG_REG_ONLY) DYNAMICJITHELPER(CORINFO_HELP_UNBOX, NULL, CORINFO_HELP_SIG_REG_ONLY) diff --git a/src/coreclr/inc/readytorun.h b/src/coreclr/inc/readytorun.h index e453dde..0934f2e 100644 --- a/src/coreclr/inc/readytorun.h +++ b/src/coreclr/inc/readytorun.h @@ -16,7 +16,7 @@ // Keep these in sync with src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs #define READYTORUN_MAJOR_VERSION 0x0007 -#define READYTORUN_MINOR_VERSION 0x0000 +#define READYTORUN_MINOR_VERSION 0x0001 #define MINIMUM_READYTORUN_MAJOR_VERSION 0x006 @@ -329,6 +329,7 @@ enum ReadyToRunHelper READYTORUN_HELPER_GenericGcTlsBase = 0x66, READYTORUN_HELPER_GenericNonGcTlsBase = 0x67, READYTORUN_HELPER_VirtualFuncPtr = 0x68, + READYTORUN_HELPER_IsInstanceOfException = 0x69, // Long mul/div/shift ops READYTORUN_HELPER_LMul = 0xC0, diff --git a/src/coreclr/inc/readytorunhelpers.h b/src/coreclr/inc/readytorunhelpers.h index 66e2d4a..b9d904b 100644 --- a/src/coreclr/inc/readytorunhelpers.h +++ b/src/coreclr/inc/readytorunhelpers.h @@ -54,6 +54,7 @@ HELPER(READYTORUN_HELPER_GenericGcTlsBase, CORINFO_HELP_GETGENERICS_GCT HELPER(READYTORUN_HELPER_GenericNonGcTlsBase, CORINFO_HELP_GETGENERICS_NONGCTHREADSTATIC_BASE,) HELPER(READYTORUN_HELPER_VirtualFuncPtr, CORINFO_HELP_VIRTUAL_FUNC_PTR, ) +HELPER(READYTORUN_HELPER_IsInstanceOfException, CORINFO_HELP_ISINSTANCEOF_EXCEPTION, ) HELPER(READYTORUN_HELPER_LMul, CORINFO_HELP_LMUL, ) HELPER(READYTORUN_HELPER_LMulOfv, CORINFO_HELP_LMUL_OVF, ) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f80271a..a69148d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2216,6 +2216,8 @@ public: bool fgNormalizeEHCase2(); bool fgNormalizeEHCase3(); + void fgCreateFiltersForGenericExceptions(); + void fgCheckForLoopsInHandlers(); #ifdef DEBUG diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index b724c0a..288c0e7 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -2590,6 +2590,10 @@ void Compiler::fgAddInternal() { noway_assert(!compIsForInlining()); + // For runtime determined Exception types we're going to emit a fake EH filter with isinst for this + // type with a runtime lookup + fgCreateFiltersForGenericExceptions(); + // The backend requires a scratch BB into which it can safely insert a P/Invoke method prolog if one is // required. Similarly, we need a scratch BB for poisoning. Create it here. if (compMethodRequiresPInvokeFrame() || compShouldPoisonFrame()) diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 66a2d65..2787234 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -2506,6 +2506,93 @@ bool Compiler::fgNormalizeEHCase2() return modified; } +//------------------------------------------------------------------------ +// fgCreateFiltersForGenericExceptions: +// For Exception types which require runtime lookup it creates a "fake" single-block +// EH filter that performs "catchArg isinst T!!" and in case of success forwards to the +// original EH handler. +// + +void Compiler::fgCreateFiltersForGenericExceptions() +{ + for (unsigned ehNum = 0; ehNum < compHndBBtabCount; ehNum++) + { + EHblkDsc* eh = ehGetDsc(ehNum); + if (eh->ebdHandlerType == EH_HANDLER_CATCH) + { + // Resolve Exception type and check if it needs a runtime lookup + CORINFO_RESOLVED_TOKEN resolvedToken; + resolvedToken.tokenContext = impTokenLookupContextHandle; + resolvedToken.tokenScope = info.compScopeHnd; + resolvedToken.token = eh->ebdTyp; + resolvedToken.tokenType = CORINFO_TOKENKIND_Casting; + info.compCompHnd->resolveToken(&resolvedToken); + + CORINFO_GENERICHANDLE_RESULT embedInfo; + info.compCompHnd->embedGenericHandle(&resolvedToken, true, &embedInfo); + if (!embedInfo.lookup.lookupKind.needsRuntimeLookup) + { + // Exception type does not need runtime lookup + continue; + } + + // Create a new bb for the fake filter + BasicBlock* filterBb = bbNewBasicBlock(BBJ_EHFILTERRET); + BasicBlock* handlerBb = eh->ebdHndBeg; + + // Now we need to spill CATCH_ARG (it should be the first thing evaluated) + GenTree* arg = new (this, GT_CATCH_ARG) GenTree(GT_CATCH_ARG, TYP_REF); + arg->gtFlags |= GTF_ORDER_SIDEEFF; + unsigned tempNum = lvaGrabTemp(false DEBUGARG("SpillCatchArg")); + lvaTable[tempNum].lvType = TYP_REF; + GenTree* argAsg = gtNewTempAssign(tempNum, arg); + arg = gtNewLclvNode(tempNum, TYP_REF); + fgInsertStmtAtBeg(filterBb, gtNewStmt(argAsg, handlerBb->firstStmt()->GetDebugInfo())); + + // Create "catchArg is TException" tree + GenTree* runtimeLookup; + if (embedInfo.lookup.runtimeLookup.indirections == CORINFO_USEHELPER) + { + GenTree* ctxTree = getRuntimeContextTree(embedInfo.lookup.lookupKind.runtimeLookupKind); + runtimeLookup = impReadyToRunHelperToTree(&resolvedToken, CORINFO_HELP_READYTORUN_GENERIC_HANDLE, + TYP_I_IMPL, &embedInfo.lookup.lookupKind, ctxTree); + } + else + { + runtimeLookup = getTokenHandleTree(&resolvedToken, true); + } + GenTree* isInstOfT = gtNewHelperCallNode(CORINFO_HELP_ISINSTANCEOF_EXCEPTION, TYP_INT, runtimeLookup, arg); + GenTree* retFilt = gtNewOperNode(GT_RETFILT, TYP_INT, isInstOfT); + + // Insert it right before the handler (and make it a pred of the handler) + fgInsertBBbefore(handlerBb, filterBb); + fgAddRefPred(handlerBb, filterBb); + fgNewStmtAtEnd(filterBb, retFilt, handlerBb->firstStmt()->GetDebugInfo()); + + filterBb->bbCatchTyp = BBCT_FILTER; + filterBb->bbCodeOffs = handlerBb->bbCodeOffs; + filterBb->bbHndIndex = handlerBb->bbHndIndex; + filterBb->bbTryIndex = handlerBb->bbTryIndex; + filterBb->bbJumpDest = handlerBb; + filterBb->bbSetRunRarely(); + filterBb->bbFlags |= BBF_INTERNAL | BBF_DONT_REMOVE; + + handlerBb->bbCatchTyp = BBCT_FILTER_HANDLER; + eh->ebdHandlerType = EH_HANDLER_FILTER; + eh->ebdFilter = filterBb; + +#ifdef DEBUG + if (verbose) + { + JITDUMP("EH%d: Adding EH filter block " FMT_BB " in front of generic handler " FMT_BB ":\n", ehNum, + filterBb->bbNum, handlerBb->bbNum); + fgDumpBlock(filterBb); + } +#endif // DEBUG + } + } +} + bool Compiler::fgNormalizeEHCase3() { bool modified = false; diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs index 4483386..9e57685 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/ExceptionHandling.cs @@ -847,7 +847,7 @@ namespace System.Runtime AssertNotRuntimeObject(pClauseType); #endif - return TypeCast.IsInstanceOfClass(pClauseType, exception) != null; + return TypeCast.IsInstanceOfException(pClauseType, exception); } private static void InvokeSecondPass(ref ExInfo exInfo, uint idxStart) diff --git a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs index 1c27761..83c4f14 100644 --- a/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs +++ b/src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/TypeCast.cs @@ -908,6 +908,43 @@ assigningNull: return IsInstanceOfClass(pTargetType, obj); } + [RuntimeExport("RhTypeCast_IsInstanceOfException")] + public static unsafe bool IsInstanceOfException(MethodTable* pTargetType, object? obj) + { + // Based on IsInstanceOfClass_Helper + + if (obj == null) + return false; + + MethodTable* pObjType = obj.GetMethodTable(); + + if (pTargetType->IsCloned) + pTargetType = pTargetType->CanonicalEEType; + + if (pObjType->IsCloned) + pObjType = pObjType->CanonicalEEType; + + if (pObjType == pTargetType) + return true; + + // arrays can be cast to System.Object and System.Array + if (pObjType->IsArray) + return WellKnownEETypes.IsSystemObject(pTargetType) || WellKnownEETypes.IsSystemArray(pTargetType); + + while (true) + { + pObjType = pObjType->NonClonedNonArrayBaseType; + if (pObjType == null) + return false; + + if (pObjType->IsCloned) + pObjType = pObjType->CanonicalEEType; + + if (pObjType == pTargetType) + return true; + } + } + [RuntimeExport("RhTypeCast_CheckCast")] public static unsafe object CheckCast(MethodTable* pTargetType, object obj) { diff --git a/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h b/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h index 8600b4c..57aac4f 100644 --- a/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h +++ b/src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h @@ -11,7 +11,7 @@ struct ReadyToRunHeaderConstants static const uint32_t Signature = 0x00525452; // 'RTR' static const uint32_t CurrentMajorVersion = 7; - static const uint32_t CurrentMinorVersion = 0; + static const uint32_t CurrentMinorVersion = 1; }; struct ReadyToRunHeader diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs index 5489553..35077dc 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs @@ -327,6 +327,10 @@ namespace System.Runtime [RuntimeImport(RuntimeLibrary, "RhTypeCast_IsInstanceOfInterface")] internal static extern unsafe object IsInstanceOfInterface(MethodTable* pTargetType, object obj); + [MethodImpl(MethodImplOptions.InternalCall)] + [RuntimeImport(RuntimeLibrary, "RhTypeCast_IsInstanceOfException")] + internal static extern unsafe bool IsInstanceOfException(MethodTable* pTargetType, object obj); + internal static unsafe object IsInstanceOfInterface(EETypePtr pTargetType, object obj) => IsInstanceOfInterface(pTargetType.ToPointer(), obj); diff --git a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs index ea705d1..c73467f 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs @@ -15,7 +15,7 @@ namespace Internal.Runtime public const uint Signature = 0x00525452; // 'RTR' public const ushort CurrentMajorVersion = 7; - public const ushort CurrentMinorVersion = 0; + public const ushort CurrentMinorVersion = 1; } #pragma warning disable 0169 diff --git a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs index 0bea2c3..193b0f3 100644 --- a/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs +++ b/src/coreclr/tools/Common/Internal/Runtime/ReadyToRunConstants.cs @@ -255,6 +255,7 @@ namespace Internal.ReadyToRunConstants GenericGcTlsBase = 0x66, GenericNonGcTlsBase = 0x67, VirtualFuncPtr = 0x68, + IsInstanceOfException = 0x69, // Long mul/div/shift ops LMul = 0xC0, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index fd17876..b0a53c8 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -84,6 +84,8 @@ namespace Internal.JitInterface CORINFO_HELP_CHKCASTCLASS_SPECIAL, // Optimized helper for classes. Assumes that the trivial cases // has been taken care of by the inlined check + CORINFO_HELP_ISINSTANCEOF_EXCEPTION, + CORINFO_HELP_BOX, // Fast box helper. Only possible exception is OutOfMemory CORINFO_HELP_BOX_NULLABLE, // special form of boxing for Nullable CORINFO_HELP_UNBOX, diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 37fe38a..752b80e 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -737,24 +737,6 @@ namespace Internal.JitInterface Get_CORINFO_SIG_INFO(method, sig: &methodInfo->args, methodIL); Get_CORINFO_SIG_INFO(methodIL.GetLocals(), &methodInfo->locals); -#if READYTORUN - if ((methodInfo->options & CorInfoOptions.CORINFO_GENERICS_CTXT_MASK) != 0) - { - foreach (var region in exceptionRegions) - { - if (region.Kind == ILExceptionRegionKind.Catch) - { - TypeDesc catchType = (TypeDesc)methodIL.GetObject(region.ClassToken); - if (catchType.IsCanonicalSubtype(CanonicalFormKind.Any)) - { - methodInfo->options |= CorInfoOptions.CORINFO_GENERICS_CTXT_KEEP_ALIVE; - break; - } - } - } - } -#endif - return true; } diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs index a4ae4c4..3cbd44d 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/JitHelper.cs @@ -248,6 +248,9 @@ namespace ILCompiler case ReadyToRunHelper.CheckInstanceAny: mangledName = "RhTypeCast_IsInstanceOf"; break; + case ReadyToRunHelper.IsInstanceOfException: + mangledName = "RhTypeCast_IsInstanceOfException"; + break; case ReadyToRunHelper.CheckCastInterface: mangledName = "RhTypeCast_CheckCastInterface"; break; diff --git a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs index 3f561a8..9f9c877 100644 --- a/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs +++ b/src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs @@ -196,15 +196,15 @@ namespace Internal.IL if (region.Kind == ILExceptionRegionKind.Filter) MarkBasicBlock(_basicBlocks[region.FilterOffset]); - // Once https://github.com/dotnet/corert/issues/3460 is done, this should be deleted. - // Throwing InvalidProgram is not great, but we want to do *something* if this happens - // because doing nothing means problems at runtime. This is not worth piping a - // a new exception with a fancy message for. if (region.Kind == ILExceptionRegionKind.Catch) { TypeDesc catchType = (TypeDesc)_methodIL.GetObject(region.ClassToken); if (catchType.IsRuntimeDeterminedSubtype) - ThrowHelper.ThrowInvalidProgramException(); + { + // For runtime determined Exception types we're going to emit a fake EH filter with isinst for this + // type with a runtime lookup + _dependencies.Add(GetGenericLookupHelper(ReadyToRunHelperId.TypeHandleForCasting, catchType), "EH filter"); + } } } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index dea0974..dce751e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -775,6 +775,9 @@ namespace Internal.JitInterface id = ReadyToRunHelper.GetRuntimeTypeHandle; break; + case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOF_EXCEPTION: + id = ReadyToRunHelper.IsInstanceOfException; + break; case CorInfoHelpFunc.CORINFO_HELP_BOX: id = ReadyToRunHelper.Box; break; diff --git a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs index ee33514..ab88c0c 100644 --- a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs +++ b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/ReadyToRunSignature.cs @@ -1782,6 +1782,10 @@ namespace ILCompiler.Reflection.ReadyToRun builder.Append("CHECK_INSTANCE_ANY"); break; + case ReadyToRunHelper.IsInstanceOfException: + builder.Append("SIMPLE_ISINSTANCE_OF"); + break; + case ReadyToRunHelper.GenericGcStaticBase: builder.Append("GENERIC_GC_STATIC_BASE"); break; diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 8dcbc88..e54366f 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -543,6 +543,9 @@ namespace Internal.JitInterface id = ReadyToRunHelper.AreTypesEquivalent; break; + case CorInfoHelpFunc.CORINFO_HELP_ISINSTANCEOF_EXCEPTION: + id = ReadyToRunHelper.IsInstanceOfException; + break; case CorInfoHelpFunc.CORINFO_HELP_BOX: id = ReadyToRunHelper.Box; break; @@ -906,13 +909,6 @@ namespace Internal.JitInterface var methodIL = (MethodIL)HandleToObject((IntPtr)_methodScope); var type = (TypeDesc)methodIL.GetObject((int)clause.ClassTokenOrOffset); - // Once https://github.com/dotnet/corert/issues/3460 is done, this should be an assert. - // Throwing InvalidProgram is not great, but we want to do *something* if this happens - // because doing nothing means problems at runtime. This is not worth piping a - // a new exception with a fancy message for. - if (type.IsCanonicalSubtype(CanonicalFormKind.Any)) - ThrowHelper.ThrowInvalidProgramException(); - var typeSymbol = _compilation.NecessaryTypeSymbolIfPossible(type); RelocType rel = (_compilation.NodeFactory.Target.IsWindows) ? diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 296943a..6c2e15f 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -3476,36 +3476,6 @@ TypeHandle EEJitManager::ResolveEHClause(EE_ILEXCEPTION_CLAUSE* pEHClause, PREFIX_ASSUME(pModule != NULL); SigTypeContext typeContext(pMD); - VarKind k = hasNoVars; - - // In the vast majority of cases the code under the "if" below - // will not be executed. - // - // First grab the representative instantiations. For code - // shared by multiple generic instantiations these are the - // canonical (representative) instantiation. - if (TypeFromToken(typeTok) == mdtTypeSpec) - { - PCCOR_SIGNATURE pSig; - ULONG cSig; - IfFailThrow(pModule->GetMDImport()->GetTypeSpecFromToken(typeTok, &pSig, &cSig)); - - SigPointer psig(pSig, cSig); - k = psig.IsPolyType(&typeContext); - - // Grab the active class and method instantiation. This exact instantiation is only - // needed in the corner case of "generic" exception catching in shared - // generic code. We don't need the exact instantiation if the token - // doesn't contain E_T_VAR or E_T_MVAR. - if ((k & hasSharableVarsMask) != 0) - { - Instantiation classInst; - Instantiation methodInst; - pCf->GetExactGenericInstantiations(&classInst, &methodInst); - SigTypeContext::InitTypeContext(pMD,classInst, methodInst,&typeContext); - } - } - return ClassLoader::LoadTypeDefOrRefOrSpecThrowing(pModule, typeTok, &typeContext, ClassLoader::ReturnNullIfNotFound); } @@ -6001,38 +5971,7 @@ TypeHandle ReadyToRunJitManager::ResolveEHClause(EE_ILEXCEPTION_CLAUSE* pEHClaus PREFIX_ASSUME(pModule != NULL); SigTypeContext typeContext(pMD); - VarKind k = hasNoVars; - mdToken typeTok = pEHClause->ClassToken; - - // In the vast majority of cases the code un der the "if" below - // will not be executed. - // - // First grab the representative instantiations. For code - // shared by multiple generic instantiations these are the - // canonical (representative) instantiation. - if (TypeFromToken(typeTok) == mdtTypeSpec) - { - PCCOR_SIGNATURE pSig; - ULONG cSig; - IfFailThrow(pModule->GetMDImport()->GetTypeSpecFromToken(typeTok, &pSig, &cSig)); - - SigPointer psig(pSig, cSig); - k = psig.IsPolyType(&typeContext); - - // Grab the active class and method instantiation. This exact instantiation is only - // needed in the corner case of "generic" exception catching in shared - // generic code. We don't need the exact instantiation if the token - // doesn't contain E_T_VAR or E_T_MVAR. - if ((k & hasSharableVarsMask) != 0) - { - Instantiation classInst; - Instantiation methodInst; - pCf->GetExactGenericInstantiations(&classInst,&methodInst); - SigTypeContext::InitTypeContext(pMD,classInst, methodInst,&typeContext); - } - } - return ClassLoader::LoadTypeDefOrRefOrSpecThrowing(pModule, typeTok, &typeContext, ClassLoader::ReturnNullIfNotFound); } diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index af79805..8fdc434 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2832,7 +2832,6 @@ HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByR FCALL_CONTRACT; TypeHandle clsHnd(type); - // @TODO right now we check for precisely the correct type. // do we want to allow inheritance? (watch out since value // classes inherit from object but do not normal object layout). @@ -2845,6 +2844,16 @@ HCIMPL2_IV(LPVOID, JIT_GetRefAny, CORINFO_CLASS_HANDLE type, TypedByRef typedByR HCIMPLEND +/*************************************************************/ +HCIMPL2(BOOL, JIT_IsInstanceOfException, CORINFO_CLASS_HANDLE type, Object* obj) +{ + FCALL_CONTRACT; + TypeHandle clsHnd(type); + return ExceptionIsOfRightType(clsHnd, obj->GetTypeHandle()); +} +HCIMPLEND + + //======================================================================== // // GENERICS HELPERS diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 51aee7d..87161fa 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -7514,49 +7514,7 @@ getMethodInfoHelper( { methInfo->options = CorInfoOptions(methInfo->options|CORINFO_GENERICS_CTXT_KEEP_ALIVE); } - else #endif // defined(PROFILING_SUPPORTED) - { - // Check all the exception clauses - - if (ftn->IsDynamicMethod()) - { - // @TODO: how do we detect the need to mark this flag? - } - else - { - COR_ILMETHOD_SECT_EH_CLAUSE_FAT ehClause; - - for (unsigned i = 0; i < methInfo->EHcount; i++) - { - const COR_ILMETHOD_SECT_EH_CLAUSE_FAT* ehInfo = - (COR_ILMETHOD_SECT_EH_CLAUSE_FAT*)header->EH->EHClause(i, &ehClause); - - // Is it a typed catch clause? - if (ehInfo->GetFlags() != COR_ILEXCEPTION_CLAUSE_NONE) - continue; - - // Check if we catch "C" ? - - DWORD catchTypeToken = ehInfo->GetClassToken(); - if (TypeFromToken(catchTypeToken) != mdtTypeSpec) - continue; - - PCCOR_SIGNATURE pSig; - ULONG cSig; - IfFailThrow(ftn->GetMDImport()->GetTypeSpecFromToken(catchTypeToken, &pSig, &cSig)); - - SigPointer psig(pSig, cSig); - - SigTypeContext sigTypeContext(ftn); - if (psig.IsPolyType(&sigTypeContext) & hasSharableVarsMask) - { - methInfo->options = CorInfoOptions(methInfo->options|CORINFO_GENERICS_CTXT_KEEP_ALIVE); - break; - } - } - } - } } PCCOR_SIGNATURE pSig = NULL; diff --git a/src/coreclr/vm/siginfo.cpp b/src/coreclr/vm/siginfo.cpp index 68b0ec71..c35e914 100644 --- a/src/coreclr/vm/siginfo.cpp +++ b/src/coreclr/vm/siginfo.cpp @@ -1869,121 +1869,6 @@ TypeHandle SigPointer::GetTypeVariable(CorElementType et, #ifndef DACCESS_COMPILE -// Does this type contain class or method type parameters whose instantiation cannot -// be determined at JIT-compile time from the instantiations in the method context? -// Return a combination of hasClassVar and hasMethodVar flags. -// See header file for more info. -VarKind SigPointer::IsPolyType(const SigTypeContext *pTypeContext) const -{ - CONTRACTL - { - INSTANCE_CHECK; - NOTHROW; - GC_NOTRIGGER; - MODE_ANY; - } - CONTRACTL_END - - SigPointer psig = *this; - CorElementType typ; - - if (FAILED(psig.GetElemType(&typ))) - return hasNoVars; - - switch(typ) { - case ELEMENT_TYPE_VAR: - case ELEMENT_TYPE_MVAR: - { - VarKind res = (typ == ELEMENT_TYPE_VAR ? hasClassVar : hasMethodVar); - if (pTypeContext != NULL) - { - TypeHandle ty = psig.GetTypeVariable(typ, pTypeContext); - if (ty.IsCanonicalSubtype()) - res = (VarKind) (res | (typ == ELEMENT_TYPE_VAR ? hasSharableClassVar : hasSharableMethodVar)); - } - return (res); - } - - case ELEMENT_TYPE_U: - case ELEMENT_TYPE_I: - case ELEMENT_TYPE_STRING: - case ELEMENT_TYPE_OBJECT: - case ELEMENT_TYPE_I1: - case ELEMENT_TYPE_U1: - case ELEMENT_TYPE_BOOLEAN: - case ELEMENT_TYPE_I2: - case ELEMENT_TYPE_U2: - case ELEMENT_TYPE_CHAR: - case ELEMENT_TYPE_I4: - case ELEMENT_TYPE_U4: - case ELEMENT_TYPE_I8: - case ELEMENT_TYPE_U8: - case ELEMENT_TYPE_R4: - case ELEMENT_TYPE_R8: - case ELEMENT_TYPE_VOID: - case ELEMENT_TYPE_CLASS: - case ELEMENT_TYPE_VALUETYPE: - case ELEMENT_TYPE_TYPEDBYREF: - return(hasNoVars); - - case ELEMENT_TYPE_GENERICINST: - { - VarKind k = psig.IsPolyType(pTypeContext); - if (FAILED(psig.SkipExactlyOne())) - return hasNoVars; - - uint32_t ntypars; - if(FAILED(psig.GetData(&ntypars))) - return hasNoVars; - - for (uint32_t i = 0; i < ntypars; i++) - { - k = (VarKind) (psig.IsPolyType(pTypeContext) | k); - if (FAILED(psig.SkipExactlyOne())) - return hasNoVars; - } - return(k); - } - - case ELEMENT_TYPE_ARRAY: - case ELEMENT_TYPE_SZARRAY: - case ELEMENT_TYPE_PINNED: - case ELEMENT_TYPE_BYREF: - case ELEMENT_TYPE_PTR: - { - return(psig.IsPolyType(pTypeContext)); - } - - case ELEMENT_TYPE_FNPTR: - { - if (FAILED(psig.GetData(NULL))) - return hasNoVars; - - // Get arg count; - uint32_t cArgs; - if (FAILED(psig.GetData(&cArgs))) - return hasNoVars; - - VarKind k = psig.IsPolyType(pTypeContext); - if (FAILED(psig.SkipExactlyOne())) - return hasNoVars; - - for (unsigned i = 0; i < cArgs; i++) - { - k = (VarKind) (psig.IsPolyType(pTypeContext) | k); - if (FAILED(psig.SkipExactlyOne())) - return hasNoVars; - } - - return(k); - } - - default: - BAD_FORMAT_NOTHROW_ASSERT(!"Bad type"); - } - return(hasNoVars); -} - BOOL SigPointer::IsStringType(Module* pModule, const SigTypeContext *pTypeContext) const { WRAPPER_NO_CONTRACT; diff --git a/src/coreclr/vm/siginfo.hpp b/src/coreclr/vm/siginfo.hpp index 29f811e..585fdb2 100644 --- a/src/coreclr/vm/siginfo.hpp +++ b/src/coreclr/vm/siginfo.hpp @@ -52,19 +52,6 @@ class ArgDestination; typedef const struct HardCodedMetaSig *LPHARDCODEDMETASIG; -//@GENERICS: flags returned from IsPolyType indicating the presence or absence of class and -// method type parameters in a type whose instantiation cannot be determined at JIT-compile time -enum VarKind -{ - hasNoVars = 0x0000, - hasClassVar = 0x0001, - hasMethodVar = 0x0002, - hasSharableClassVar = 0x0004, - hasSharableMethodVar = 0x0008, - hasAnyVarsMask = 0x0003, - hasSharableVarsMask = 0x000c -}; - //--------------------------------------------------------------------------------------- struct ScanContext; @@ -257,21 +244,6 @@ public: public: //------------------------------------------------------------------------ - // Does this type contain class or method type parameters whose instantiation cannot - // be determined at JIT-compile time from the instantiations in the method context? - // Return a combination of hasClassVar and hasMethodVar flags. - // - // Example: class C containing instance method m - // Suppose that the method context is C::m - // Then the type Dict is considered to have *no* "polymorphic" type parameters because - // !0 is known to be float and !!0 is known to be double - // But Dict has polymorphic class *and* method type parameters because both - // !1=string and !!1=object are reference types and so code using these can be shared with - // other reference instantiations. - //------------------------------------------------------------------------ - VarKind IsPolyType(const SigTypeContext *pTypeContext) const; - - //------------------------------------------------------------------------ // Tests if the element type is a System.String. Accepts // either ELEMENT_TYPE_STRING or ELEMENT_TYPE_CLASS encoding. //------------------------------------------------------------------------ diff --git a/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.il b/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.il new file mode 100644 index 0000000..33aa542 --- /dev/null +++ b/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.il @@ -0,0 +1,151 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime {} +.assembly extern System.Console {} +.assembly GenericCatchInterface {} + +// IL in this file is based on this C# program with a few IL modifications: + +// static class Program +// { +// private static int retCode = 0; +// static int Main() +// { +// try +// { +// try +// { +// throw new MyException(); +// } +// catch (MyException) // IL edit: Change to catch (IMyInterface) +// { +// Console.WriteLine("FAIL"); +// retCode++; +// } +// } +// catch +// { +// Console.WriteLine("PASS"); +// retCode += 50; +// } +// +// GenericCatch(); // IL edit: Change to GenericCatch() +// +// return retCode; +// } +// +// [MethodImpl(MethodImplOptions.NoInlining)] +// static void GenericCatch() where T : Exception // IL edit: Remove constraint +// { +// try +// { +// try +// { +// throw new MyException(); +// } +// catch (T) +// { +// Console.WriteLine("FAIL"); +// retCode++; +// } +// } +// catch +// { +// Console.WriteLine("PASS"); +// retCode += 50; +// } +// } +// } +// interface IMyInterface {} +// class MyException : Exception, IMyInterface {} + + +.class private abstract auto ansi sealed beforefieldinit GenericCatchInterfaceProgram + extends [System.Runtime]System.Object +{ + // + .field private static int32 retCode + + .method private hidebysig static int32 Main() cil managed + { + .entrypoint + .maxstack 2 + IL_0000: newobj instance void MyException::.ctor() + IL_0005: throw + + IL_0006: pop + IL_0007: ldstr "FAIL" + IL_000c: call void [System.Console]System.Console::WriteLine(string) + IL_0011: ldsfld int32 GenericCatchInterfaceProgram::retCode + IL_0016: ldc.i4.1 + IL_0017: add + IL_0018: stsfld int32 GenericCatchInterfaceProgram::retCode + IL_001d: leave.s IL_001f + + IL_001f: leave.s IL_003b + + IL_0021: pop + IL_0022: ldstr "PASS" + IL_0027: call void [System.Console]System.Console::WriteLine(string) + IL_002c: ldsfld int32 GenericCatchInterfaceProgram::retCode + IL_0031: ldc.i4.s 50 + IL_0033: add + IL_0034: stsfld int32 GenericCatchInterfaceProgram::retCode + IL_0039: leave.s IL_003b + + IL_003b: call void GenericCatchInterfaceProgram::GenericCatch() + IL_0040: ldsfld int32 GenericCatchInterfaceProgram::retCode + IL_0045: ret + IL_0046: + .try IL_0000 to IL_0006 catch IMyInterface handler IL_0006 to IL_001f + .try IL_0000 to IL_0021 catch [System.Runtime]System.Object handler IL_0021 to IL_003b + } + + .method private hidebysig static void GenericCatch<([System.Runtime]System.Object) T>() cil managed noinlining + { + .maxstack 2 + IL_0000: newobj instance void MyException::.ctor() + IL_0005: throw + + IL_0006: pop + IL_0007: ldstr "FAIL" + IL_000c: call void [System.Console]System.Console::WriteLine(string) + IL_0011: ldsfld int32 GenericCatchInterfaceProgram::retCode + IL_0016: ldc.i4.1 + IL_0017: add + IL_0018: stsfld int32 GenericCatchInterfaceProgram::retCode + IL_001d: leave.s IL_001f + + IL_001f: leave.s IL_003b + + IL_0021: pop + IL_0022: ldstr "PASS" + IL_0027: call void [System.Console]System.Console::WriteLine(string) + IL_002c: ldsfld int32 GenericCatchInterfaceProgram::retCode + IL_0031: ldc.i4.s 50 + IL_0033: add + IL_0034: stsfld int32 GenericCatchInterfaceProgram::retCode + IL_0039: leave.s IL_003b + + IL_003b: ret + IL_003c: + .try IL_0000 to IL_0006 catch !!T handler IL_0006 to IL_001f + .try IL_0000 to IL_0021 catch [System.Runtime]System.Object handler IL_0021 to IL_003b + } +} + +.class interface private abstract auto ansi IMyInterface {} + +.class private auto ansi beforefieldinit MyException + extends [System.Runtime]System.Exception + implements IMyInterface +{ + .method public hidebysig specialname rtspecialname instance void .ctor() cil managed + { + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Exception::.ctor() + IL_0006: ret + } +} \ No newline at end of file diff --git a/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.ilproj b/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.ilproj new file mode 100644 index 0000000..1f01486 --- /dev/null +++ b/src/tests/JIT/Generics/Exceptions/GenericCatchInterfaceProgram.ilproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + \ No newline at end of file diff --git a/src/tests/issues.targets b/src/tests/issues.targets index 84c550d..1cda405 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -1369,24 +1369,6 @@ - - - - - - - - - - - - - - - - - - @@ -1454,6 +1436,9 @@ + + https://github.com/dotnet/runtime/issues/72827 + https://github.com/dotnet/runtime/issues/71656 diff --git a/src/tests/nativeaot/SmokeTests/Exceptions/Exceptions.cs b/src/tests/nativeaot/SmokeTests/Exceptions/Exceptions.cs index 76440c7..3bc01ac 100644 --- a/src/tests/nativeaot/SmokeTests/Exceptions/Exceptions.cs +++ b/src/tests/nativeaot/SmokeTests/Exceptions/Exceptions.cs @@ -30,6 +30,8 @@ public class BringUpTest new BringUpTest().ToString(); } + TestGenericExceptions(); + int counter = 0; AppDomain.CurrentDomain.UnhandledException += UnhandledExceptionEventHandler; @@ -50,8 +52,8 @@ public class BringUpTest Console.WriteLine("Exception caught!"); if (e.Message != "My exception") { - Console.WriteLine("Unexpected exception message!"); - return Fail; + Console.WriteLine("Unexpected exception message!"); + return Fail; } string stackTrace = e.StackTrace; @@ -65,7 +67,7 @@ public class BringUpTest try { - g.myObjectField = new Object(); + g.myObjectField = new Object(); } catch (NullReferenceException) { @@ -75,14 +77,14 @@ public class BringUpTest try { - try - { - g.myField++; - } - finally - { - counter++; - } + try + { + g.myField++; + } + finally + { + counter++; + } } catch (NullReferenceException) { @@ -99,8 +101,8 @@ public class BringUpTest Console.WriteLine("Exception caught via filter!"); if (e.Message != "Testing filter") { - Console.WriteLine("Unexpected exception message!"); - return Fail; + Console.WriteLine("Unexpected exception message!"); + return Fail; } counter++; } @@ -209,6 +211,36 @@ public class BringUpTest } } + static void TestGenericExceptions() + { + if (CatchGenericException(100, 0) != 42) + { + Environment.Exit(Fail); + } + + try + { + CatchGenericException(100, 0); + } + catch (DivideByZeroException) + { + return; + } + Environment.Exit(Fail); + } + + static int CatchGenericException(int a, int b) where T : Exception + { + try + { + return a / b; + } + catch (T) + { + return 42; + } + } + static bool FilterWithStackTrace(Exception e) { var stackTrace = new StackTrace(0, true); -- 2.7.4