diff --git a/docs/design/coreclr/botr/clr-abi.md b/docs/design/coreclr/botr/clr-abi.md index 9b69c69e67d82b..62e75ab0f7a125 100644 --- a/docs/design/coreclr/botr/clr-abi.md +++ b/docs/design/coreclr/botr/clr-abi.md @@ -452,7 +452,7 @@ The code this finally returns to looks like this: In this case, it zeros out the ShadowSP slot that it previously set to 0xFC, then jumps to the address that is the actual target of the leave from the finally. -The JIT does this "end finally restore" by creating a GT_END_LFIN tree node, with the appropriate stack level as an operand, that generates this code. +The JIT does this "end finally restore" by creating a GT_END_LFIN tree node, with the appropriate EH region ID as an operand, that generates this code. In the case of an exceptional 'finally' invocation, the VM sets up the 'return address' to whatever address it wants the JIT to return to. @@ -476,7 +476,7 @@ The VM walks the ShadowSP slots in the function `GetHandlerFrameInfo()`, and set An aside on the JIT implementation for x86. -The JIT creates BBJ_CALLFINALLY/BBJ_ALWAYS pairs for calling the 'finally' clause. The BBJ_CALLFINALLY block will have a series of CORINFO_JIT_ENDCATCH calls appended at the end, if we need to "leave" a series of nested catches before calling the finally handler (due to a single 'leave' opcode attempting to leave multiple levels of different types of handlers). Then, a GT_END_LFIN statement with the finally clause handler nesting level as an argument is added to the step block where the finally returns to. This is used to generate code to zero out the appropriate level of the ShadowSP slot array after the finally has been executed. The BBJ_CALLFINALLY block itself generates the code to insert the 0xFC value into the ShadowSP slot array. If the 'finally' is invoked by the VM, in exceptional cases, then the VM itself updates the ShadowSP slot array before invoking the 'finally'. +The JIT creates BBJ_CALLFINALLY/BBJ_ALWAYS pairs for calling the 'finally' clause. The BBJ_CALLFINALLY block will have a series of CORINFO_JIT_ENDCATCH calls appended at the end, if we need to "leave" a series of nested catches before calling the finally handler (due to a single 'leave' opcode attempting to leave multiple levels of different types of handlers). Then, a GT_END_LFIN statement with EH region ID as an argument is added to the step block where the finally returns to. This is used to generate code to zero out the appropriate level of the ShadowSP slot array after the finally has been executed and the final EH nesting depth is known. The BBJ_CALLFINALLY block itself generates the code to insert the 0xFC value into the ShadowSP slot array. If the 'finally' is invoked by the VM, in exceptional cases, then the VM itself updates the ShadowSP slot array before invoking the 'finally'. At the end of a finally or filter, a GT_RETFILT is inserted. For a finally, this is a TYP_VOID which is just a placeholder. For a filter, it takes an argument which evaluates to the return value from the filter. On legacy JIT, this tree triggers the generation of both the return value load (for filters) and the "funclet" exit sequence, which is either a "pop eax; jmp eax" for a finally, or a "ret" for a filter. When processing the BBJ_EHFINALLYRET or BBJ_EHFILTERRET block itself (at the end of code generation for the block), nothing is generated. In RyuJIT, the GT_RETFILT only loads up the return value (for filters) and does nothing for finally, and the block type processing after all the tree processing triggers the exit sequence to be generated. There is no real difference between these, except to centralize all "exit sequence" generation in the same place. diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp index 4b14b8d1c451dd..39be84ecc03d55 100644 --- a/src/coreclr/jit/codegenxarch.cpp +++ b/src/coreclr/jit/codegenxarch.cpp @@ -2200,13 +2200,22 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) #if defined(FEATURE_EH_WINDOWS_X86) case GT_END_LFIN: + { + // Find the eh table entry via the eh ID + // + unsigned const ehID = (unsigned)treeNode->AsVal()->gtVal1; + assert(ehID < compiler->compEHID); + assert(compiler->m_EHIDtoEHblkDsc != nullptr); + + EHblkDsc* HBtab = nullptr; + bool found = compiler->m_EHIDtoEHblkDsc->Lookup(ehID, &HBtab); + assert(found); + assert(HBtab != nullptr); // Have to clear the ShadowSP of the nesting level which encloses the finally. Generates: // mov dword ptr [ebp-0xC], 0 // for some slot of the ShadowSP local var - - size_t finallyNesting; - finallyNesting = treeNode->AsVal()->gtVal1; - noway_assert(treeNode->AsVal()->gtVal1 < compiler->compHndBBtabCount); + // + const size_t finallyNesting = HBtab->ebdHandlerNestingLevel; noway_assert(finallyNesting < compiler->compHndBBtabCount); // The last slot is reserved for ICodeManager::FixContext(ppEndRegion) @@ -2220,6 +2229,7 @@ void CodeGen::genCodeForTreeNode(GenTree* treeNode) GetEmitter()->emitIns_S_I(INS_mov, EA_PTRSIZE, compiler->lvaShadowSPslotsVar, (unsigned)curNestingSlotOffs, 0); break; + } #endif // FEATURE_EH_WINDOWS_X86 case GT_PINVOKE_PROLOG: diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index b0797e1b27334f..01ab30bc2f26a9 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5207,6 +5207,21 @@ void Compiler::FinalizeEH() lvaSetVarAddrExposed(lvaShadowSPslotsVar DEBUGARG(AddressExposedReason::EXTERNALLY_VISIBLE_IMPLICITLY)); } + // Build up a mapping from EH IDs to EHblkDsc* + // + assert(m_EHIDtoEHblkDsc == nullptr); + + if (compHndBBtabCount > 0) + { + m_EHIDtoEHblkDsc = new (getAllocator()) EHIDtoEHblkDscMap(getAllocator()); + + for (unsigned XTnum = 0; XTnum < compHndBBtabCount; XTnum++) + { + EHblkDsc* const HBtab = &compHndBBtab[XTnum]; + m_EHIDtoEHblkDsc->Set(HBtab->ebdID, HBtab); + } + } + #endif // FEATURE_EH_WINDOWS_X86 // We should not make any more alterations to the EH table structure. diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 9d265af58c019f..f8b6bc19e7489e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2722,8 +2722,12 @@ class Compiler // etc. unsigned ehMaxHndNestingCount = 0; + typedef JitHashTable, EHblkDsc*> EHIDtoEHblkDscMap; + EHIDtoEHblkDscMap* m_EHIDtoEHblkDsc = nullptr; + #endif // FEATURE_EH_WINDOWS_X86 + EHblkDsc* ehFindEHblkDscById(unsigned short ehID); bool ehTableFinalized = false; void FinalizeEH(); @@ -10977,9 +10981,10 @@ class Compiler size_t compInfoBlkSize; BYTE* compInfoBlkAddr; - EHblkDsc* compHndBBtab = nullptr; // array of EH data - unsigned compHndBBtabCount = 0; // element count of used elements in EH data array - unsigned compHndBBtabAllocCount = 0; // element count of allocated elements in EH data array + EHblkDsc* compHndBBtab = nullptr; // array of EH data + unsigned compHndBBtabCount = 0; // element count of used elements in EH data array + unsigned compHndBBtabAllocCount = 0; // element count of allocated elements in EH data array + unsigned short compEHID = 0; // unique ID for EH data array entries #if defined(FEATURE_EH_WINDOWS_X86) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index e593b92c6b7da6..613a0538c71883 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -3646,6 +3646,7 @@ void Compiler::fgFindBasicBlocks() BADCODE3("end of hnd block beyond end of method for try", " at offset %04X", tryBegOff); } + HBtab->ebdID = impInlineRoot()->compEHID++; HBtab->ebdTryBegOffset = tryBegOff; HBtab->ebdTryEndOffset = tryEndOff; HBtab->ebdFilterBegOffset = filterBegOff; diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 6e0b726266bad6..61bd417b5d16ec 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -690,7 +690,9 @@ PhaseStatus Compiler::fgRemoveEmptyTry() // Handler index of any nested blocks will update when we // remove the EH table entry. Change handler exits to jump to // the continuation. Clear catch type on handler entry. - // Decrement nesting level of enclosed GT_END_LFINs. + // + // GT_END_LFIN no longer need updates here, now their gtVal1 fields refer to EH IDs. + // for (BasicBlock* const block : Blocks(firstHandlerBlock, lastHandlerBlock)) { if (block == firstHandlerBlock) @@ -725,25 +727,6 @@ PhaseStatus Compiler::fgRemoveEmptyTry() } } } - -#if defined(FEATURE_EH_WINDOWS_X86) - if (!UsesFunclets()) - { - // If we're in a non-funclet model, decrement the nesting - // level of any GT_END_LFIN we find in the handler region, - // since we're removing the enclosing handler. - for (Statement* const stmt : block->Statements()) - { - GenTree* expr = stmt->GetRootNode(); - if (expr->gtOper == GT_END_LFIN) - { - const size_t nestLevel = expr->AsVal()->gtVal1; - assert(nestLevel > 0); - expr->AsVal()->gtVal1 = nestLevel - 1; - } - } - } -#endif // FEATURE_EH_WINDOWS_X86 } // (6) Update any impacted ACDs. @@ -2870,6 +2853,8 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info, compHndBBtab[XTnum] = compHndBBtab[originalXTnum]; EHblkDsc* const ebd = &compHndBBtab[XTnum]; + ebd->ebdID = impInlineRoot()->compEHID++; + // Note the outermost region enclosing indices stay the same, because the original // clause entries got adjusted when we inserted the new clauses. // @@ -3028,6 +3013,22 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info, newBlock->bbRefs++; } } + +#if defined(FEATURE_EH_WINDOWS_X86) + // Update the EH ID for any cloned GT_END_LFIN. + // + for (Statement* const stmt : newBlock->Statements()) + { + GenTree* const rootNode = stmt->GetRootNode(); + if (rootNode->OperIs(GT_END_LFIN)) + { + GenTreeVal* const endNode = rootNode->AsVal(); + EHblkDsc* const oldEbd = ehFindEHblkDscById((unsigned short)endNode->gtVal1); + EHblkDsc* const newEbd = oldEbd + indexShift; + endNode->gtVal1 = newEbd->ebdID; + } + } +#endif } JITDUMP("Done fixing region indices\n"); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c900ae003fe1dc..fe3165adaec471 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1463,6 +1463,7 @@ void Compiler::fgAddSyncMethodEnterExit() // Initialize the new entry + newEntry->ebdID = impInlineRoot()->compEHID++; newEntry->ebdHandlerType = EH_HANDLER_FAULT; newEntry->ebdTryBeg = tryBegBB; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 86357b0536c09b..ff2c9583ea8af1 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -12333,7 +12333,7 @@ void Compiler::gtDispLeaf(GenTree* tree, IndentStack* indentStack) #if defined(FEATURE_EH_WINDOWS_X86) case GT_END_LFIN: - printf(" endNstLvl=%d", tree->AsVal()->gtVal1); + printf(" ehID=%d", tree->AsVal()->gtVal1); break; #endif // FEATURE_EH_WINDOWS_X86 diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c4fae10ce005ab..1e503daabb0f33 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -4747,12 +4747,15 @@ void Compiler::impImportLeaveEHRegions(BasicBlock* block) } #endif - unsigned finallyNesting = compHndBBtab[XTnum].ebdHandlerNestingLevel; - assert(finallyNesting <= compHndBBtabCount); + // We now record the EH region ID on GT_END_LFIN instead of the finally nesting depth, + // as the later can change as we optimize the code. + // + unsigned const ehID = compHndBBtab[XTnum].ebdID; + assert(ehID <= impInlineRoot()->compEHID); - GenTree* endLFin = new (this, GT_END_LFIN) GenTreeVal(GT_END_LFIN, TYP_VOID, finallyNesting); - endLFinStmt = gtNewStmt(endLFin); - endCatches = NULL; + GenTree* const endLFin = new (this, GT_END_LFIN) GenTreeVal(GT_END_LFIN, TYP_VOID, ehID); + endLFinStmt = gtNewStmt(endLFin); + endCatches = NULL; encFinallies++; } diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 3356349e61adc4..0ed9dcdd24c847 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -241,7 +241,7 @@ bool EHblkDsc::ebdIsSameTry(BasicBlock* ebdTryBeg, BasicBlock* ebdTryLast) void EHblkDsc::DispEntry(unsigned XTnum) { - printf(" %2u ::", XTnum); + printf(" %2u %2u ::", ebdID, XTnum); #if defined(FEATURE_EH_WINDOWS_X86) if (ebdHandlerNestingLevel == 0) @@ -1258,6 +1258,30 @@ EHblkDsc* Compiler::ehInitTryBlockRange(BasicBlock* blk, BasicBlock** tryBeg, Ba return tryTab; } +//------------------------------------------------------------------------ +// ehFindEHblkDscById: find an eh table entry by its ID +// +// Argument: +// ID to use in search +// +// Returns: +// Pointer to the entry, or nullptr +// +EHblkDsc* Compiler::ehFindEHblkDscById(unsigned short id) +{ + EHblkDsc* result = nullptr; + for (EHblkDsc* const xtab : EHClauses(this)) + { + if (xtab->ebdID == id) + { + result = xtab; + break; + } + } + + return result; +} + /***************************************************************************** * This method updates the value of ebdTryBeg */ @@ -3230,6 +3254,9 @@ void Compiler::fgVerifyHandlerTab() // block (case 3)? bool multipleLastBlockNormalizationDone = false; // Currently disabled + BitVecTraits traits(impInlineRoot()->compEHID, this); + BitVec ids(BitVecOps::MakeEmpty(&traits)); + assert(compHndBBtabCount <= compHndBBtabAllocCount); unsigned XTnum; @@ -3237,6 +3264,11 @@ void Compiler::fgVerifyHandlerTab() for (XTnum = 0, HBtab = compHndBBtab; XTnum < compHndBBtabCount; XTnum++, HBtab++) { + // EH IDs should be unique and in range + // + assert(HBtab->ebdID < impInlineRoot()->compEHID); + assert(BitVecOps::TryAddElemD(&traits, ids, HBtab->ebdID)); + assert(HBtab->ebdTryBeg != nullptr); assert(HBtab->ebdTryLast != nullptr); assert(HBtab->ebdHndBeg != nullptr); @@ -3763,7 +3795,7 @@ void Compiler::fgDispHandlerTab() return; } - printf("\nindex "); + printf("\n id, index "); #if defined(FEATURE_EH_WINDOWS_X86) if (!UsesFunclets()) { diff --git a/src/coreclr/jit/jiteh.h b/src/coreclr/jit/jiteh.h index eb4c1bfbd5baf6..34cae18ec950cb 100644 --- a/src/coreclr/jit/jiteh.h +++ b/src/coreclr/jit/jiteh.h @@ -89,6 +89,8 @@ struct EHblkDsc unsigned ebdTyp; // Exception type (a class token), otherwise }; + unsigned short ebdID; // Unique ID for this eh descriptor (stable across add/delete/inlining) + EHHandlerType ebdHandlerType; #if defined(FEATURE_EH_WINDOWS_X86)