From: George Karpenkov <ekarpenkov@apple.com>
Date: Wed, 7 Mar 2018 19:27:32 +0000 (+0000)
Subject: [analyzer] [NFC] Minor refactoring of NonNullParamChecker
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6522d1a8032cc5c07590d2508ca0b4b9f92c1aa2;p=clang

[analyzer] [NFC] Minor refactoring of NonNullParamChecker

Differential Revision: https://reviews.llvm.org/D43917

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@326935 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
index 6d05159e51..838be2829e 100644
--- a/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -44,13 +44,9 @@ public:
 };
 } // end anonymous namespace
 
-void NonNullParamChecker::checkPreCall(const CallEvent &Call,
-                                       CheckerContext &C) const {
+/// \return Bitvector marking non-null attributes.
+static llvm::SmallBitVector getNonNullAttrs(const CallEvent &Call) {
   const Decl *FD = Call.getDecl();
-  if (!FD)
-    return;
-
-  // Merge all non-null attributes
   unsigned NumArgs = Call.getNumArgs();
   llvm::SmallBitVector AttrNonNull(NumArgs);
   for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
@@ -64,43 +60,47 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call,
       AttrNonNull.set(Val);
     }
   }
+  return AttrNonNull;
+}
 
-  ProgramStateRef state = C.getState();
+void NonNullParamChecker::checkPreCall(const CallEvent &Call,
+                                       CheckerContext &C) const {
+  if (!Call.getDecl())
+    return;
+
+  llvm::SmallBitVector AttrNonNull = getNonNullAttrs(Call);
+  unsigned NumArgs = Call.getNumArgs();
 
-  CallEvent::param_type_iterator TyI = Call.param_type_begin(),
-                                 TyE = Call.param_type_end();
+  ProgramStateRef state = C.getState();
+  ArrayRef<ParmVarDecl*> parms = Call.parameters();
 
   for (unsigned idx = 0; idx < NumArgs; ++idx) {
+    // For vararg functions, a corresponding parameter decl may not exist.
+    bool HasParam = idx < parms.size();
 
     // Check if the parameter is a reference. We want to report when reference
     // to a null pointer is passed as a parameter.
-    bool haveRefTypeParam = false;
-    if (TyI != TyE) {
-      haveRefTypeParam = (*TyI)->isReferenceType();
-      TyI++;
-    }
-
+    bool haveRefTypeParam =
+        HasParam ? parms[idx]->getType()->isReferenceType() : false;
     bool haveAttrNonNull = AttrNonNull[idx];
-    if (!haveAttrNonNull) {
-      // Check if the parameter is also marked 'nonnull'.
-      ArrayRef<ParmVarDecl*> parms = Call.parameters();
-      if (idx < parms.size())
-        haveAttrNonNull = parms[idx]->hasAttr<NonNullAttr>();
-    }
 
-    if (!haveRefTypeParam && !haveAttrNonNull)
+    // Check if the parameter is also marked 'nonnull'.
+    if (!haveAttrNonNull && HasParam)
+      haveAttrNonNull = parms[idx]->hasAttr<NonNullAttr>();
+
+    if (!haveAttrNonNull && !haveRefTypeParam)
       continue;
 
     // If the value is unknown or undefined, we can't perform this check.
     const Expr *ArgE = Call.getArgExpr(idx);
     SVal V = Call.getArgSVal(idx);
-    Optional<DefinedSVal> DV = V.getAs<DefinedSVal>();
+    auto DV = V.getAs<DefinedSVal>();
     if (!DV)
       continue;
 
-    // Process the case when the argument is not a location.
     assert(!haveRefTypeParam || DV->getAs<Loc>());
 
+    // Process the case when the argument is not a location.
     if (haveAttrNonNull && !DV->getAs<Loc>()) {
       // If the argument is a union type, we want to handle a potential
       // transparent_union GCC extension.
@@ -112,66 +112,63 @@ void NonNullParamChecker::checkPreCall(const CallEvent &Call,
       if (!UT || !UT->getDecl()->hasAttr<TransparentUnionAttr>())
         continue;
 
-      if (Optional<nonloc::CompoundVal> CSV =
-              DV->getAs<nonloc::CompoundVal>()) {
-        nonloc::CompoundVal::iterator CSV_I = CSV->begin();
-        assert(CSV_I != CSV->end());
-        V = *CSV_I;
-        DV = V.getAs<DefinedSVal>();
-        assert(++CSV_I == CSV->end());
-        // FIXME: Handle (some_union){ some_other_union_val }, which turns into
-        // a LazyCompoundVal inside a CompoundVal.
-        if (!V.getAs<Loc>())
-          continue;
-        // Retrieve the corresponding expression.
-        if (const CompoundLiteralExpr *CE = dyn_cast<CompoundLiteralExpr>(ArgE))
-          if (const InitListExpr *IE =
-                dyn_cast<InitListExpr>(CE->getInitializer()))
-             ArgE = dyn_cast<Expr>(*(IE->begin()));
-
-      } else {
-        // FIXME: Handle LazyCompoundVals?
+      auto CSV = DV->getAs<nonloc::CompoundVal>();
+
+      // FIXME: Handle LazyCompoundVals?
+      if (!CSV)
         continue;
-      }
+
+      V = *(CSV->begin());
+      DV = V.getAs<DefinedSVal>();
+      assert(++CSV->begin() == CSV->end());
+      // FIXME: Handle (some_union){ some_other_union_val }, which turns into
+      // a LazyCompoundVal inside a CompoundVal.
+      if (!V.getAs<Loc>())
+        continue;
+
+      // Retrieve the corresponding expression.
+      if (const auto *CE = dyn_cast<CompoundLiteralExpr>(ArgE))
+        if (const auto *IE = dyn_cast<InitListExpr>(CE->getInitializer()))
+          ArgE = dyn_cast<Expr>(*(IE->begin()));
     }
 
     ConstraintManager &CM = C.getConstraintManager();
     ProgramStateRef stateNotNull, stateNull;
     std::tie(stateNotNull, stateNull) = CM.assumeDual(state, *DV);
 
-    if (stateNull) {
-      if (!stateNotNull) {
-        // Generate an error node.  Check for a null node in case
-        // we cache out.
-        if (ExplodedNode *errorNode = C.generateErrorNode(stateNull)) {
-
-          std::unique_ptr<BugReport> R;
-          if (haveAttrNonNull)
-            R = genReportNullAttrNonNull(errorNode, ArgE);
-          else if (haveRefTypeParam)
-            R = genReportReferenceToNullPointer(errorNode, ArgE);
-
-          // Highlight the range of the argument that was null.
-          R->addRange(Call.getArgSourceRange(idx));
-
-          // Emit the bug report.
-          C.emitReport(std::move(R));
-        }
-
-        // Always return.  Either we cached out or we just emitted an error.
-        return;
+    // Generate an error node.  Check for a null node in case
+    // we cache out.
+    if (stateNull && !stateNotNull) {
+      if (ExplodedNode *errorNode = C.generateErrorNode(stateNull)) {
+
+        std::unique_ptr<BugReport> R;
+        if (haveAttrNonNull)
+          R = genReportNullAttrNonNull(errorNode, ArgE);
+        else if (haveRefTypeParam)
+          R = genReportReferenceToNullPointer(errorNode, ArgE);
+
+        // Highlight the range of the argument that was null.
+        R->addRange(Call.getArgSourceRange(idx));
+
+        // Emit the bug report.
+        C.emitReport(std::move(R));
       }
+
+      // Always return.  Either we cached out or we just emitted an error.
+      return;
+    }
+
+    if (stateNull) {
       if (ExplodedNode *N = C.generateSink(stateNull, C.getPredecessor())) {
         ImplicitNullDerefEvent event = {
-            V, false, N, &C.getBugReporter(),
-            /*IsDirectDereference=*/haveRefTypeParam};
+          V, false, N, &C.getBugReporter(),
+          /*IsDirectDereference=*/haveRefTypeParam};
         dispatchEvent(event);
       }
     }
 
     // If a pointer value passed the check we should assume that it is
     // indeed not null from this point forward.
-    assert(stateNotNull);
     state = stateNotNull;
   }