Merge pull request #2671 from SavchenkoValeriy/analyzer_cherry_picks_march_2020

Static analyzer cherrypicks 19

Author: SavchenkoValeriy

clang/lib/Analysis/CalledOnceCheck.cpp

@@ -48,9 +48,12 @@ static constexpr unsigned EXPECTED_NUMBER_OF_BASIC_BLOCKS = 8;
 template <class T>
 using CFGSizedVector = llvm::SmallVector<T, EXPECTED_NUMBER_OF_BASIC_BLOCKS>;
 constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = {
-    "completionHandler", "completion", "withCompletionHandler"};
+    "completionHandler", "completion",      "withCompletionHandler",
+    "withCompletion",    "completionBlock", "withCompletionBlock",
+    "replyTo",           "reply",           "withReplyTo"};
 constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
-    "WithCompletionHandler", "WithCompletion"};
+    "WithCompletionHandler", "WithCompletion", "WithCompletionBlock",
+    "WithReplyTo", "WithReply"};
 constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = {
     "error", "cancel", "shouldCall", "done", "OK", "success"};
 
@@ -994,13 +997,15 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
       return hasConventionalSuffix(MethodSelector.getNameForSlot(0));
     }
 
-    return isConventional(MethodSelector.getNameForSlot(PieceIndex));
+    llvm::StringRef PieceName = MethodSelector.getNameForSlot(PieceIndex);
+    return isConventional(PieceName) || hasConventionalSuffix(PieceName);
   }
 
   bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const {
     return isExplicitlyMarked(Parameter) ||
            (CheckConventionalParameters &&
-            isConventional(Parameter->getName()) &&
+            (isConventional(Parameter->getName()) ||
+             hasConventionalSuffix(Parameter->getName())) &&
             isConventional(Parameter->getType()));
   }
 

clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

@@ -508,6 +508,7 @@ class StdLibraryFunctionsChecker
   mutable FunctionSummaryMapType FunctionSummaryMap;
 
   mutable std::unique_ptr<BugType> BT_InvalidArg;
+  mutable bool SummariesInitialized = false;
 
   static SVal getArgSVal(const CallEvent &Call, ArgNo ArgN) {
     return ArgN == Ret ? Call.getReturnValue() : Call.getArgSVal(ArgN);
@@ -823,7 +824,7 @@ StdLibraryFunctionsChecker::findFunctionSummary(const CallEvent &Call,
 
 void StdLibraryFunctionsChecker::initFunctionSummaries(
     CheckerContext &C) const {
-  if (!FunctionSummaryMap.empty())
+  if (SummariesInitialized)
     return;
 
   SValBuilder &SVB = C.getSValBuilder();
@@ -2485,6 +2486,8 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
         Signature(ArgTypes{VoidPtrRestrictTy}, RetType{VoidTy}),
         Summary(EvalCallAsPure));
   }
+
+  SummariesInitialized = true;
 }
 
 void ento::registerStdCLibraryFunctionsChecker(CheckerManager &mgr) {

clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

@@ -19,6 +19,8 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableSet.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -1395,12 +1397,23 @@ class RangeConstraintManager : public RangedConstraintManager {
     return EquivalenceClass::merge(getBasicVals(), F, State, LHS, RHS);
   }
 
-  LLVM_NODISCARD inline ProgramStateRef setConstraint(ProgramStateRef State,
-                                                      EquivalenceClass Class,
-                                                      RangeSet Constraint) {
+  LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED static bool
+  areFeasible(ConstraintRangeTy Constraints) {
+    return llvm::none_of(
+        Constraints,
+        [](const std::pair<EquivalenceClass, RangeSet> &ClassConstraint) {
+          return ClassConstraint.second.isEmpty();
+        });
+  }
+
+  LLVM_NODISCARD ProgramStateRef setConstraint(ProgramStateRef State,
+                                               EquivalenceClass Class,
+                                               RangeSet Constraint) {
     ConstraintRangeTy Constraints = State->get<ConstraintRange>();
     ConstraintRangeTy::Factory &CF = State->get_context<ConstraintRange>();
 
+    assert(!Constraint.isEmpty() && "New constraint should not be empty");
+
     // Add new constraint.
     Constraints = CF.add(Constraints, Class, Constraint);
 
@@ -1413,9 +1426,18 @@ class RangeConstraintManager : public RangedConstraintManager {
       for (EquivalenceClass DisequalClass : Class.getDisequalClasses(State)) {
         RangeSet UpdatedConstraint =
             getRange(State, DisequalClass).Delete(getBasicVals(), F, *Point);
+
+        // If we end up with at least one of the disequal classes to be
+        // constrainted with an empty range-set, the state is infeasible.
+        if (UpdatedConstraint.isEmpty())
+          return nullptr;
+
         Constraints = CF.add(Constraints, DisequalClass, UpdatedConstraint);
       }
 
+    assert(areFeasible(Constraints) && "Constraint manager shouldn't produce "
+                                       "a state with infeasible constraints");
+
     return State->set<ConstraintRange>(Constraints);
   }
 

clang/test/Analysis/PR49490.cpp

@@ -0,0 +1,30 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+struct toggle {
+  bool value;
+};
+
+toggle global_toggle;
+toggle get_global_toggle() { return global_toggle; }
+
+int oob_access();
+
+bool compare(toggle one, bool other) {
+  if (one.value != other)
+    return true;
+
+  if (one.value)
+    oob_access();
+  return true;
+}
+
+bool coin();
+
+void bar() {
+  bool left = coin();
+  bool right = coin();
+  for (;;)
+    compare(get_global_toggle(), left) && compare(get_global_toggle(), right);
+}

clang/test/SemaObjC/warn-called-once.m

@@ -1036,6 +1036,20 @@ - (void)testWithCompletion:(void (^)(void))callback {
   }
 }
 
+- (void)test:(int)cond fooWithReplyTo:(void (^)(void))handler {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking false branch}}
+    handler();
+  }
+}
+
+- (void)test:(int)cond with:(void (^)(void))fooWithCompletionBlock {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking false branch}}
+    fooWithCompletionBlock();
+  }
+}
+
 - (void)completion_handler_wrong_type:(int (^)(void))completionHandler {
   // We don't want to consider completion handlers with non-void return types.
   if ([self condition]) {