diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 37fe0128d4d5d8..fe8e2bbd21930c 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -4899,7 +4899,7 @@ class Compiler void fgPerNodeLocalVarLiveness(GenTreeHWIntrinsic* hwintrinsic); #endif // FEATURE_HW_INTRINSICS - VARSET_VALRET_TP fgGetHandlerLiveVars(BasicBlock* block); + bool fgGetHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars); void fgLiveVarAnalysis(bool updateInternalOnly = false); diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index f59d83e6fe26fb..b8242d946f983a 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -649,7 +649,7 @@ bool Compiler::ehIsBlockEHLast(BasicBlock* block) //------------------------------------------------------------------------ // ehGetBlockExnFlowDsc: // Get the EH descriptor for the most nested region (if any) that may -// handle exceptions raised in the given block +// handle exceptions raised in the given block. // // Arguments: // block - Consider exceptions raised from this block @@ -658,7 +658,7 @@ bool Compiler::ehIsBlockEHLast(BasicBlock* block) // nullptr - The given block's exceptions propagate to caller // non-null - This region is the innermost handler for exceptions raised in // the given block - +// EHblkDsc* Compiler::ehGetBlockExnFlowDsc(BasicBlock* block) { EHblkDsc* hndDesc = ehGetBlockHndDsc(block); @@ -668,9 +668,10 @@ EHblkDsc* Compiler::ehGetBlockExnFlowDsc(BasicBlock* block) // If an exception is thrown in a filter (or escapes a callee in a filter), // or if exception_continue_search (0/false) is returned at // the end of a filter, the (original) exception is propagated to - // the next outer handler. The "next outer handler" is the handler - // of the try region enclosing the try that the filter protects. - // This may not be the same as the try region enclosing the filter, + // the next outer handler. This is because a new exception thrown in + // a filter is discarded and the filter is treated as returning 0. + // The "next outer handler" is the handler of the try region enclosing the try + // that the filter protects. This may not be the same as the try region enclosing the filter, // e.g. in cases like this: // try { // ... diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index d45d90b90df2b3..19986177121347 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -1044,9 +1044,11 @@ void Compiler::fgExtendDbgLifetimes() // // Arguments: // block - the block in question +// ehHandlerLiveVars - On entry, contains an allocated VARSET_TP. On exit, contains the set of handler live vars. // // Returns: -// Additional set of locals to be considered live throughout the block. +// `true` if there are any EH live vars (`ehHandlerLiveVars` is updated in this case). +// `false` if there are no EH live vars. // // Notes: // Assumes caller has screened candidate blocks to only those with @@ -1073,57 +1075,59 @@ void Compiler::fgExtendDbgLifetimes() // { // Console.WriteLine("In catch 1"); // } - -VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block) +// +bool Compiler::fgGetHandlerLiveVars(BasicBlock* block, VARSET_TP& ehHandlerLiveVars) { - noway_assert(block); - noway_assert(ehBlockHasExnFlowDsc(block)); + bool anyEhVars = false; - VARSET_TP liveVars(VarSetOps::MakeEmpty(this)); - EHblkDsc* HBtab = ehGetBlockExnFlowDsc(block); + VarSetOps::ClearD(this, ehHandlerLiveVars); - do + if (ehBlockHasExnFlowDsc(block)) { - /* Either we enter the filter first or the catch/finally */ - if (HBtab->HasFilter()) + anyEhVars = true; + + EHblkDsc* HBtab = ehGetBlockExnFlowDsc(block); + + do { - VarSetOps::UnionD(this, liveVars, HBtab->ebdFilter->bbLiveIn); + /* Either we enter the filter first or the catch/finally */ + if (HBtab->HasFilter()) + { + VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdFilter->bbLiveIn); #if defined(FEATURE_EH_FUNCLETS) - // The EH subsystem can trigger a stack walk after the filter - // has returned, but before invoking the handler, and the only - // IP address reported from this method will be the original - // faulting instruction, thus everything in the try body - // must report as live any variables live-out of the filter - // (which is the same as those live-in to the handler) - VarSetOps::UnionD(this, liveVars, HBtab->ebdHndBeg->bbLiveIn); + // The EH subsystem can trigger a stack walk after the filter + // has returned, but before invoking the handler, and the only + // IP address reported from this method will be the original + // faulting instruction, thus everything in the try body + // must report as live any variables live-out of the filter + // (which is the same as those live-in to the handler) + VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdHndBeg->bbLiveIn); #endif // FEATURE_EH_FUNCLETS - } - else - { - VarSetOps::UnionD(this, liveVars, HBtab->ebdHndBeg->bbLiveIn); - } + } + else + { + VarSetOps::UnionD(this, ehHandlerLiveVars, HBtab->ebdHndBeg->bbLiveIn); + } - /* If we have nested try's edbEnclosing will provide them */ - noway_assert((HBtab->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) || - (HBtab->ebdEnclosingTryIndex > ehGetIndex(HBtab))); + /* If we have nested try's edbEnclosing will provide them */ + noway_assert((HBtab->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) || + (HBtab->ebdEnclosingTryIndex > ehGetIndex(HBtab))); - unsigned outerIndex = HBtab->ebdEnclosingTryIndex; - if (outerIndex == EHblkDsc::NO_ENCLOSING_INDEX) - { - break; - } - HBtab = ehGetDsc(outerIndex); + unsigned outerIndex = HBtab->ebdEnclosingTryIndex; + if (outerIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + break; + } + HBtab = ehGetDsc(outerIndex); - } while (true); + } while (true); + } // If this block is within a filter, we also need to report as live // any vars live into enclosed finally or fault handlers, since the // filter will run during the first EH pass, and enclosed or enclosing // handlers will run during the second EH pass. So all these handlers // are "exception flow" successors of the filter. - // - // Note we are relying on ehBlockHasExnFlowDsc to return true - // for any filter block that we should examine here. if (block->hasHndIndex()) { const unsigned thisHndIndex = block->getHndIndex(); @@ -1170,7 +1174,8 @@ VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block) if (enclosedHBtab->HasFinallyOrFaultHandler()) { - VarSetOps::UnionD(this, liveVars, enclosedHBtab->ebdHndBeg->bbLiveIn); + anyEhVars = true; + VarSetOps::UnionD(this, ehHandlerLiveVars, enclosedHBtab->ebdHndBeg->bbLiveIn); } } // Once we run across a non-enclosed region, we can stop searching. @@ -1182,7 +1187,7 @@ VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block) } } - return liveVars; + return anyEhVars; } class LiveVarAnalysis @@ -1195,6 +1200,7 @@ class LiveVarAnalysis unsigned m_memoryLiveOut; VARSET_TP m_liveIn; VARSET_TP m_liveOut; + VARSET_TP m_ehHandlerLiveVars; LiveVarAnalysis(Compiler* compiler) : m_compiler(compiler) @@ -1203,6 +1209,7 @@ class LiveVarAnalysis , m_memoryLiveOut(emptyMemoryKindSet) , m_liveIn(VarSetOps::MakeEmpty(compiler)) , m_liveOut(VarSetOps::MakeEmpty(compiler)) + , m_ehHandlerLiveVars(VarSetOps::MakeEmpty(compiler)) { } @@ -1278,11 +1285,10 @@ class LiveVarAnalysis // Does this block have implicit exception flow to a filter or handler? // If so, include the effects of that flow. - if (m_compiler->ehBlockHasExnFlowDsc(block)) + if (m_compiler->fgGetHandlerLiveVars(block, m_ehHandlerLiveVars)) { - const VARSET_TP& liveVars(m_compiler->fgGetHandlerLiveVars(block)); - VarSetOps::UnionD(m_compiler, m_liveIn, liveVars); - VarSetOps::UnionD(m_compiler, m_liveOut, liveVars); + VarSetOps::UnionD(m_compiler, m_liveIn, m_ehHandlerLiveVars); + VarSetOps::UnionD(m_compiler, m_liveOut, m_ehHandlerLiveVars); // Implicit eh edges can induce loop-like behavior, // so make sure we iterate to closure. @@ -2568,6 +2574,8 @@ void Compiler::fgInterBlockLocalVarLiveness() * Now fill in liveness info within each basic block - Backward DataFlow */ + VARSET_TP volatileVars(VarSetOps::MakeEmpty(this)); + for (BasicBlock* const block : Blocks()) { /* Tell everyone what block we're working on */ @@ -2577,12 +2585,8 @@ void Compiler::fgInterBlockLocalVarLiveness() /* Remember those vars live on entry to exception handlers */ /* if we are part of a try block */ - VARSET_TP volatileVars(VarSetOps::MakeEmpty(this)); - - if (ehBlockHasExnFlowDsc(block)) + if (fgGetHandlerLiveVars(block, volatileVars)) { - VarSetOps::Assign(this, volatileVars, fgGetHandlerLiveVars(block)); - // volatileVars is a subset of exceptVars noway_assert(VarSetOps::IsSubset(this, volatileVars, exceptVars)); } diff --git a/src/coreclr/jit/lsra.cpp b/src/coreclr/jit/lsra.cpp index b33bd2ab325237..7fa95c3550e09b 100644 --- a/src/coreclr/jit/lsra.cpp +++ b/src/coreclr/jit/lsra.cpp @@ -2396,9 +2396,10 @@ void LinearScan::checkLastUses(BasicBlock* block) VARSET_TP liveInNotComputedLive(VarSetOps::Diff(compiler, block->bbLiveIn, computedLive)); // We may have exception vars in the liveIn set of exception blocks that are not computed live. - if (compiler->ehBlockHasExnFlowDsc(block)) + VARSET_TP ehHandlerLiveVars(VarSetOps::MakeEmpty(compiler)); + if (compiler->fgGetHandlerLiveVars(block, ehHandlerLiveVars)) { - VarSetOps::DiffD(compiler, liveInNotComputedLive, compiler->fgGetHandlerLiveVars(block)); + VarSetOps::DiffD(compiler, liveInNotComputedLive, ehHandlerLiveVars); } VarSetOps::Iter liveInNotComputedLiveIter(compiler, liveInNotComputedLive); unsigned liveInNotComputedLiveIndex = 0; diff --git a/src/coreclr/jit/promotionliveness.cpp b/src/coreclr/jit/promotionliveness.cpp index 9791bd08a0d90b..c81af123abc9d3 100644 --- a/src/coreclr/jit/promotionliveness.cpp +++ b/src/coreclr/jit/promotionliveness.cpp @@ -352,10 +352,9 @@ bool PromotionLiveness::PerBlockLiveness(BasicBlock* block) BitVecOps::LivenessD(m_bvTraits, m_liveIn, bbInfo.VarDef, bbInfo.VarUse, bbInfo.LiveOut); - if (m_compiler->ehBlockHasExnFlowDsc(block)) + AddHandlerLiveVars(block, m_ehLiveVars); + if (!BitVecOps::IsEmpty(m_bvTraits, m_ehLiveVars)) { - BitVecOps::ClearD(m_bvTraits, m_ehLiveVars); - AddHandlerLiveVars(block, m_ehLiveVars); BitVecOps::UnionD(m_bvTraits, m_liveIn, m_ehLiveVars); BitVecOps::UnionD(m_bvTraits, bbInfo.LiveOut, m_ehLiveVars); m_hasPossibleBackEdge = true; @@ -385,51 +384,52 @@ bool PromotionLiveness::PerBlockLiveness(BasicBlock* block) // void PromotionLiveness::AddHandlerLiveVars(BasicBlock* block, BitVec& ehLiveVars) { - assert(m_compiler->ehBlockHasExnFlowDsc(block)); - EHblkDsc* HBtab = m_compiler->ehGetBlockExnFlowDsc(block); + BitVecOps::ClearD(m_bvTraits, ehLiveVars); - do + if (m_compiler->ehBlockHasExnFlowDsc(block)) { - // Either we enter the filter first or the catch/finally - if (HBtab->HasFilter()) + EHblkDsc* HBtab = m_compiler->ehGetBlockExnFlowDsc(block); + + do { - BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdFilter->bbNum].LiveIn); + // Either we enter the filter first or the catch/finally + if (HBtab->HasFilter()) + { + BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdFilter->bbNum].LiveIn); #if defined(FEATURE_EH_FUNCLETS) - // The EH subsystem can trigger a stack walk after the filter - // has returned, but before invoking the handler, and the only - // IP address reported from this method will be the original - // faulting instruction, thus everything in the try body - // must report as live any variables live-out of the filter - // (which is the same as those live-in to the handler) - BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdHndBeg->bbNum].LiveIn); + // The EH subsystem can trigger a stack walk after the filter + // has returned, but before invoking the handler, and the only + // IP address reported from this method will be the original + // faulting instruction, thus everything in the try body + // must report as live any variables live-out of the filter + // (which is the same as those live-in to the handler) + BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdHndBeg->bbNum].LiveIn); #endif // FEATURE_EH_FUNCLETS - } - else - { - BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdHndBeg->bbNum].LiveIn); - } + } + else + { + BitVecOps::UnionD(m_bvTraits, ehLiveVars, m_bbInfo[HBtab->ebdHndBeg->bbNum].LiveIn); + } - // If we have nested try's edbEnclosing will provide them - assert((HBtab->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) || - (HBtab->ebdEnclosingTryIndex > m_compiler->ehGetIndex(HBtab))); + // If we have nested try's edbEnclosing will provide them + assert((HBtab->ebdEnclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) || + (HBtab->ebdEnclosingTryIndex > m_compiler->ehGetIndex(HBtab))); - unsigned outerIndex = HBtab->ebdEnclosingTryIndex; - if (outerIndex == EHblkDsc::NO_ENCLOSING_INDEX) - { - break; - } - HBtab = m_compiler->ehGetDsc(outerIndex); + unsigned outerIndex = HBtab->ebdEnclosingTryIndex; + if (outerIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + break; + } + HBtab = m_compiler->ehGetDsc(outerIndex); - } while (true); + } while (true); + } // If this block is within a filter, we also need to report as live // any vars live into enclosed finally or fault handlers, since the // filter will run during the first EH pass, and enclosed or enclosing // handlers will run during the second EH pass. So all these handlers // are "exception flow" successors of the filter. - // - // Note we are relying on ehBlockHasExnFlowDsc to return true - // for any filter block that we should examine here. if (block->hasHndIndex()) { const unsigned thisHndIndex = block->getHndIndex(); @@ -508,12 +508,7 @@ void PromotionLiveness::FillInLiveness() BasicBlockLiveness& bbInfo = m_bbInfo[block->bbNum]; - BitVecOps::ClearD(m_bvTraits, volatileVars); - - if (m_compiler->ehBlockHasExnFlowDsc(block)) - { - AddHandlerLiveVars(block, volatileVars); - } + AddHandlerLiveVars(block, volatileVars); BitVecOps::Assign(m_bvTraits, life, bbInfo.LiveOut); diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.cs b/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.cs new file mode 100644 index 00000000000000..81eebb9e1a342a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.cs @@ -0,0 +1,67 @@ +// 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 Xunit; + +// Repro for https://github.com/dotnet/runtime/issues/88168. +// Derived from (and similar to) test GitHub_22820 for https://github.com/dotnet/coreclr/issues/22820. +// +// Run with optimized codegen and DOTNET_GCStress=0x4 + +class DisposableObject : IDisposable +{ + public void Dispose() + { + Console.WriteLine("In dispose"); + } +} + +public class Program +{ + public static bool IsExpectedException(Exception e) + { + Console.WriteLine("In filter"); + GC.Collect(); + return e is OperationCanceledException; + } + + public static IDisposable AllocateObject() + { + return new DisposableObject(); + } + + [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)] + private static void top_level_filter_test() + { + try + { + using (AllocateObject()) + { + throw new Exception(); + } + } + catch (Exception e1) when (IsExpectedException(e1)) + { + Console.WriteLine("In catch 1"); + } + } + + [Fact] + public static int TestEntryPoint() + { + int result = 0; + + try + { + top_level_filter_test(); + } + catch (Exception e2) + { + Console.WriteLine("In catch 2"); + result = 100; + } + + return result; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.csproj new file mode 100644 index 00000000000000..501217e4d86892 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_88168/Runtime_88168.csproj @@ -0,0 +1,9 @@ + + + None + True + + + + +