Fix splitBasicBlock API

[sabuasal]

Summary

Avoid dereferencing a sentinel node in splitBasicBlock while updating Phi nodes in successor basic blocks. This is needed after r279314 added an assertion to guard against that. The for loop updated in this patch can possibly deference a pointer which could be pointing to a Sentinel and hence hitting the assertion.

[efriedma]

In the future, please make sure you add llvm-commits as a subscriber when you create the revision, not afterwards. If you do it the wrong way around, the patch itself is never sent to llvm-commits.

[mehdi_amini]

This could happen only if the block is empty, right?

[MatzeB]

LLVM language ref says: "Each basic block may optionally start with a label (giving the basic block a symbol table entry), contains a list of instructions, and ends with a terminator instruction (such as a branch or function return)." So I would expect a terminator after the last PHI instruction or is this used in some intermediate steps where that rule is temporarily broken?

[sabuasal]

What if it is a basic block that only has a terminator instruction without any Phi nodes? is that an allowed basic block in LLVM?

[MatzeB]

In D25757#578003, @sabuasal wrote:

What if it is a basic block that only has a terminator instruction without any Phi nodes? is that an allowed basic block in LLVM?

That is fine, but then this loop should have no problem because that terminator will return nullptr for dyn_cast<PHINode>.

lib/IR/BasicBlock.cpp

BasicBlock *BasicBlock::splitBasicBlock(iterator I, const Twine &BBName) {
   // successors.  If there were PHI nodes in the successors, then they need to
   // know that incoming branches will be from New, not from Old.
   //
   for (succ_iterator I = succ_begin(New), E = succ_end(New); I != E; ++I) {
     // Loop over any phi nodes in the basic block, updating the BB field of
     // incoming values...
     BasicBlock *Successor = *I;
     PHINode *PN;
-    for (BasicBlock::iterator II = Successor->begin();
-         (PN = dyn_cast<PHINode>(II)); ++II) {
+    for (BasicBlock::iterator II = Successor->begin(), E = Successor->end();
+         II != E && (PN = dyn_cast<PHINode>(II)); ++II) {
       int IDX = PN->getBasicBlockIndex(this);
       while (IDX != -1) {
         PN->setIncomingBlock((unsigned)IDX, New);
         IDX = PN->getBasicBlockIndex(this);
       }
     }
   }
   return New;