Insert a breakpoint instruction after throwing calls

eng/Subsets.props

@@ -394,7 +394,7 @@
       Test="true" Category="tools"/>
   </ItemGroup>
 
-  <ItemGroup Condition="$(_subset.Contains('+clr.nativecorelib+'))">
+  <ItemGroup Condition="$(_subset.Contains('+clr.nativecorelib+')) and '$(TargetArchitecture)' != 'arm'">
     <ProjectToBuild Include="$(CoreClrProjectRoot)crossgen-corelib.proj" Category="clr" />
   </ItemGroup>
 

src/coreclr/jit/codegenarmarch.cpp

@@ -3411,6 +3411,18 @@ void CodeGen::genCall(GenTreeCall* call)
     assert((gcInfo.gcRegByrefSetCur & killMask) == 0);
 #endif
 
+    if (call->IsNoReturn())
+    {
+        // There are several situations when we need to add another instruction
+        // after a throwing call to help the OS unwinder determine the correct context during unwind.
+        // It also ensures that the gc register liveness doesn't change across throwing call instructions
+        // in fully-interruptible mode.
+        instGen(INS_BREAKPOINT);
+
+        // nothing else needs to be emitted for this call
+        return;
+    }
+
     var_types returnType = call->TypeGet();
     if (returnType != TYP_VOID)
     {

src/coreclr/jit/codegenlinear.cpp

@@ -404,7 +404,7 @@ void CodeGen::genCodeForBBlist()
         }
 
         // Traverse the block in linear order, generating code for each node as we
-        // as we encounter it.
+        // encounter them.
 
 #ifdef DEBUG
         // Set the use-order numbers for each node.
@@ -469,11 +469,36 @@ void CodeGen::genCodeForBBlist()
 #endif // DEBUG
             }
 
+            // generate code for the node
             genCodeForTreeNode(node);
+
             if (node->gtHasReg(compiler) && node->IsUnusedValue())
             {
                 genConsumeReg(node);
             }
+
+            if (node->gtOper == GT_CALL && node->AsCall()->IsNoReturn())
+            {
+                assert(block->KindIs(BBJ_THROW));
+                if (node->IsValue() && !node->IsUnusedValue())
+                {
+                    node->SetUnusedValue();
+                    if (node->gtHasReg(compiler))
+                    {
+                        genConsumeReg(node);
+                    }
+                }
+
+                // the rest of the block is unreachable.
+                LIR::Range& blockRange = LIR::AsRange(block);
+                GenTree*    lastNode   = blockRange.LastNode();
+                if (lastNode != node)
+                {
+                    blockRange.Delete(compiler, block, node->gtNext, lastNode);
+                }
+
+                break;
+            }
         } // end for each node in block
 
 #ifdef DEBUG
@@ -571,29 +596,32 @@ void CodeGen::genCodeForBBlist()
         // it up to date for vars that are not register candidates
         // (it would be nice to have a xor set function)
 
-        VARSET_TP mismatchLiveVars(VarSetOps::Diff(compiler, block->bbLiveOut, compiler->compCurLife));
-        VarSetOps::UnionD(compiler, mismatchLiveVars,
-                          VarSetOps::Diff(compiler, compiler->compCurLife, block->bbLiveOut));
-        VarSetOps::Iter mismatchLiveVarIter(compiler, mismatchLiveVars);
-        unsigned        mismatchLiveVarIndex  = 0;
-        bool            foundMismatchedRegVar = false;
-        while (mismatchLiveVarIter.NextElem(&mismatchLiveVarIndex))
+        if (!block->KindIs(BBJ_THROW))
         {
-            LclVarDsc* varDsc = compiler->lvaGetDescByTrackedIndex(mismatchLiveVarIndex);
-            if (varDsc->lvIsRegCandidate())
+            VARSET_TP mismatchLiveVars(VarSetOps::Diff(compiler, block->bbLiveOut, compiler->compCurLife));
+            VarSetOps::UnionD(compiler, mismatchLiveVars,
+                              VarSetOps::Diff(compiler, compiler->compCurLife, block->bbLiveOut));
+            VarSetOps::Iter mismatchLiveVarIter(compiler, mismatchLiveVars);
+            unsigned        mismatchLiveVarIndex  = 0;
+            bool            foundMismatchedRegVar = false;
+            while (mismatchLiveVarIter.NextElem(&mismatchLiveVarIndex))
             {
-                if (!foundMismatchedRegVar)
+                LclVarDsc* varDsc = compiler->lvaGetDescByTrackedIndex(mismatchLiveVarIndex);
+                if (varDsc->lvIsRegCandidate())
                 {
-                    JITDUMP("Mismatched live reg vars after " FMT_BB ":", block->bbNum);
-                    foundMismatchedRegVar = true;
+                    if (!foundMismatchedRegVar)
+                    {
+                        JITDUMP("Mismatched live reg vars after " FMT_BB ":", block->bbNum);
+                        foundMismatchedRegVar = true;
+                    }
+                    JITDUMP(" V%02u", compiler->lvaTrackedIndexToLclNum(mismatchLiveVarIndex));
                 }
-                JITDUMP(" V%02u", compiler->lvaTrackedIndexToLclNum(mismatchLiveVarIndex));
             }
-        }
-        if (foundMismatchedRegVar)
-        {
-            JITDUMP("\n");
-            assert(!"Found mismatched live reg var(s) after block");
+            if (foundMismatchedRegVar)
+            {
+                JITDUMP("\n");
+                assert(!"Found mismatched live reg var(s) after block");
+            }
         }
 #endif
 
@@ -695,41 +723,37 @@ void CodeGen::genCodeForBBlist()
                 break;
 
             case BBJ_THROW:
+#ifdef DEBUG
+            {
                 // If we have a throw at the end of a function or funclet, we need to emit another instruction
                 // afterwards to help the OS unwinder determine the correct context during unwind.
-                // We insert an unexecuted breakpoint instruction in several situations
-                // following a throw instruction:
+                // We need that in the following situations:
                 // 1. If the throw is the last instruction of the function or funclet. This helps
                 //    the OS unwinder determine the correct context during an unwind from the
                 //    thrown exception.
                 // 2. If this is this is the last block of the hot section.
                 // 3. If the subsequent block is a special throw block.
                 // 4. On AMD64, if the next block is in a different EH region.
-                if (block->IsLast() || block->Next()->HasFlag(BBF_FUNCLET_BEG) ||
-                    !BasicBlock::sameEHRegion(block, block->Next()) ||
-                    (!isFramePointerUsed() && compiler->fgIsThrowHlpBlk(block->Next())) ||
-                    block->IsLastHotBlock(compiler))
+                //
+                // The extra instruction is also needed to ensure that gc register liveness doesn't change
+                // across call instructions in fully-interruptible mode.
+                //
+                // Considering that the throwing code is typically rare, and is not on the common/fast path
+                // we just add the instruction after all throwing calls.
+                // We will test that invariant in the most common case - when the throw ends the block.
+                GenTree* last = block->lastNode();
+                if ((last != nullptr) && (last->gtOper == GT_CALL))
                 {
-                    instGen(INS_BREAKPOINT); // This should never get executed
-                }
-                // Do likewise for blocks that end in DOES_NOT_RETURN calls
-                // that were not caught by the above rules. This ensures that
-                // gc register liveness doesn't change across call instructions
-                // in fully-interruptible mode.
-                else
-                {
-                    GenTree* call = block->lastNode();
-
-                    if ((call != nullptr) && (call->gtOper == GT_CALL))
+                    GenTreeCall* call = last->AsCall();
+                    if (call->IsNoReturn())
                     {
-                        if (call->AsCall()->IsNoReturn())
-                        {
-                            instGen(INS_BREAKPOINT); // This should never get executed
-                        }
+                        assert(GetEmitter()->emitIsLastInsBreakpoint() || call->IsFastTailCall());
                     }
                 }
+            }
+#endif // DEBUG
 
-                break;
+            break;
 
             case BBJ_CALLFINALLY:
                 block = genCallFinally(block);
@@ -760,15 +784,6 @@ void CodeGen::genCodeForBBlist()
 
             case BBJ_ALWAYS:
             {
-#ifdef DEBUG
-                GenTree* call = block->lastNode();
-                if ((call != nullptr) && (call->gtOper == GT_CALL))
-                {
-                    // At this point, BBJ_ALWAYS should never end with a call that doesn't return.
-                    assert(!call->AsCall()->IsNoReturn());
-                }
-#endif // DEBUG
-
                 // If this block jumps to the next one, we might be able to skip emitting the jump
                 if (block->CanRemoveJumpToNext(compiler))
                 {

src/coreclr/jit/codegenloongarch64.cpp

@@ -6419,6 +6419,18 @@ void CodeGen::genCall(GenTreeCall* call)
     assert((gcInfo.gcRegByrefSetCur & killMask) == 0);
 #endif
 
+    if (call->IsNoReturn())
+    {
+        // There are several situations when we need to add another instruction
+        // after a throwing call to help the OS unwinder determine the correct context during unwind.
+        // It also ensures that the gc register liveness doesn't change across throwing call instructions
+        // in fully-interruptible mode.
+        instGen(INS_BREAKPOINT);
+
+        // nothing else needs to be emitted for this call
+        return;
+    }
+
     var_types returnType = call->TypeGet();
     if (returnType != TYP_VOID)
     {

src/coreclr/jit/codegenriscv64.cpp

@@ -6495,6 +6495,18 @@ void CodeGen::genCall(GenTreeCall* call)
     assert((gcInfo.gcRegByrefSetCur & killMask) == 0);
 #endif
 
+    if (call->IsNoReturn())
+    {
+        // There are several situations when we need to add another instruction
+        // after a throwing call to help the OS unwinder determine the correct context during unwind.
+        // It also ensures that the gc register liveness doesn't change across throwing call instructions
+        // in fully-interruptible mode.
+        instGen(INS_BREAKPOINT);
+
+        // nothing else needs to be emitted for this call
+        return;
+    }
+
     var_types returnType = call->TypeGet();
     if (returnType != TYP_VOID)
     {

src/coreclr/jit/codegenxarch.cpp

@@ -6045,6 +6045,30 @@ void CodeGen::genCall(GenTreeCall* call)
     assert((gcInfo.gcRegByrefSetCur & killMask) == 0);
 #endif
 
+    unsigned stackAdjustBias = 0;
+
+#if defined(TARGET_X86)
+    // Is the caller supposed to pop the arguments?
+    if (call->CallerPop() && (stackArgBytes != 0))
+    {
+        stackAdjustBias = stackArgBytes;
+    }
+
+    SubtractStackLevel(stackArgBytes);
+#endif // TARGET_X86
+
+    if (call->IsNoReturn())
+    {
+        // There are several situations when we need to add another instruction
+        // after a throwing call to help the OS unwinder determine the correct context during unwind.
+        // It also ensures that the gc register liveness doesn't change across throwing call instructions
+        // in fully-interruptible mode.
+        instGen(INS_BREAKPOINT);
+
+        // nothing else needs to be emitted for this call
+        return;
+    }
+
     var_types returnType = call->TypeGet();
     if (returnType != TYP_VOID)
     {
@@ -6169,18 +6193,6 @@ void CodeGen::genCall(GenTreeCall* call)
     }
 #endif // FEATURE_EH_WINDOWS_X86
 
-    unsigned stackAdjustBias = 0;
-
-#if defined(TARGET_X86)
-    // Is the caller supposed to pop the arguments?
-    if (call->CallerPop() && (stackArgBytes != 0))
-    {
-        stackAdjustBias = stackArgBytes;
-    }
-
-    SubtractStackLevel(stackArgBytes);
-#endif // TARGET_X86
-
     genRemoveAlignmentAfterCall(call, stackAdjustBias);
 }
 

src/coreclr/jit/emit.cpp

@@ -2569,6 +2569,17 @@ bool emitter::emitHasEpilogEnd()
 
 #endif // JIT32_GCENCODER
 
+// Is the last instruction emitted a breakpoint instruction?
+bool emitter::emitIsLastInsBreakpoint()
+{
+    if (emitHasLastIns() && (emitLastIns->idIns() == INS_BREAKPOINT))
+    {
+        return true;
+    }
+
+    return false;
+}
+
 #ifdef TARGET_XARCH
 
 /*****************************************************************************

src/coreclr/jit/emit.h

@@ -2757,6 +2757,9 @@ class emitter
         return (emitLastIns != nullptr);
     }
 
+    // Is the last instruction emitted a breakpoint instruction?
+    bool emitIsLastInsBreakpoint();
+
     // Checks to see if we can cross between the two given IG boundaries.
     //
     // We have the following checks: