bruntib created this revision.
bruntib added reviewers: alexfh, NoQ, Szelethus, xazax.hun.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a project: clang.

There are some functions which can't be given a null pointer as parameter 
either because it has a nonnull attribute or it is declared to have undefined 
behavior (e.g. strcmp()). Sometimes it is hard to determine from the checker 
message which parameter is null at the invocation, so now this information is 
included in the message.

This commit fixes https://bugs.llvm.org/show_bug.cgi?id=39358


Repository:
  rC Clang

https://reviews.llvm.org/D66333

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/null-deref-ps.c

Index: clang/test/Analysis/null-deref-ps.c
===================================================================
--- clang/test/Analysis/null-deref-ps.c
+++ clang/test/Analysis/null-deref-ps.c
@@ -88,21 +88,21 @@
 int bar(int* p, int q) __attribute__((nonnull));
 
 int f6(int *p) { 
-  return !p ? bar(p, 1) // expected-warning {{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return !p ? bar(p, 1) // expected-warning {{Null pointer passed as an argument to a 'nonnull' 1st parameter}}
          : bar(p, 0);   // no-warning
 }
 
 int bar2(int* p, int q) __attribute__((nonnull(1)));
 
 int f6b(int *p) { 
-  return !p ? bar2(p, 1) // expected-warning {{Null pointer passed as an argument to a 'nonnull' parameter}}
+  return !p ? bar2(p, 1) // expected-warning {{Null pointer passed as an argument to a 'nonnull' 1st parameter}}
          : bar2(p, 0);   // no-warning
 }
 
 int bar3(int*p, int q, int *r) __attribute__((nonnull(1,3)));
 
 int f6c(int *p, int *q) {
-   return !p ? bar3(q, 2, p) // expected-warning {{Null pointer passed as an argument to a 'nonnull' parameter}}
+   return !p ? bar3(q, 2, p) // expected-warning {{Null pointer passed as an argument to a 'nonnull' 3rd parameter}}
              : bar3(p, 2, q); // no-warning
 }
 
Index: clang/test/Analysis/misc-ps-region-store.m
===================================================================
--- clang/test/Analysis/misc-ps-region-store.m
+++ clang/test/Analysis/misc-ps-region-store.m
@@ -1205,7 +1205,7 @@
 void rdar_8642434_funcB(struct rdar_8642434_typeA *x, struct rdar_8642434_typeA *y) {
   rdar_8642434_funcA(x);
   if (!y)
-    rdar_8642434_funcA(y); // expected-warning{{Null pointer passed as an argument to a 'nonnull' parameter}}
+    rdar_8642434_funcA(y); // expected-warning{{Null pointer passed as an argument to a 'nonnull' 1st parameter}}
 }
 
 // <rdar://problem/8848957> - Handle loads and stores from a symbolic index
Index: clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
+++ clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
@@ -6141,12 +6141,12 @@
      </array>
      <key>depth</key><integer>0</integer>
      <key>extended_message</key>
-     <string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+     <string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
      <key>message</key>
-     <string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+     <string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
     </dict>
    </array>
-   <key>description</key><string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+   <key>description</key><string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
    <key>category</key><string>API</string>
    <key>type</key><string>Argument with &apos;nonnull&apos; attribute passed null</string>
    <key>check_name</key><string>core.NonNullParamChecker</string>
Index: clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
+++ clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
@@ -10853,12 +10853,12 @@
      </array>
      <key>depth</key><integer>0</integer>
      <key>extended_message</key>
-     <string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+     <string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
      <key>message</key>
-     <string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+     <string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
     </dict>
    </array>
-   <key>description</key><string>Null pointer passed as an argument to a &apos;nonnull&apos; parameter</string>
+   <key>description</key><string>Null pointer passed as an argument to a &apos;nonnull&apos; 1st parameter</string>
    <key>category</key><string>API</string>
    <key>type</key><string>Argument with &apos;nonnull&apos; attribute passed null</string>
    <key>check_name</key><string>core.NonNullParamChecker</string>
Index: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -36,7 +36,9 @@
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
 
   std::unique_ptr<BugReport>
-  genReportNullAttrNonNull(const ExplodedNode *ErrorN, const Expr *ArgE) const;
+  genReportNullAttrNonNull(const ExplodedNode *ErrorN,
+                           const Expr *ArgE,
+                           unsigned Idx) const;
   std::unique_ptr<BugReport>
   genReportReferenceToNullPointer(const ExplodedNode *ErrorN,
                                   const Expr *ArgE) const;
@@ -143,7 +145,7 @@
 
         std::unique_ptr<BugReport> R;
         if (haveAttrNonNull)
-          R = genReportNullAttrNonNull(errorNode, ArgE);
+          R = genReportNullAttrNonNull(errorNode, ArgE, idx + 1);
         else if (haveRefTypeParam)
           R = genReportReferenceToNullPointer(errorNode, ArgE);
 
@@ -179,7 +181,8 @@
 
 std::unique_ptr<BugReport>
 NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode,
-                                              const Expr *ArgE) const {
+                                              const Expr *ArgE,
+                                              unsigned Idx) const {
   // Lazily allocate the BugType object if it hasn't already been
   // created. Ownership is transferred to the BugReporter object once
   // the BugReport is passed to 'EmitWarning'.
@@ -187,9 +190,12 @@
     BTAttrNonNull.reset(new BugType(
         this, "Argument with 'nonnull' attribute passed null", "API"));
 
-  auto R = std::make_unique<BugReport>(
-      *BTAttrNonNull,
-      "Null pointer passed as an argument to a 'nonnull' parameter", ErrorNode);
+  llvm::SmallString<256> SBuf;
+  llvm::raw_svector_ostream OS(SBuf);
+  OS << "Null pointer passed as an argument to a 'nonnull' ";
+  OS << Idx << llvm::getOrdinalSuffix(Idx) << " parameter";
+
+  auto R = llvm::make_unique<BugReport>(*BTAttrNonNull, SBuf, ErrorNode);
   if (ArgE)
     bugreporter::trackExpressionValue(ErrorNode, ArgE, *R);
 
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -198,7 +198,8 @@
   ProgramStateRef checkNonNull(CheckerContext &C,
                                    ProgramStateRef state,
                                    const Expr *S,
-                                   SVal l) const;
+                                   SVal l,
+                                   unsigned idx = -1u) const;
   ProgramStateRef CheckLocation(CheckerContext &C,
                                     ProgramStateRef state,
                                     const Expr *S,
@@ -277,7 +278,8 @@
 
 ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
                                             ProgramStateRef state,
-                                            const Expr *S, SVal l) const {
+                                            const Expr *S, SVal l,
+                                            unsigned idx) const {
   // If a previous check has failed, propagate the failure.
   if (!state)
     return nullptr;
@@ -291,6 +293,8 @@
       llvm::raw_svector_ostream os(buf);
       assert(CurrentFunctionDescription);
       os << "Null pointer argument in call to " << CurrentFunctionDescription;
+      if (idx != -1u)
+        os << ' ' << idx << llvm::getOrdinalSuffix(idx) << " parameter";
 
       emitNullArgBug(C, stateNull, S, os.str());
     }
@@ -1541,14 +1545,14 @@
   const Expr *Dst = CE->getArg(0);
   SVal DstVal = state->getSVal(Dst, LCtx);
 
-  state = checkNonNull(C, state, Dst, DstVal);
+  state = checkNonNull(C, state, Dst, DstVal, 1);
   if (!state)
     return;
 
   // Check that the source is non-null.
   const Expr *srcExpr = CE->getArg(1);
   SVal srcVal = state->getSVal(srcExpr, LCtx);
-  state = checkNonNull(C, state, srcExpr, srcVal);
+  state = checkNonNull(C, state, srcExpr, srcVal, 2);
   if (!state)
     return;
 
@@ -1904,14 +1908,14 @@
   // Check that the first string is non-null
   const Expr *s1 = CE->getArg(0);
   SVal s1Val = state->getSVal(s1, LCtx);
-  state = checkNonNull(C, state, s1, s1Val);
+  state = checkNonNull(C, state, s1, s1Val, 1);
   if (!state)
     return;
 
   // Check that the second string is non-null.
   const Expr *s2 = CE->getArg(1);
   SVal s2Val = state->getSVal(s2, LCtx);
-  state = checkNonNull(C, state, s2, s2Val);
+  state = checkNonNull(C, state, s2, s2Val, 2);
   if (!state)
     return;
 
@@ -2038,14 +2042,14 @@
   // Check that the search string pointer is non-null (though it may point to
   // a null string).
   SVal SearchStrVal = State->getSVal(SearchStrPtr, LCtx);
-  State = checkNonNull(C, State, SearchStrPtr, SearchStrVal);
+  State = checkNonNull(C, State, SearchStrPtr, SearchStrVal, 1);
   if (!State)
     return;
 
   // Check that the delimiter string is non-null.
   const Expr *DelimStr = CE->getArg(1);
   SVal DelimStrVal = State->getSVal(DelimStr, LCtx);
-  State = checkNonNull(C, State, DelimStr, DelimStrVal);
+  State = checkNonNull(C, State, DelimStr, DelimStrVal, 2);
   if (!State)
     return;
 
@@ -2148,7 +2152,7 @@
 
   // Ensure the memory area is not null.
   // If it is NULL there will be a NULL pointer dereference.
-  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal);
+  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1);
   if (!State)
     return;
 
@@ -2195,7 +2199,7 @@
 
   // Ensure the memory area is not null.
   // If it is NULL there will be a NULL pointer dereference.
-  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal);
+  State = checkNonNull(C, StateNonZeroSize, Mem, MemVal, 1);
   if (!State)
     return;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to