r259222 - [analyzer] Suppress null reports from defensive checks in function-like macros.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Jan 29 12:47:13 2016
New Revision: 259222

URL: http://llvm.org/viewvc/llvm-project?rev=259222&view=rev
Log:
[analyzer] Suppress null reports from defensive checks in function-like macros.

We already do this for case splits introduced as a result of defensive null
checks in functions and methods, so do the same for function-like macros.

rdar://problem/19640441

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/inlining/false-positive-suppression.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=259222&r1=259221&r2=259222&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Jan 29 12:47:13 2016
@@ -828,8 +828,33 @@ SuppressInlineDefensiveChecksVisitor::Vi
     // Check if this is inlined defensive checks.
     const LocationContext *CurLC =Succ->getLocationContext();
     const LocationContext *ReportLC = BR.getErrorNode()->getLocationContext();
-    if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC))
+    if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) {
       BR.markInvalid("Suppress IDC", CurLC);
+      return nullptr;
+    }
+
+    // Treat defensive checks in function-like macros as if they were an inlined
+    // defensive check.
+    auto CurPoint = Succ->getLocation().getAs<BlockEdge>();
+    auto BugPoint = BR.getErrorNode()->getLocation().getAs<StmtPoint>();
+
+    if (!CurPoint || !BugPoint)
+      return nullptr;
+
+    SourceLocation CurLoc =
+        CurPoint->getSrc()->getTerminator().getStmt()->getLocStart();
+    SourceLocation BugLoc = BugPoint->getStmt()->getLocStart();
+
+    if (CurLoc.isMacroID() && !BugLoc.isMacroID()) {
+      const SourceManager &SMgr = BRC.getSourceManager();
+      std::pair<FileID, unsigned> CLInfo = SMgr.getDecomposedLoc(CurLoc);
+      SrcMgr::SLocEntry SE = SMgr.getSLocEntry(CLInfo.first);
+      const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion();
+      if (EInfo.isFunctionMacroExpansion()) {
+        BR.markInvalid("Suppress Macro IDC", CurLC);
+        return nullptr;
+      }
+    }
   }
   return nullptr;
 }

Modified: cfe/trunk/test/Analysis/inlining/false-positive-suppression.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/false-positive-suppression.c?rev=259222&r1=259221&r2=259222&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/false-positive-suppression.c (original)
+++ cfe/trunk/test/Analysis/inlining/false-positive-suppression.c Fri Jan 29 12:47:13 2016
@@ -93,6 +93,74 @@ int triggerDivZero () {
   return 5/y; // expected-warning {{Division by zero}}
 }
 
+// Treat a function-like macro similarly to an inlined function, so suppress
+// warnings along paths resulting from inlined checks.
+#define MACRO_WITH_CHECK(a) ( ((a) != 0) ? *a : 17)
+void testInlineCheckInMacro(int *p) {
+  int i = MACRO_WITH_CHECK(p);
+  (void)i;
+
+  *p = 1; // no-warning
+}
+
+#define MACRO_WITH_NESTED_CHECK(a) ( { int j = MACRO_WITH_CHECK(a); j; } )
+void testInlineCheckInNestedMacro(int *p) {
+  int i = MACRO_WITH_NESTED_CHECK(p);
+  (void)i;
+
+  *p = 1; // no-warning
+}
+
+// If there is a check in a macro that is not function-like, don't treat
+// it like a function so don't suppress.
+#define NON_FUNCTION_MACRO_WITH_CHECK ( ((p) != 0) ? *p : 17)
+void testNonFunctionMacro(int *p) {
+  int i = NON_FUNCTION_MACRO_WITH_CHECK ;
+  (void)i;
+
+  *p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+}
+
+
+// This macro will dereference its argument if the argument is NULL.
+#define MACRO_WITH_ERROR(a) ( ((a) != 0) ? 0 : *a)
+void testErrorInMacro(int *p) {
+  int i = MACRO_WITH_ERROR(p); // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+  (void)i;
+}
+
+// Here the check (the "if") is not in a macro, so we should still warn.
+#define MACRO_IN_GUARD(a) (!(a))
+void testMacroUsedAsGuard(int *p) {
+  if (MACRO_IN_GUARD(p))
+    *p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+}
+
+// When a nil case split is introduced in a macro and the macro is in a guard,
+// we still shouldn't warn.
+int isNull(int *p);
+int isEqual(int *p, int *q);
+#define ISNULL(ptr)    ((ptr) == 0 || isNull(ptr))
+#define ISEQUAL(a, b)    ((int *)(a) == (int *)(b) || (ISNULL(a) && ISNULL(b)) || isEqual(a,b))
+#define ISNOTEQUAL(a, b)   (!ISEQUAL(a, b))
+void testNestedDisjunctiveMacro(int *p, int *q) {
+  if (ISNOTEQUAL(p,q)) {
+    (void)*p; // no-warning
+    (void)*q; // no-warning
+  }
+
+  (void)*p; // no-warning
+  (void)*q; // no-warning
+}
+
+// Here the check is entirely in non-macro code even though the code itself
+// is a macro argument.
+#define MACRO_DO_IT(a) (a)
+void testErrorInArgument(int *p) {
+  int i = MACRO_DO_IT((p ? 0 : *p)); // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}c
+  (void)i;
+}
+
 // --------------------------
 // "Suppression suppression"
 // --------------------------