[scan-build] fix logic error warnings emitted on llvm code base

[apelete]

Summary

Fix the last few "called C++ object pointer is null" warnings emitted
by Clang Static Analyzer on the following files:

• lib/Transforms/Scalar/LoopStrengthReduce.cpp,
• lib/Transforms/Scalar/LoadCombine.cpp,
• lib/Transforms/ObjCARC/ObjCARCContract.cpp,
• lib/Target/XCore/XCoreInstrInfo.cpp,
• lib/Target/PowerPC/PPCInstrInfo.cpp,
• lib/Target/NVPTX/NVPTXISelLowering.cpp,
• lib/CodeGen/CriticalAntiDepBreaker.cpp.

Signed-off-by: Apelete Seketeli <[e-mail]>

[jlebar]

NVPTX changes lgtm.

[apelete]

[scan-build] fix logic error warnings emitted on llvm code base

Changes since last revision:

• fast forward rebase on git master branch.

[apelete]

Waiting for review, could someone please have a look at this one ?

[atrick]

Adding these asserts seems harmless.

[apelete]

In D19632#418685, @atrick wrote:

Adding these asserts seems harmless.

Great, please go ahead and commit this for me as I don't have commit access.
Thanks.

[apelete]

[scan-build] fix logic error warnings emitted on llvm code base

Changes since last revision:

• lib/Target/XCore/XCoreInstrInfo.cpp: remove both assert and check against 'SecondLastInst' MachineInstr variable since function would have returned earlier if there was only one terminator.

[• friedgold]

Thanks for making the update, XCore changes look good to me.

[apelete]

[scan-build] fix logic error warnings emitted on llvm code base

Changes since last commit:

• rebased patch on trunk and validated against test suite.

[apelete]

In D19632#422884, @friedgold wrote:

Thanks for making the update, XCore changes look good to me.

I don't have commit access so please go ahead and commit this for me when you're ready.

[Eugene.Zelenko]

Looks like patch was not committed.

lib/CodeGen/CriticalAntiDepBreaker.cpp

for (unsigned i = 0, e = SUnits.size(); i != e; ++i) {
     const SUnit *SU = &SUnits[i];
     MISUnitMap[SU->getInstr()] = SU;
     if (!Max || SU->getDepth() + SU->Latency > Max->getDepth() + Max->Latency)
       Max = SU;
   }
 
 #ifndef NDEBUG
   {
+    assert(Max && "does not point to any node in the scheduling DAG");
     DEBUG(dbgs() << "Critical path has total latency "
           << (Max->getDepth() + Max->Latency) << "\n");
     DEBUG(dbgs() << "Available regs:");
     for (unsigned Reg = 0; Reg < TRI->getNumRegs(); ++Reg) {
       if (KillIndices[Reg] == ~0u)
         DEBUG(dbgs() << " " << TRI->getName(Reg));
     }
     DEBUG(dbgs() << '\n');
   }
 #endif
 
   // Track progress along the critical path through the SUnit graph as we walk
   // the instructions.
   const SUnit *CriticalPathSU = Max;
+  assert(CriticalPathSU && "does not point to any node in the scheduling DAG");
   MachineInstr *CriticalPathMI = CriticalPathSU->getInstr();
 
   // Consider this pattern:
   //   A = ...
   //   ... = A
   //   A = ...
   //   ... = A
   //   A = ...

lib/Target/NVPTX/NVPTXISelLowering.cpp

if (retTy && retTy->isVectorTy()) {
         bool needTruncate = false;
         if (VTs[i].isInteger() && sz < 8) {
           sz = 8;
           needTruncate = true;
         }
 
         SmallVector<EVT, 4> LoadRetVTs;
         EVT TheLoadType = VTs[i];
-        if (retTy->isIntegerTy() && DL.getTypeAllocSizeInBits(retTy) < 32) {
+        if (retTy && retTy->isIntegerTy() && DL.getTypeAllocSizeInBits(retTy) < 32) {
           // This is for integer types only, and specifically not for
           // aggregates.
           LoadRetVTs.push_back(MVT::i32);
           TheLoadType = MVT::i32;
           needTruncate = true;
         } else if (sz < 16) {
           // If loading i1/i8 result, generate
           //   load i8 (-> i16)

lib/Target/PowerPC/PPCInstrInfo.cpp

bool PPCInstrInfo::AnalyzeBranch(MachineBasicBlock &MBB,MachineBasicBlock *&TBB,
 
   // Get the instruction before it if it's a terminator.
   MachineInstr *SecondLastInst = I;
 
   // If there are three terminators, we don't know what sort of block this is.
   if (SecondLastInst && I != MBB.begin() && isUnpredicatedTerminator(*--I))
     return true;
 
+  assert(SecondLastInst && "does not point to a valid instruction");
   // If the block ends with PPC::B and PPC:BCC, handle it.
   if (SecondLastInst->getOpcode() == PPC::BCC &&
       LastInst->getOpcode() == PPC::B) {
     if (!SecondLastInst->getOperand(2).isMBB() ||
         !LastInst->getOperand(0).isMBB())
       return true;
     TBB =  SecondLastInst->getOperand(2).getMBB();
     Cond.push_back(SecondLastInst->getOperand(0));

lib/Target/XCore/XCoreInstrInfo.cpp

if (I == MBB.begin() || !isUnpredicatedTerminator(*--I)) {
     Cond.push_back(LastInst->getOperand(0));
     return false;
   }
   
   // Get the instruction before it if it's a terminator.
   MachineInstr *SecondLastInst = I;
 
   // If there are three terminators, we don't know what sort of block this is.
-  if (SecondLastInst && I != MBB.begin() && isUnpredicatedTerminator(*--I))
+  if (I != MBB.begin() && isUnpredicatedTerminator(*--I))
     return true;
-  
   unsigned SecondLastOpc    = SecondLastInst->getOpcode();
   XCore::CondCode BranchCode = GetCondFromBranchOpc(SecondLastOpc);
   
   // If the block ends with conditional branch followed by unconditional,
   // handle it.
   if (BranchCode != XCore::COND_INVALID
     && IsBRU(LastInst->getOpcode())) {
 

lib/Transforms/ObjCARC/ObjCARCContract.cpp

DEBUG(
                    << "            Retain:  " << *Retain << "\n"
                    << "            Load:    " << *Load << "\n");
 
   LLVMContext &C = Release->getContext();
   Type *I8X = PointerType::getUnqual(Type::getInt8Ty(C));
   Type *I8XX = PointerType::getUnqual(I8X);
 
   Value *Args[] = { Load->getPointerOperand(), New };
+  assert((Args[0] && Args[1]) && "is an empty Value array");
   if (Args[0]->getType() != I8XX)
     Args[0] = new BitCastInst(Args[0], I8XX, "", Store);
   if (Args[1]->getType() != I8X)
     Args[1] = new BitCastInst(Args[1], I8X, "", Store);
   Constant *Decl = EP.get(ARCRuntimeEntryPointKind::StoreStrong);
   CallInst *StoreStrong = CallInst::Create(Decl, Args, "", Store);
   StoreStrong->setDoesNotThrow();
   StoreStrong->setDebugLoc(Store->getDebugLoc());

lib/Transforms/Scalar/LoadCombine.cpp

DEBUG({
     for (const auto &L : Loads) {
       dbgs() << L.POP.Offset << ": " << *L.Load << "\n";
     }
   });
 
   // Find first load. This is where we put the new load.
   LoadPOPPair FirstLP;
   FirstLP.InsertOrder = -1u;
+  FirstLP.POP.Pointer = nullptr;
   for (const auto &L : Loads)
     if (L.InsertOrder < FirstLP.InsertOrder)
       FirstLP = L;
 
+  assert(FirstLP.POP.Pointer && "could not find first load");
   unsigned AddressSpace =
       FirstLP.POP.Pointer->getType()->getPointerAddressSpace();
 
   Builder->SetInsertPoint(FirstLP.Load);
   Value *Ptr = Builder->CreateConstGEP1_64(
       Builder->CreatePointerCast(Loads[0].POP.Pointer,
                                  Builder->getInt8PtrTy(AddressSpace)),
       Loads[0].POP.Offset);

lib/Transforms/Scalar/LoopStrengthReduce.cpp

while (IVOpIter != IVOpEnd) {
     }
     IVOpIter = findIVOperand(std::next(IVOpIter), IVOpEnd, L, SE);
   }
   if (IVOpIter == IVOpEnd) {
     // Gracefully give up on this chain.
     DEBUG(dbgs() << "Concealed chain head: " << *Head.UserInst << "\n");
     return;
   }
+  assert(IVSrc && "induction variable is needed to generate a chain");
 
   DEBUG(dbgs() << "Generate chain at: " << *IVSrc << "\n");
   Type *IVTy = IVSrc->getType();
   Type *IntTy = SE.getEffectiveSCEVType(IVTy);
   const SCEV *LeftOverExpr = nullptr;
   for (const IVInc &Inc : Chain) {
     Instruction *InsertPt = Inc.UserInst;
     if (isa<PHINode>(InsertPt))