[analyzer] Fix analyzer warnings.

[NoQ]

Summary

They ain't gonna fix themselves... well, not yet.

This is an outcome for this summer's GSoC! - http://lists.llvm.org/pipermail/cfe-dev/2019-August/063195.html

Found one real crash, added a couple tests:

Addressed 17 style issues that aren't real crashes - mostly misuses of dyn_cast / getAs (i.e., "if the dyn_ part is really necessary here, then you crash with a null dereference a few lines below because you didn't check the result"). These changes indeed made the code cleaner and shorter.

Suppressed 4 false positives; i found these assertions/refactorings nice anyway, except one (which i'd prefer to have as a post-condition on a certain function but we didn't invent post-conditions yet in C++).

That leaves us with only one remaining false positive that isn't really actionable; i'll have to investigate.

[NoQ]

Use auto in one more place.

[Szelethus]

if the dyn_ part is really necessary here, then you crash with a null dereference a few lines below because you didn't check the result

Hmm, can we detect things like this?:

if (isa<FunctionDecl>(D)) {
  const auto *FD = dyn_cast(D); // warn: D is known to be a FunctionDecl, prefer using cast<>
  // ...
}

[NoQ]

In D66847#1648209, @Szelethus wrote:

if the dyn_ part is really necessary here, then you crash with a null dereference a few lines below because you didn't check the result

Hmm, can we detect things like this?:

if (isa<FunctionDecl>(D)) {
  const auto *FD = dyn_cast(D); // warn: D is known to be a FunctionDecl, prefer using cast<>
  // ...
}

We should be detecting things like this as of D66325 and D66423 (we remember which casts were successful and don't re-assume them again), but LocalizationChecker.cpp turned out to have a false positive of this kind. @Charusso, you may want to take a look.

clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

885–888

Note: this was a false positive.

[NoQ]

I mean, such code should indeed be refactored to query the custom RTTI only once, but given that it's locally obvious that there's no null dereference in this code, we should not be emitting a warning on it. Or, at least, emitting a different kind of warning.

[Szelethus]

Another common mistake is this:

if (A) {
  const auto *B = dyn_cast_or_null<FunctionDecl>(A); // warn: A is known to be non-null, prefer dyn_cast
}

[NoQ]

In D66847#1648209, @Szelethus wrote:

if (isa<FunctionDecl>(D)) {
  const auto *FD = dyn_cast<FunctionDecl>(D); // warn: D is known to be a FunctionDecl, prefer using cast<>
  // ...
}

In D66847#1648339, @Szelethus wrote:

if (A) {
  const auto *B = dyn_cast_or_null<FunctionDecl>(A); // warn: A is known to be non-null, prefer dyn_cast
}

Both of these are must-problems. They are hard to solve via symbolic execution because all execution paths need to be taken into account simultaneously. Eg.:

if (isa<FunctionDecl>(D)) {
  if (coin)
    D = getDecl(); // Maybe not a FunctionDecl.

  // It's not enough to know that D is a FunctionDecl assuming coin is false.
  const auto *FD = dyn_cast<FunctionDecl>(D); // no-warning:
}

We could maybe make a syntactic checker or a data flow checker to find these anti-patterns. @Charusso also had an idea for a clang-tidy checker that emits fixits for code like dyn_cast<T>(x)->foo() ("use 'cast' instead").

[NoQ]

Filed the remaining false positive back on myself as https://bugs.llvm.org/show_bug.cgi?id=43135.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h

public:
   ///
   /// When this region represents a subexpression, the method is for printing
   /// an expression containing it.
   virtual void printPrettyAsExpr(raw_ostream &os) const;
 
   Kind getKind() const { return kind; }
 
   template<typename RegionTy> const RegionTy* getAs() const;
+  template<typename RegionTy> const RegionTy* castAs() const;
 
   virtual bool isBoundable() const { return false; }
 
   /// Get descriptive name for memory region. The name is obtained from
   /// the variable/field declaration retrieved from the memory region.
   /// Regions that point to an element of an array are returned as: "arr[0]".
   /// Regions that point to a struct are returned as: "st.var".
   //

 template<typename RegionTy>
 const RegionTy* MemRegion::getAs() const {
   if (const auto *RT = dyn_cast<RegionTy>(this))
     return RT;
 
   return nullptr;
 }
 
+template<typename RegionTy>
+const RegionTy* MemRegion::castAs() const {
+  return cast<RegionTy>(this);
+}
+
 //===----------------------------------------------------------------------===//
 // MemRegionManager - Factory object for creating regions.
 //===----------------------------------------------------------------------===//
 
 class MemRegionManager {
   ASTContext &C;
   llvm::BumpPtrAllocator& A;
   llvm::FoldingSet<MemRegion> Regions;

clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp

 }
 
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
     return false;
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
-  const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
-  const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
+  const auto *DstArgDRE = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
+  const auto *LenArgDRE =
+      dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
   uint64_t DstOff = 0;
   if (isSizeof(LenArg, DstArg))
     return false;
+
   // - size_t dstlen = sizeof(dst)
-  if (LenArgDecl) {
-    const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl());
+  if (LenArgDRE) {
+    const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDRE->getDecl());
+    // If it's an EnumConstantDecl instead, then we're missing out on something.
+    if (!LenArgVal) {
+      assert(isa<EnumConstantDecl>(LenArgDRE->getDecl()));
+      return false;
+    }
     if (LenArgVal->getInit())
       LenArg = LenArgVal->getInit();
   }
 
   // - integral value
   // We try to figure out if the last argument is possibly longer
   // than the destination can possibly handle if its size can be defined.
   if (const auto *IL = dyn_cast<IntegerLiteral>(LenArg->IgnoreParenImpCasts())) {
     uint64_t ILRawVal = IL->getValue().getZExtValue();
 
     // Case when there is pointer arithmetic on the destination buffer
     // especially when we offset from the base decreasing the
     // buffer length accordingly.
-    if (!DstArgDecl) {
-      if (const auto *BE = dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) {
-        DstArgDecl = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts());
+    if (!DstArgDRE) {
+      if (const auto *BE =
+              dyn_cast<BinaryOperator>(DstArg->IgnoreParenImpCasts())) {
+        DstArgDRE = dyn_cast<DeclRefExpr>(BE->getLHS()->IgnoreParenImpCasts());
         if (BE->getOpcode() == BO_Add) {
           if ((IL = dyn_cast<IntegerLiteral>(BE->getRHS()->IgnoreParenImpCasts()))) {
             DstOff = IL->getValue().getZExtValue();
           }
         }
       }
     }
-    if (DstArgDecl) {
-      if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) {
+    if (DstArgDRE) {
+      if (const auto *Buffer =
+              dyn_cast<ConstantArrayType>(DstArgDRE->getType())) {
         ASTContext &C = BR.getContext();
         uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
         auto RemainingBufferLen = BufferLen - DstOff;
         if (RemainingBufferLen < ILRawVal)
           return true;
       }
     }
   }

clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

 }
 
 //===----------------------------------------------------------------------===//
 // Check: floating point variable used as loop counter.
 // Originally: <rdar://problem/6336718>
 // Implements: CERT security coding advisory FLP-30.
 //===----------------------------------------------------------------------===//
 
+// Returns either 'x' or 'y', depending on which one of them is incremented
+// in 'expr', or nullptr if none of them is incremented.
 static const DeclRefExpr*
 getIncrementedVar(const Expr *expr, const VarDecl *x, const VarDecl *y) {
   expr = expr->IgnoreParenCasts();
 
   if (const BinaryOperator *B = dyn_cast<BinaryOperator>(expr)) {
     if (!(B->isAssignmentOp() || B->isCompoundAssignmentOp() ||
           B->getOpcode() == BO_Comma))
       return nullptr;
void WalkAST::checkLoopConditionForFloat(const ForStmt *FS) {
   const VarDecl *vdLHS = drLHS ? dyn_cast<VarDecl>(drLHS->getDecl()) : nullptr;
   const VarDecl *vdRHS = drRHS ? dyn_cast<VarDecl>(drRHS->getDecl()) : nullptr;
 
   if (!vdLHS && !vdRHS)
     return;
 
   // Does either variable appear in increment?
   const DeclRefExpr *drInc = getIncrementedVar(increment, vdLHS, vdRHS);
-
   if (!drInc)
     return;
 
+  const VarDecl *vdInc = cast<VarDecl>(drInc->getDecl());
+  assert(vdInc && (vdInc == vdLHS || vdInc == vdRHS));
+
   // Emit the error.  First figure out which DeclRefExpr in the condition
   // referenced the compared variable.
-  assert(drInc->getDecl());
-  const DeclRefExpr *drCond = vdLHS == drInc->getDecl() ? drLHS : drRHS;
+  const DeclRefExpr *drCond = vdLHS == vdInc ? drLHS : drRHS;
 
   SmallVector<SourceRange, 2> ranges;
   SmallString<256> sbuf;
   llvm::raw_svector_ostream os(sbuf);
 
   os << "Variable '" << drCond->getDecl()->getName()
      << "' with floating point type '" << drCond->getType().getAsString()
      << "' should not be used as a loop counter";

clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

static const ObjCObjectPointerType *getMostInformativeDerivedClassImpl(
   if (To->getObjectType()->getSuperClassType().isNull()) {
     // If To has no super class and From and To aren't the same then
     // To was not actually a descendent of From. In this case the best we can
     // do is 'From'.
     return From;
   }
 
   const auto *SuperOfTo =
-      To->getObjectType()->getSuperClassType()->getAs<ObjCObjectType>();
+      To->getObjectType()->getSuperClassType()->castAs<ObjCObjectType>();
   assert(SuperOfTo);
   QualType SuperPtrOfToQual =
       C.getObjCObjectPointerType(QualType(SuperOfTo, 0));
-  const auto *SuperPtrOfTo = SuperPtrOfToQual->getAs<ObjCObjectPointerType>();
+  const auto *SuperPtrOfTo = SuperPtrOfToQual->castAs<ObjCObjectPointerType>();
   if (To->isUnspecialized())
     return getMostInformativeDerivedClassImpl(From, SuperPtrOfTo, SuperPtrOfTo,
                                               C);
   else
     return getMostInformativeDerivedClassImpl(From, SuperPtrOfTo,
                                               MostInformativeCandidate, C);
 }
 
void DynamicTypePropagation::checkPostObjCMessage(const ObjCMethodCall &M,
   Selector Sel = MessageExpr->getSelector();
   ProgramStateRef State = C.getState();
   // Inference for class variables.
   // We are only interested in cases where the class method is invoked on a
   // class. This method is provided by the runtime and available on all classes.
   if (MessageExpr->getReceiverKind() == ObjCMessageExpr::Class &&
       Sel.getAsString() == "class") {
     QualType ReceiverType = MessageExpr->getClassReceiver();
-    const auto *ReceiverClassType = ReceiverType->getAs<ObjCObjectType>();
+    const auto *ReceiverClassType = ReceiverType->castAs<ObjCObjectType>();
+    if (!ReceiverClassType->isSpecialized())
+      return;
+
     QualType ReceiverClassPointerType =
         C.getASTContext().getObjCObjectPointerType(
             QualType(ReceiverClassType, 0));
-
-    if (!ReceiverClassType->isSpecialized())
-      return;
     const auto *InferredType =
-        ReceiverClassPointerType->getAs<ObjCObjectPointerType>();
-    assert(InferredType);
+        ReceiverClassPointerType->castAs<ObjCObjectPointerType>();
 
     State = State->set<MostSpecializedTypeArgsMap>(RetSym, InferredType);
     C.addTransition(State);
     return;
   }
 
   // Tracking for return types.
   SymbolRef RecSym = M.getReceiverSVal().getAsSymbol();

clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

void IteratorChecker::checkPostCall(const CallEvent &Call,
   // Record new iterator positions and iterator position changes
   const auto *Func = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
   if (!Func)
     return;
 
   if (Func->isOverloadedOperator()) {
     const auto Op = Func->getOverloadedOperator();
     if (isAssignmentOperator(Op)) {
-      const auto *InstCall = dyn_cast<CXXInstanceCall>(&Call);
+      // Overloaded 'operator=' must be a non-static member function.
+      const auto *InstCall = cast<CXXInstanceCall>(&Call);
       if (cast<CXXMethodDecl>(Func)->isMoveAssignmentOperator()) {
         handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
                      Call.getArgSVal(0));
         return;
       }
 
       handleAssign(C, InstCall->getCXXThisVal());
       return;

clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

void NonLocalizedStringChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
 
   if (isNonLocalized) {
     reportLocalizationError(svTitle, msg, C, argumentNumber + 1);
   }
 }
 
 void NonLocalizedStringChecker::checkPreCall(const CallEvent &Call,
                                              CheckerContext &C) const {
-  const Decl *D = Call.getDecl();
-  if (D && isa<FunctionDecl>(D)) {
-    const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
-    auto formals = FD->parameters();
-    for (unsigned i = 0,
-                  ei = std::min(unsigned(formals.size()), Call.getNumArgs());
-         i != ei; ++i) {
-      if (isAnnotatedAsTakingLocalized(formals[i])) {
-        auto actual = Call.getArgSVal(i);
-        if (hasNonLocalizedState(actual, C)) {
-          reportLocalizationError(actual, Call, C, i + 1);
-        }
-      }
-    }
-  }
-}
+  const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+  if (!FD)
+    return;
+
+  for (unsigned i = 0, ei = std::min(static_cast<unsigned>(formals.size()),
+                                     Call.getNumArgs()); i != ei; ++i) {
 
 static inline bool isNSStringType(QualType T, ASTContext &Ctx) {
 
   const ObjCObjectPointerType *PT = T->getAs<ObjCObjectPointerType>();
   if (!PT)
     return false;
 
   ObjCInterfaceDecl *Cls = PT->getObjectType()->getInterface();

clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

 
 PathDiagnosticPieceRef MPIBugReporter::RequestNodeVisitor::VisitNode(
     const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) {
 
   if (IsNodeFound)
     return nullptr;
 
   const Request *const Req = N->getState()->get<RequestMap>(RequestRegion);
+  assert(Req && "The region must be tracked and alive, given that we've "
+                "just emitted a report against it");
   const Request *const PrevReq =
       N->getFirstPred()->getState()->get<RequestMap>(RequestRegion);
 
   // Check if request was previously unused or in a different state.
-  if ((Req && !PrevReq) || (Req->CurrentState != PrevReq->CurrentState)) {
+  if (!PrevReq || (Req->CurrentState != PrevReq->CurrentState)) {
     IsNodeFound = true;
 
     ProgramPoint P = N->getFirstPred()->getLocation();
     PathDiagnosticLocation L =
         PathDiagnosticLocation::create(P, BRC.getSourceManager());
 
     return std::make_shared<PathDiagnosticEventPiece>(L, ErrorText);
   }
 
   return nullptr;
 }
 
 } // end of namespace: mpi
 } // end of namespace: ento
 } // end of namespace: clang

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C,
   SVal ElementCount;
   const SubRegion *Region;
   if (NE->isArray()) {
     const Expr *SizeExpr = *NE->getArraySize();
     ElementCount = C.getSVal(SizeExpr);
     // Store the extent size for the (symbolic)region
     // containing the elements.
     Region = Target.getAsRegion()
-                 ->getAs<SubRegion>()
+                 ->castAs<SubRegion>()
                  ->StripCasts()
-                 ->getAs<SubRegion>();
+                 ->castAs<SubRegion>();
   } else {
     ElementCount = svalBuilder.makeIntVal(1, true);
-    Region = Target.getAsRegion()->getAs<SubRegion>();
+    Region = Target.getAsRegion()->castAs<SubRegion>();
   }
-  assert(Region);
 
   // Set the region's extent equal to the Size in Bytes.
   QualType ElementType = NE->getAllocatedType();
   ASTContext &AstContext = C.getASTContext();
   CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
 
   if (ElementCount.getAs<NonLoc>()) {
     DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder);
if (!statePrev->get<RegionState>(FailedReallocSymbol)) {
       Msg = "Attempt to reallocate memory";
       StackHint = new StackHintGeneratorForSymbol(Sym,
                                                  "Returned reallocated memory");
       FailedReallocSymbol = nullptr;
       Mode = Normal;
     }
   }
 
-  if (Msg.empty())
+  if (Msg.empty()) {
+    // Silence a memory leak warning by MallocChecker in MallocChecker.cpp :)
+    assert(!StackHint && "Memory leak!");
     return nullptr;
+  }
+
   assert(StackHint);
 
   // Generate the extra diagnostic.
   PathDiagnosticLocation Pos;
   if (!S) {
     assert(RS->getAllocationFamily() == AF_InnerBuffer);
     auto PostImplCall = N->getLocation().getAs<PostImplicitCall>();
     if (!PostImplCall)

clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp

void checkASTCodeBody(const Decl *D, AnalysisManager& mgr,
     AnalysisDeclContext *ADC = mgr.getAnalysisDeclContext(D);
     CastedAllocFinder Finder(&BR.getContext());
     Finder.Visit(D->getBody());
     for (CastedAllocFinder::CallVec::iterator i = Finder.Calls.begin(),
          e = Finder.Calls.end(); i != e; ++i) {
       QualType CastedType = i->CastedExpr->getType();
       if (!CastedType->isPointerType())
         continue;
-      QualType PointeeType = CastedType->getAs<PointerType>()->getPointeeType();
+      QualType PointeeType = CastedType->getPointeeType();
       if (PointeeType->isVoidType())
         continue;
 
       for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
            ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
         if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType())
           continue;
 

clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp

 /// In case there is a derived to base cast above the array element, the
 /// Polymorphic output value is set to true. AKind output value is set to the
 /// allocation kind of the inspected region.
 const MemRegion *PointerArithChecker::getArrayRegion(const MemRegion *Region,
                                                      bool &Polymorphic,
                                                      AllocKind &AKind,
                                                      CheckerContext &C) const {
   assert(Region);
-  while (Region->getKind() == MemRegion::Kind::CXXBaseObjectRegionKind) {
-    Region = Region->getAs<CXXBaseObjectRegion>()->getSuperRegion();
+  while (const auto *BaseRegion = dyn_cast<CXXBaseObjectRegion>(Region)) {
+    Region = BaseRegion->getSuperRegion();
     Polymorphic = true;
   }
-  if (Region->getKind() == MemRegion::Kind::ElementRegionKind) {
-    Region = Region->getAs<ElementRegion>()->getSuperRegion();
+  if (const auto *ElemRegion = dyn_cast<ElementRegion>(Region)) {
+    Region = ElemRegion->getSuperRegion();
   }
 
   ProgramStateRef State = C.getState();
   if (const AllocKind *Kind = State->get<RegionState>(Region)) {
     AKind = *Kind;
     if (*Kind == AllocKind::Array)
       return Region;
     else
       return nullptr;
   }
   // When the region is symbolic and we do not have any information about it,
   // assume that this is an array to avoid false positives.
-  if (Region->getKind() == MemRegion::Kind::SymbolicRegionKind)
+  if (isa<SymbolicRegion>(Region))
     return Region;
 
   // No AllocKind stored and not symbolic, assume that it points to a single
   // object.
   return nullptr;
 }
 
 void PointerArithChecker::reportPointerArithMisuse(const Expr *E,

clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp

if (const BinaryOperator *B = dyn_cast<BinaryOperator>(StoreE)) {
         }
       }
 
       ex = B->getRHS();
       break;
     }
 
     if (const DeclStmt *DS = dyn_cast<DeclStmt>(StoreE)) {
-      const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
+      const VarDecl *VD = cast<VarDecl>(DS->getSingleDecl());
       ex = VD->getInit();
     }
 
     if (const auto *CD =
             dyn_cast<CXXConstructorDecl>(C.getStackFrame()->getDecl())) {
       if (CD->isImplicit()) {
         for (auto I : CD->inits()) {
           if (I->getInit()->IgnoreImpCasts() == StoreE) {

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

while (const MemRegion *Tmp = State->getSVal(R, DynT).getAsRegion()) {
 
     DynT = R->getLocationType();
     // In order to ensure that this loop terminates, we're also checking the
     // dynamic type of R, since type hierarchy is finite.
     if (isDereferencableType(DynT->getPointeeType()))
       break;
   }
 
-  while (R->getAs<CXXBaseObjectRegion>()) {
+  while (isa<CXXBaseObjectRegion>(R)) {
     NeedsCastBack = true;
-
-    if (!isa<TypedValueRegion>(R->getSuperRegion()))
+    const auto *SuperR = dyn_cast<TypedValueRegion>(R->getSuperRegion());
+    if (!SuperR)
       break;
-    R = R->getSuperRegion()->getAs<TypedValueRegion>();
+
+    R = SuperR;
   }
 
   return DereferenceInfo{R, NeedsCastBack, /*IsCyclic*/ false};
 }
 
 static bool isVoidPointer(QualType T) {
   while (!T.isNull()) {
     if (T->isVoidPointerType())
       return true;
     T = T->getPointeeType();
   }
   return false;
 }

clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp

void VirtualCallChecker::checkPreCall(const CallEvent &Call,
   if (!MC)
     return;
 
   const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(Call.getDecl());
   if (!MD)
     return;
 
   ProgramStateRef State = C.getState();
-  const CallExpr *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  // Member calls are always represented by a call-expression.
+  const auto *CE = cast<CallExpr>(Call.getOriginExpr());
   if (!isVirtualCall(CE))
     return;
 
   const MemRegion *Reg = MC->getCXXThisVal().getAsRegion();
   const ObjectState *ObState = State->get<CtorDtorMap>(Reg);
   if (!ObState)
     return;
 

clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp

if (Assign->isAssignmentOp()) {
       // Ordinary assignment
       RHS = Assign->getRHS();
       if (auto DE = dyn_cast_or_null<DeclRefExpr>(Assign->getLHS()))
         VD = dyn_cast_or_null<VarDecl>(DE->getDecl());
     }
   } else if (auto PD = dyn_cast_or_null<DeclStmt>(S)) {
     // Initialization
     assert(PD->isSingleDecl() && "We process decls one by one");
-    VD = dyn_cast_or_null<VarDecl>(PD->getSingleDecl());
+    VD = cast<VarDecl>(PD->getSingleDecl());
     RHS = VD->getAnyInitializer();
   }
 
   return std::make_pair(VD, RHS);
 }
 
 Nullability getNullabilityAnnotation(QualType Type) {
   const auto *AttrType = Type->getAs<AttributedType>();

clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp

 void ExprEngine::
 VisitOffsetOfExpr(const OffsetOfExpr *OOE,
                   ExplodedNode *Pred, ExplodedNodeSet &Dst) {
   StmtNodeBuilder B(Pred, Dst, *currBldrCtx);
   Expr::EvalResult Result;
   if (OOE->EvaluateAsInt(Result, getContext())) {
     APSInt IV = Result.Val.getInt();
     assert(IV.getBitWidth() == getContext().getTypeSize(OOE->getType()));
-    assert(OOE->getType()->isBuiltinType());
-    assert(OOE->getType()->getAs<BuiltinType>()->isInteger());
+    assert(OOE->getType()->castAs<BuiltinType>()->isInteger());
     assert(IV.isSigned() == OOE->getType()->isSignedIntegerType());
     SVal X = svalBuilder.makeIntVal(IV);
     B.generateNode(OOE, Pred,
                    Pred->getState()->BindExpr(OOE, Pred->getLocationContext(),
                                               X));
   }
   // FIXME: Handle the case where __builtin_offsetof is not a constant.
 }

clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
 
   StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
 
   SVal Result = symVal;
 
   if (CNE->isArray()) {
     // FIXME: allocating an array requires simulating the constructors.
     // For now, just return a symbolicated region.
-    if (const SubRegion *NewReg =
-            dyn_cast_or_null<SubRegion>(symVal.getAsRegion())) {
-      QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
+    if (const auto *NewReg = cast_or_null<SubRegion>(symVal.getAsRegion())) {
+      QualType ObjTy = CNE->getType()->getPointeeType();
       const ElementRegion *EleReg =
           getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
       Result = loc::MemRegionVal(EleReg);
     }
     State = State->BindExpr(CNE, Pred->getLocationContext(), Result);
     Bldr.generateNode(CNE, Pred, State);
     return;
   }

clang/lib/StaticAnalyzer/Core/MemRegion.cpp

static bool isValidBaseClass(const CXXRecordDecl *BaseClass,
   return false;
 }
 
 const CXXBaseObjectRegion *
 MemRegionManager::getCXXBaseObjectRegion(const CXXRecordDecl *RD,
                                          const SubRegion *Super,
                                          bool IsVirtual) {
   if (isa<TypedValueRegion>(Super)) {
-    assert(isValidBaseClass(RD, dyn_cast<TypedValueRegion>(Super), IsVirtual));
+    assert(isValidBaseClass(RD, cast<TypedValueRegion>(Super), IsVirtual));
     (void)&isValidBaseClass;
 
     if (IsVirtual) {
       // Virtual base regions should not be layered, since the layout rules
       // are different.
       while (const auto *Base = dyn_cast<CXXBaseObjectRegion>(Super))
         Super = cast<SubRegion>(Base->getSuperRegion());
       assert(Super && !isa<MemSpaceRegion>(Super));
case MemRegion::ElementRegionKind: {
         // We cannot compute offset for non-concrete index.
         SymbolicOffsetBase = R;
       }
       break;
     }
     case MemRegion::FieldRegionKind: {
       const auto *FR = cast<FieldRegion>(R);
       R = FR->getSuperRegion();
+      assert(R);
 
       const RecordDecl *RD = FR->getDecl()->getParent();
       if (RD->isUnion() || !RD->isCompleteDefinition()) {
         // We cannot compute offset for incomplete type.
         // For unions, we could treat everything as offset 0, but we'd rather
         // treat each field as a symbolic offset so they aren't stored on top
         // of each other, since we depend on things in typed regions actually
         // matching their types.

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

RegionStoreManager::bindArray(RegionBindingsConstRef B,
 
   return NewB;
 }
 
 RegionBindingsRef RegionStoreManager::bindVector(RegionBindingsConstRef B,
                                                  const TypedValueRegion* R,
                                                  SVal V) {
   QualType T = R->getValueType();
-  assert(T->isVectorType());
-  const VectorType *VT = T->getAs<VectorType>(); // Use getAs for typedefs.
+  const VectorType *VT = T->castAs<VectorType>(); // Use castAs for typedefs.
 
   // Handle lazy compound values and symbolic values.
   if (V.getAs<nonloc::LazyCompoundVal>() || V.getAs<nonloc::SymbolVal>())
     return bindAggregate(B, R, V);
 
   // We may get non-CompoundVal accidentally due to imprecise cast logic or
   // that we are binding symbolic struct value. Kill the field values, and if
   // the value is symbolic go and bind it as a "default" binding.
RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B,
                                                  const TypedValueRegion* R,
                                                  SVal V) {
   if (!Features.supportsFields())
     return B;
 
   QualType T = R->getValueType();
   assert(T->isStructureOrClassType());
 
-  const RecordType* RT = T->getAs<RecordType>();
+  const RecordType* RT = T->castAs<RecordType>();
   const RecordDecl *RD = RT->getDecl();
 
   if (!RD->isCompleteDefinition())
     return B;
 
   // Handle lazy compound values and symbolic values.
   if (Optional<nonloc::LazyCompoundVal> LCV =
         V.getAs<nonloc::LazyCompoundVal>()) {

clang/test/Analysis/cstring-syntax-weird.c

+// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
+// RUN:                    -verify %s
+
+// expected-no-diagnostics
+
+typedef __SIZE_TYPE__ size_t;
+// The last parameter is normally size_t but the test is about the abnormal
+// situation when it's not a size_t.
+size_t strlcpy(char *, const char *, int);
+
+enum WeirdDecl {
+  AStrangeWayToSpecifyStringLengthCorrectly = 10UL,
+  AStrangeWayToSpecifyStringLengthIncorrectly = 5UL
+};
+void testWeirdDecls(const char *src) {
+  char dst[10];
+  strlcpy(dst, src, AStrangeWayToSpecifyStringLengthCorrectly); // no-crash
+  strlcpy(dst, src, AStrangeWayToSpecifyStringLengthIncorrectly); // no-crash // no-warning
+}

clang/test/Analysis/cstring-syntax-weird2.c

+// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
+// RUN:                    -verify %s
+
+// expected-no-diagnostics
+
+typedef __SIZE_TYPE__ size_t;
+// The last parameter is normally size_t but the test is about the abnormal
+// situation when it's not a size_t.
+size_t strlcpy(char *, const char *, void (*)());
+
+void foo();
+
+void testWeirdDecls(const char *src) {
+  char dst[10];
+  strlcpy(dst, src, foo); // no-crash
+  strlcpy(dst, src, &foo); // no-crash
+}

clang/test/Analysis/cstring-syntax.c

-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple aarch64_be-none-linux-gnu -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:                    -Wno-strncat-size -Wno-sizeof-pointer-memaccess     \
+// RUN:                    -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:                    -Wno-strncat-size -Wno-sizeof-pointer-memaccess     \
+// RUN:                    -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:                    -triple armv7-a15-linux
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:                    -Wno-strncat-size -Wno-sizeof-pointer-memaccess     \
+// RUN:                    -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:                    -triple aarch64_be-none-linux-gnu
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:                    -Wno-strncat-size -Wno-sizeof-pointer-memaccess     \
+// RUN:                    -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:                    -triple i386-apple-darwin10
 
 typedef __SIZE_TYPE__ size_t;
 char  *strncat(char *, const char *, size_t);
 size_t strlen (const char *s);
 size_t strlcpy(char *, const char *, size_t);
 size_t strlcat(char *, const char *, size_t);
 
 void testStrncat(const char *src) {