[NFC] Fix potential dereferencing of null return value.

[schittir]

Summary

Replace getAs with castAs and add assert if needed.

[aaron.ballman]

LGTM with the think-o fixed.

clang/lib/Frontend/FrontendActions.cpp

461

This doesn't build. ;-)

[steakhal]

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

294–295

[schittir]

Fix the assert causing build failure and address another review comment.

clang/lib/Frontend/FrontendActions.cpp

461

Thanks for the catch. Something was wrong with my workspace build setup, and didn't diagnose this at all! I changed this in the new patch.

[schittir]

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

294–295

Wouldn't it be better to do an with a comment, like below?

assert(lstate && "lstate should not be null");

[steakhal]

Looks good. Thanks!

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

294–295

As a Static Analyzer dev I don't think its necessary. StateRefs are ubiquitous and here we probably know it cannot be null. And if it turns out to be null we would get a segfault. So that sense I don't think its necessary.

And speaking of a comment like "it should not be null" I think the segfault would sort of imply that.

[schittir]

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

294–295

That makes sense! Thanks!

clang/lib/AST/ASTContext.cpp

 
 /// getExtVectorType - Return the unique reference to an extended vector type of
 /// the specified element type and size. VectorType must be a built-in type.
 QualType ASTContext::getExtVectorType(QualType vecType,
                                       unsigned NumElts) const {
   assert(vecType->isBuiltinType() || vecType->isDependentType() ||
          (vecType->isBitIntType() &&
           // Only support _BitInt elements with byte-sized power of 2 NumBits.
-          llvm::isPowerOf2_32(vecType->getAs<BitIntType>()->getNumBits()) &&
-          vecType->getAs<BitIntType>()->getNumBits() >= 8));
+          llvm::isPowerOf2_32(vecType->castAs<BitIntType>()->getNumBits()) &&
+          vecType->castAs<BitIntType>()->getNumBits() >= 8));
 
   // Check if we've already instantiated a vector of this type.
   llvm::FoldingSetNodeID ID;
   VectorType::Profile(ID, vecType, NumElts, Type::ExtVector,
                       VectorType::GenericVector);
   void *InsertPos = nullptr;
   if (VectorType *VTP = VectorTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(VTP, 0);

clang/lib/Frontend/FrontendActions.cpp

if (const auto *Decl = dyn_cast<TagDecl>(NamedTemplate)) {
           Decl->getLocation().print(OS, TheSema.getSourceManager());
           return;
         }
       }
       OS << "unnamed " << Decl->getKindName();
       return;
     }
 
+    assert(NamedCtx, "NamedCtx cannot be null");
+
     if (const auto *Decl = dyn_cast<ParmVarDecl>(NamedTemplate)) {
       OS << "unnamed function parameter " << Decl->getFunctionScopeIndex()
          << " ";
       if (Decl->getFunctionScopeDepth() > 0)
         OS << "(at depth " << Decl->getFunctionScopeDepth() << ") ";
       OS << "of ";
       NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
       return;

clang/lib/Sema/SemaType.cpp

if ((!T->isDependentType() && !T->isIntegerType() &&
        !T->isRealFloatingType()) ||
       (IsNoBoolVecLang && T->isBooleanType())) {
     Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << T;
     return QualType();
   }
 
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
   if (T->isBitIntType()) {
-    unsigned NumBits = T->getAs<BitIntType>()->getNumBits();
+    unsigned NumBits = T->castAs<BitIntType>()->getNumBits();
     if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
       Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
           << (NumBits < 8);
       return QualType();
     }
   }
 
   if (!ArraySize->isTypeDependent() && !ArraySize->isValueDependent()) {

clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

 /// the ivar. Returns nullptr if the instance symbol cannot be found.
 SymbolRef
 ObjCDeallocChecker::getInstanceSymbolFromIvarSymbol(SymbolRef IvarSym) const {
 
   const ObjCIvarRegion *IvarRegion = getIvarRegionForIvarSymbol(IvarSym);
   if (!IvarRegion)
     return nullptr;
 
-  return IvarRegion->getSymbolicBase()->getSymbol();
+  const SymbolicRegion *SR = IvarRegion->getSymbolicBase();
+  assert(SR && "Symbolic base should not be nullptr");
+  return SR->getSymbol();
 }
 
 /// If we are in -dealloc or -dealloc is on the stack, handle the call if it is
 /// a release or a nilling-out property setter.
 void ObjCDeallocChecker::checkPreObjCMessage(
     const ObjCMethodCall &M, CheckerContext &C) const {
   // Only run if -dealloc is on the stack.
   SVal DeallocedInstance;

clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp

 // In PthreadSemantics, pthread_mutex_destroy() returns zero if the lock is
 // successfully destroyed and it returns a non-zero value otherwise.
 ProgramStateRef PthreadLockChecker::resolvePossiblyDestroyedMutex(
     ProgramStateRef state, const MemRegion *lockR, const SymbolRef *sym) const {
   const LockState *lstate = state->get<LockMap>(lockR);
   // Existence in DestroyRetVal ensures existence in LockMap.
   // Existence in Destroyed also ensures that the lock state for lockR is either
   // UntouchedAndPossiblyDestroyed or UnlockedAndPossiblyDestroyed.
-  assert(lstate->isUntouchedAndPossiblyDestroyed() ||
-         lstate->isUnlockedAndPossiblyDestroyed());
+  assert(lstate && (lstate->isUntouchedAndPossiblyDestroyed() ||
+                    lstate->isUnlockedAndPossiblyDestroyed()));
 
   ConstraintManager &CMgr = state->getConstraintManager();
   ConditionTruthVal retZero = CMgr.isNull(state, *sym);
   if (retZero.isConstrainedFalse()) {
     if (lstate->isUntouchedAndPossiblyDestroyed())
       state = state->remove<LockMap>(lockR);
     else if (lstate->isUnlockedAndPossiblyDestroyed())
       state = state->set<LockMap>(lockR, LockState::getUnlocked());