JIT: Assert bbJumpKind in BasicBlock::HasInitializedJumpDest (#95152)

Based on feedback in #94239, before reading a block's bbJumpDest in BasicBlock::HasInitializedJumpDest, we should assert it has a valid jump kind. In other words, calling HasInitializedJumpDest on a block that is never expected to have a jump destination should be considered invalid behavior.

src/coreclr/jit/block.h

@@ -541,10 +541,10 @@ struct BasicBlock : private LIR::Range
     void SetJumpKind(BBjumpKinds jumpKind)
     {
         // If this block's jump kind requires a target, ensure it is already set
-        assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE));
+        assert(!HasJumpDest() || HasInitializedJumpDest());
         bbJumpKind = jumpKind;
         // If new jump kind requires a target, ensure a target is already set
-        assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE));
+        assert(!HasJumpDest() || HasInitializedJumpDest());
     }
 
     BasicBlock* Prev() const
@@ -611,10 +611,16 @@ struct BasicBlock : private LIR::Range
         assert(KindIs(BBJ_ALWAYS, BBJ_COND, BBJ_LEAVE));
     }
 
+    bool HasJumpDest() const
+    {
+        // These block types should always have bbJumpDest set
+        return KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_EHFILTERRET, BBJ_LEAVE);
+    }
+
     BasicBlock* GetJumpDest() const
     {
         // If bbJumpKind indicates this block has a jump, bbJumpDest cannot be null
-        assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE));
+        assert(!HasJumpDest() || HasInitializedJumpDest());
         return bbJumpDest;
     }
 
@@ -623,7 +629,7 @@ struct BasicBlock : private LIR::Range
         // SetJumpKindAndTarget() nulls jumpDest for non-jump kinds,
         // so don't use SetJumpDest() to null bbJumpDest without updating bbJumpKind.
         bbJumpDest = jumpDest;
-        assert(HasJump());
+        assert(!HasJumpDest() || HasInitializedJumpDest());
     }
 
     void SetJumpKindAndTarget(BBjumpKinds jumpKind, BasicBlock* jumpDest = nullptr)
@@ -632,24 +638,24 @@ struct BasicBlock : private LIR::Range
         bbJumpDest = jumpDest;
 
         // If bbJumpKind indicates this block has a jump, bbJumpDest cannot be null
-        assert(HasJump() || !KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE));
+        assert(!HasJumpDest() || HasInitializedJumpDest());
     }
 
-    bool HasJump() const
+    bool HasInitializedJumpDest() const
     {
+        assert(HasJumpDest());
         return (bbJumpDest != nullptr);
     }
 
     bool HasJumpTo(const BasicBlock* jumpDest) const
     {
-        assert(HasJump());
-        assert(!KindIs(BBJ_SWITCH, BBJ_EHFINALLYRET));
+        assert(HasInitializedJumpDest());
         return (bbJumpDest == jumpDest);
     }
 
     bool JumpsToNext() const
     {
-        assert(HasJump());
+        assert(HasInitializedJumpDest());
         return (bbJumpDest == bbNext);
     }
 

src/coreclr/jit/fgbasic.cpp

@@ -2934,7 +2934,7 @@ void Compiler::fgLinkBasicBlocks()
             {
                 BasicBlock* const jumpDest = fgLookupBB(curBBdesc->GetJumpOffs());
                 // Redundantly use SetJumpKindAndTarget() instead of SetJumpDest() just this once,
-                // so we don't break the HasJump() invariant of SetJumpDest().
+                // so we don't break the HasInitializedJumpDest() invariant of SetJumpDest().
                 curBBdesc->SetJumpKindAndTarget(curBBdesc->GetJumpKind(), jumpDest);
                 fgAddRefPred<initializingPreds>(jumpDest, curBBdesc, oldEdge);
 
@@ -5611,7 +5611,7 @@ BasicBlock* Compiler::fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst)
             // (It's possible for bSrc's jump destination to not be set yet,
             // so check with BasicBlock::HasJump before calling BasicBlock::JumpsToNext)
             //
-            if (bSrc->KindIs(BBJ_ALWAYS) && !(bSrc->bbFlags & BBF_KEEP_BBJ_ALWAYS) && bSrc->HasJump() &&
+            if (bSrc->KindIs(BBJ_ALWAYS) && !(bSrc->bbFlags & BBF_KEEP_BBJ_ALWAYS) && bSrc->HasInitializedJumpDest() &&
                 bSrc->JumpsToNext())
             {
                 bSrc->SetJumpKindAndTarget(BBJ_NONE);

src/coreclr/jit/fgdiagnostic.cpp

@@ -3121,9 +3121,9 @@ void Compiler::fgDebugCheckBBlist(bool checkBBNum /* = false */, bool checkBBRef
         }
 
         // Blocks with these jump kinds must have non-null jump targets
-        if (block->KindIs(BBJ_ALWAYS, BBJ_CALLFINALLY, BBJ_COND, BBJ_EHCATCHRET, BBJ_LEAVE))
+        if (block->HasJumpDest())
         {
-            assert(block->HasJump());
+            assert(block->HasInitializedJumpDest());
         }
 
         // A branch or fall-through to a BBJ_CALLFINALLY block must come from the `try` region associated

src/coreclr/jit/fgehopt.cpp

@@ -454,7 +454,7 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
         // Time to optimize.
         //
         // (1) Convert the callfinally to a normal jump to the handler
-        assert(callFinally->HasJump());
+        assert(callFinally->HasInitializedJumpDest());
         callFinally->SetJumpKind(BBJ_ALWAYS);
 
         // Identify the leave block and the continuation
@@ -1081,7 +1081,7 @@ PhaseStatus Compiler::fgCloneFinally()
             newBlock->clearHndIndex();
 
             // Jump dests are set in a post-pass; make sure CloneBlockState hasn't tried to set them.
-            assert(!newBlock->HasJump());
+            assert(newBlock->KindIs(BBJ_NONE));
         }
 
         if (!clonedOk)

src/coreclr/jit/fgprofile.cpp

@@ -3796,7 +3796,7 @@ void EfficientEdgeCountReconstructor::PropagateEdges(BasicBlock* block, BlockInf
     {
         assert(nSucc == 1);
         assert(block == pseudoEdge->m_sourceBlock);
-        assert(block->HasJump());
+        assert(block->HasInitializedJumpDest());
         FlowEdge* const flowEdge = m_comp->fgGetPredForBlock(block->GetJumpDest(), block);
         assert(flowEdge != nullptr);
         flowEdge->setLikelihood(1.0);

src/coreclr/jit/ifconversion.cpp

@@ -743,7 +743,7 @@ bool OptIfConversionDsc::optIfConvert()
 
     // Update the flow from the original block.
     m_comp->fgRemoveAllRefPreds(m_startBlock->Next(), m_startBlock);
-    assert(m_startBlock->HasJump());
+    assert(m_startBlock->HasInitializedJumpDest());
     m_startBlock->SetJumpKind(BBJ_ALWAYS);
 
 #ifdef DEBUG

src/coreclr/jit/importer.cpp

@@ -4323,7 +4323,7 @@ void Compiler::impImportLeave(BasicBlock* block)
                 assert(step == DUMMY_INIT(NULL));
                 callBlock = block;
 
-                assert(callBlock->HasJump());
+                assert(callBlock->HasInitializedJumpDest());
                 fgRemoveRefPred(callBlock->GetJumpDest(), callBlock);
 
                 // callBlock will call the finally handler. Convert the BBJ_LEAVE to BBJ_CALLFINALLY
@@ -4353,7 +4353,7 @@ void Compiler::impImportLeave(BasicBlock* block)
                 callBlock = fgNewBBinRegion(BBJ_CALLFINALLY, XTnum + 1, 0, step, HBtab->ebdHndBeg);
 
                 // step's jump target shouldn't be set yet
-                assert(!step->HasJump());
+                assert(!step->HasInitializedJumpDest());
 
                 // the previous call to a finally returns to this call (to the next finally in the chain)
                 step->SetJumpDest(callBlock);
@@ -4453,7 +4453,7 @@ void Compiler::impImportLeave(BasicBlock* block)
         finalStep->bbFlags |= BBF_KEEP_BBJ_ALWAYS;
 
         // step's jump target shouldn't be set yet
-        assert(!step->HasJump());
+        assert(!step->HasInitializedJumpDest());
 
         step->SetJumpDest(finalStep);
         fgAddRefPred(finalStep, step);
@@ -4598,7 +4598,7 @@ void Compiler::impImportLeave(BasicBlock* block)
                 BasicBlock* exitBlock = fgNewBBinRegion(BBJ_EHCATCHRET, 0, XTnum + 1, step);
 
                 assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET));
-                assert((step == block) || !step->HasJump());
+                assert((step == block) || !step->HasInitializedJumpDest());
                 if (step == block)
                 {
                     fgRemoveRefPred(step->GetJumpDest(), step);
@@ -4669,7 +4669,7 @@ void Compiler::impImportLeave(BasicBlock* block)
 
                 callBlock = block;
 
-                assert(callBlock->HasJump());
+                assert(callBlock->HasInitializedJumpDest());
                 fgRemoveRefPred(callBlock->GetJumpDest(), callBlock);
 
                 // callBlock will call the finally handler. Convert the BBJ_LEAVE to BBJ_CALLFINALLY
@@ -4706,7 +4706,7 @@ void Compiler::impImportLeave(BasicBlock* block)
                 // stack walks.)
 
                 assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET));
-                assert((step == block) || !step->HasJump());
+                assert((step == block) || !step->HasInitializedJumpDest());
 
 #if FEATURE_EH_CALLFINALLY_THUNKS
                 if (step->KindIs(BBJ_EHCATCHRET))
@@ -4749,7 +4749,7 @@ void Compiler::impImportLeave(BasicBlock* block)
 #endif // !FEATURE_EH_CALLFINALLY_THUNKS
 
                 assert(step->KindIs(BBJ_ALWAYS, BBJ_EHCATCHRET));
-                assert((step == block) || !step->HasJump());
+                assert((step == block) || !step->HasInitializedJumpDest());
 
                 // callBlock will call the finally handler
                 callBlock =
@@ -4841,7 +4841,7 @@ void Compiler::impImportLeave(BasicBlock* block)
             if ((stepType == ST_FinallyReturn) || (stepType == ST_Catch))
             {
                 assert(step);
-                assert((step == block) || !step->HasJump());
+                assert((step == block) || !step->HasInitializedJumpDest());
 
                 if (stepType == ST_FinallyReturn)
                 {
@@ -4909,7 +4909,7 @@ void Compiler::impImportLeave(BasicBlock* block)
     }
     else
     {
-        assert((step == block) || !step->HasJump());
+        assert((step == block) || !step->HasInitializedJumpDest());
 
         if (step == block)
         {
@@ -7530,11 +7530,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 assertImp((genActualType(op1) == genActualType(op2)) || (varTypeIsI(op1) && varTypeIsI(op2)) ||
                           (varTypeIsFloating(op1) && varTypeIsFloating(op2)));
 
-                if (block->KindIs(BBJ_NONE))
-                {
-                    assert(!block->HasJump());
-                }
-
                 if (opts.OptimizationEnabled() && (block->KindIs(BBJ_NONE) || block->JumpsToNext()))
                 {
                     // We may have already modified `block`'s jump kind, if this is a re-importation.

src/coreclr/jit/optimizer.cpp

@@ -2389,7 +2389,7 @@ class LoopSearch
             case BBJ_CALLFINALLY:
             case BBJ_ALWAYS:
             case BBJ_EHCATCHRET:
-                assert(block->HasJump());
+                assert(block->HasInitializedJumpDest());
                 exitPoint = block->GetJumpDest();
 
                 if (!loopBlocks.IsMember(exitPoint->bbNum))
@@ -4402,7 +4402,7 @@ PhaseStatus Compiler::optUnrollLoops()
                     newBlock->scaleBBWeight(1.0 / BB_LOOP_WEIGHT_SCALE);
 
                     // Jump dests are set in a post-pass; make sure CloneBlockState hasn't tried to set them.
-                    assert(!newBlock->HasJump());
+                    assert(newBlock->KindIs(BBJ_NONE));
 
                     if (block == bottom)
                     {