[llvm] [BOLT] a local out-of-range stub might lead to infinite loop in LongJmp (PR #73918)

[Vladislav Khmelevsky via llvm-commits]

================
@@ -202,10 +202,31 @@ LongJmpPass::replaceTargetWithStub(BinaryBasicBlock &BB, MCInst &Inst,
     }
   } else if (LocalStubsIter != Stubs.end() &&
              LocalStubsIter->second.count(TgtBB)) {
-    // If we are replacing a local stub (because it is now out of range),
-    // use its target instead of creating a stub to jump to another stub
+    // The TgtBB and TgtSym now are the BB and label of an out-of-range
+    // local stub. We replace with the target of the local stub instead
+    // of creating a stub to jump to another stub.
+    // e.g.
+    // change the out-of-range stub
+    //    .Ltmp1:
+    //        ldr	x8, [x8, #0x10]
+    //        tbz	w0, #0x0, .LStub1111
+    //        ...
+    //    .LStub1111:
+    //        b	.Ltmp22
+    //  into the old target(target of the stub)
+    //    .Ltmp1:
+    //        ldr	x8, [x8, #0x10]
+    //        tbz	w0, #0x0, .Ltmp22
+    //        ...
+    //    .LStub1111:
+    //        b	.Ltmp22
     TgtSym = BC.MIB->getTargetSymbol(*TgtBB->begin());
-    TgtBB = BB.getSuccessor(TgtSym, BI);
+    BB.replaceSuccessor(TgtBB, TgtBB->getSuccessor(TgtSym, BI), BI.Count,
----------------
yota9 wrote:

TgtBB->getSuccessor(TgtSym, BI) might return NULL, since the veneer/stub "successor" might be the other function (e.g. function call was replaced by call to the veneer and jump from it). replaceSuccessor would fail in this case.  So replaceSuccessor logic and further execution count calculations must be call under "TgtBB->getSuccessor(TgtSym, BI)" condition. The last getsuccessor might be be called outside this condition, since NULL is expected return in such case. 

https://github.com/llvm/llvm-project/pull/73918