[Coverity] Fix explicit null dereferences

[akshaykhadse]

Summary

This change fixes static code analysis errors

[skan]

llvm/include/llvm/ADT/SparseSet.h

206

Sparse != nullptr -> Sparse

llvm/include/llvm/CodeGen/PBQP/CostAllocator.h

103

I think it's a UB due to the sequence issue https://en.wikipedia.org/wiki/Sequence_point , right?

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1138

Why is this function special?

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1276

MMI != nullptr -> MMI

llvm/lib/CodeGen/ScheduleDAG.cpp

728

Same here

[akshaykhadse]

llvm/include/llvm/ADT/SparseSet.h

206

"Sparse != nullptr" makes code more readable. Do you still want me to change this?

llvm/include/llvm/CodeGen/PBQP/CostAllocator.h

103

Yes. That's correct.

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

1138

This is not special. But, Coverity identified that this is where the dereference happens. So, potentially all the functions that call this one will lead to Coverity warning sooner or later.

[skan]

LGTM

[TWeaver]

Good morning from the UK,

My apologies but I've had to revert this patch as it caused the following two build bots to begin failing:

https://lab.llvm.org/buildbot/#/builders/216/builds/20322
https://lab.llvm.org/buildbot/#/builders/123/builds/18511

Warmest regards,
Tom W

llvm/include/llvm/ADT/SparseSet.h

public:
 
   /// findIndex - Find an element by its index.
   ///
   /// @param   Idx A valid index to find.
   /// @returns An iterator to the element identified by key, or end().
   ///
   iterator findIndex(unsigned Idx) {
     assert(Idx < Universe && "Key out of range");
+    assert(Sparse != nullptr && "Invalid sparse type");
     const unsigned Stride = std::numeric_limits<SparseT>::max() + 1u;
     for (unsigned i = Sparse[Idx], e = size(); i < e; i += Stride) {
       const unsigned FoundIdx = ValIndexOf(Dense[i]);
       assert(FoundIdx < Universe && "Invalid key in set. Did object mutate?");
       if (Idx == FoundIdx)
         return begin() + i;
       // Stride is 0 when SparseT >= unsigned.  We don't need to loop.
       if (!Stride)

llvm/include/llvm/CodeGen/PBQP/CostAllocator.h

public:
   template <typename ValueKeyT> PoolRef getValue(ValueKeyT ValueKey) {
     typename EntrySetT::iterator I = EntrySet.find_as(ValueKey);
 
     if (I != EntrySet.end())
       return PoolRef((*I)->shared_from_this(), &(*I)->getValue());
 
     auto P = std::make_shared<PoolEntry>(*this, std::move(ValueKey));
     EntrySet.insert(P.get());
-    return PoolRef(std::move(P), &P->getValue());
+    return PoolRef(P, &P->getValue());
   }
 };
 
 template <typename VectorT, typename MatrixT> class PoolCostAllocator {
 private:
   using VectorCostPool = ValuePool<VectorT>;
   using MatrixCostPool = ValuePool<MatrixT>;
 

llvm/include/llvm/CodeGen/SelectionDAGNodes.h

inline SDValue::SDValue(SDNode *node, unsigned resno)
   // callers that use SDValue(N, 0) with a deleted N to indicate successful
   // combines.
   assert((!Node || !ResNo || ResNo < Node->getNumValues()) &&
          "Invalid result number for the given node!");
   assert(ResNo < -2U && "Cannot use result numbers reserved for DenseMaps.");
 }
 
 inline unsigned SDValue::getOpcode() const {
+  assert(Node != nullptr && "Invalid Node");
   return Node->getOpcode();
 }
 
 inline EVT SDValue::getValueType() const {
   return Node->getValueType(ResNo);
 }
 
 inline unsigned SDValue::getNumOperands() const {

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

AsmPrinter::getFunctionCFISectionType(const Function &F) const {
   // Ignore functions that won't get emitted.
   if (F.isDeclarationForLinker())
     return CFISection::None;
 
   if (MAI->getExceptionHandlingType() == ExceptionHandling::DwarfCFI &&
       F.needsUnwindTableEntry())
     return CFISection::EH;
 
+  assert(MMI != nullptr && "Invalid machine module info");
   if (MMI->hasDebugInfo() || TM.Options.ForceDwarfFrameSection)
     return CFISection::Debug;
 
   return CFISection::None;
 }
 
 AsmPrinter::CFISection
 AsmPrinter::getFunctionCFISectionType(const MachineFunction &MF) const {

llvm/lib/CodeGen/ScheduleDAG.cpp

void ScheduleDAGTopologicalSort::AddSUnitWithoutPredecessors(const SUnit *SU) {
   assert(SU->NumPreds == 0 && "Can only add SU's with no predecessors");
   Node2Index.push_back(Index2Node.size());
   Index2Node.push_back(SU->NodeNum);
   Visited.resize(Node2Index.size());
 }
 
 bool ScheduleDAGTopologicalSort::IsReachable(const SUnit *SU,
                                              const SUnit *TargetSU) {
+  assert(TargetSU != nullptr && "Invalid target SUnit");
+  assert(SU != nullptr && "Invalid SUnit");
   FixOrder();
   // If insertion of the edge SU->TargetSU would create a cycle
   // then there is a path from TargetSU to SU.
   int UpperBound, LowerBound;
   LowerBound = Node2Index[TargetSU->NodeNum];
   UpperBound = Node2Index[SU->NodeNum];
   bool HasLoop = false;
   // Is Ord(TargetSU) < Ord(SU) ?

llvm/lib/CodeGen/ValueTypes.cpp

Type *EVT::getTypeForEVT(LLVMContext &Context) const {
   }
   // clang-format on
 }
 
 /// Return the value type corresponding to the specified type.  This returns all
 /// pointers as MVT::iPTR.  If HandleUnknown is true, unknown types are returned
 /// as Other, otherwise they are invalid.
 MVT MVT::getVT(Type *Ty, bool HandleUnknown){
+  assert(Ty != nullptr && "Invalid type");
   switch (Ty->getTypeID()) {
   default:
     if (HandleUnknown) return MVT(MVT::Other);
     llvm_unreachable("Unknown type!");
   case Type::VoidTyID:
     return MVT::isVoid;
   case Type::IntegerTyID:
     return getIntegerVT(cast<IntegerType>(Ty)->getBitWidth());

llvm/lib/IR/Mangler.cpp

static void addByteCountSuffix(raw_ostream &OS, const Function *F,
   }
 
   OS << '@' << ArgWords;
 }
 
 void Mangler::getNameWithPrefix(raw_ostream &OS, const GlobalValue *GV,
                                 bool CannotUsePrivateLabel) const {
   ManglerPrefixTy PrefixTy = Default;
+  assert(GV != nullptr && "Invalid Global Value");
   if (GV->hasPrivateLinkage()) {
     if (CannotUsePrivateLabel)
       PrefixTy = LinkerPrivate;
     else
       PrefixTy = Private;
   }
 
   const DataLayout &DL = GV->getParent()->getDataLayout();

llvm/lib/TableGen/Record.cpp

RecTy *llvm::resolveTypes(RecTy *T1, RecTy *T2) {
   if (T1 == T2)
     return T1;
 
   if (RecordRecTy *RecTy1 = dyn_cast<RecordRecTy>(T1)) {
     if (RecordRecTy *RecTy2 = dyn_cast<RecordRecTy>(T2))
       return resolveRecordTypes(RecTy1, RecTy2);
   }
 
+  assert(T1 != nullptr && "Invalid record type");
   if (T1->typeIsConvertibleTo(T2))
     return T2;
+
+  assert(T2 != nullptr && "Invalid record type");
   if (T2->typeIsConvertibleTo(T1))
     return T1;
 
   if (ListRecTy *ListTy1 = dyn_cast<ListRecTy>(T1)) {
     if (ListRecTy *ListTy2 = dyn_cast<ListRecTy>(T2)) {
       RecTy* NewType = resolveTypes(ListTy1->getElementType(),
                                     ListTy2->getElementType());
       if (NewType)

llvm/lib/Target/X86/X86InstrInfo.cpp

 static unsigned getLoadStoreRegOpcode(Register Reg,
                                       const TargetRegisterClass *RC,
                                       bool IsStackAligned,
                                       const X86Subtarget &STI, bool Load) {
   bool HasAVX = STI.hasAVX();
   bool HasAVX512 = STI.hasAVX512();
   bool HasVLX = STI.hasVLX();
 
+  assert(RC != nullptr && "Invalid target register class");
   switch (STI.getRegisterInfo()->getSpillSize(*RC)) {
   default:
     llvm_unreachable("Unknown spill size");
   case 1:
     assert(X86::GR8RegClass.hasSubClassEq(RC) && "Unknown 1-byte regclass");
     if (STI.is64Bit())
       // Copying to or from a physical H register on x86-64 requires a NOREX
       // move.  Otherwise use a normal move.

llvm/utils/TableGen/GlobalISelEmitter.cpp

if (Predicate.isStore()) {
     if (Predicate.isNonTruncStore()) {
       // We need to check the sizes match here otherwise we could incorrectly
       // match truncating stores with non-truncating ones.
       InsnMatcher.addPredicate<MemoryVsLLTSizePredicateMatcher>(
           0, MemoryVsLLTSizePredicateMatcher::EqualTo, 0);
     }
   }
 
+  assert(SrcGIEquivOrNull != nullptr && "Invalid SrcGIEquivOrNull value");
   // No check required. We already did it by swapping the opcode.
   if (!SrcGIEquivOrNull->isValueUnset("IfSignExtend") &&
       Predicate.isSignExtLoad())
     return InsnMatcher;
 
   // No check required. We already did it by swapping the opcode.
   if (!SrcGIEquivOrNull->isValueUnset("IfZeroExtend") &&
       Predicate.isZeroExtLoad())