[analyzer] suppress nullability inference from a macro when result is used in another...

The current code used to not suppress the report, if the dereference was
performed in a macro, assuming it is that same macro.
However, the assumption might not be correct, and XNU has quite a bit of
code where dereference is actually performed in a different macro.

As the code uses macro name and not a unique identifier it might be fragile,
but in a worst-case scenario we would simply emit an extra diagnostic.

rdar://36160245

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

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

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 972f4c5f3da2ecec858725316dd1f2c3795d35aa..eb8f0bab3eb701fa24bb53435d8223a73621eaab 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -839,6 +839,13 @@ const char *SuppressInlineDefensiveChecksVisitor::getTag() {
   return "IDCVisitor";
 }
 
+/// \return name of the macro inside the location \p Loc.
+static StringRef getMacroName(SourceLocation Loc,
+    BugReporterContext &BRC) {
+  return Lexer::getImmediateMacroName(
+      Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
+}
+
 std::shared_ptr<PathDiagnosticPiece>
 SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
                                                 const ExplodedNode *Pred,
@@ -878,9 +885,6 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
     if (!BugPoint)
       return nullptr;
 
-    SourceLocation BugLoc = BugPoint->getStmt()->getLocStart();
-    if (BugLoc.isMacroID())
-      return nullptr;
 
     ProgramPoint CurPoint = Succ->getLocation();
     const Stmt *CurTerminatorStmt = nullptr;
@@ -907,7 +911,13 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
       SrcMgr::SLocEntry SE = SMgr.getSLocEntry(TLInfo.first);
       const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion();
       if (EInfo.isFunctionMacroExpansion()) {
-        BR.markInvalid("Suppress Macro IDC", CurLC);
+        SourceLocation BugLoc = BugPoint->getStmt()->getLocStart();
+
+        // Suppress reports unless we are in that same macro.
+        if (!BugLoc.isMacroID() ||
+            getMacroName(BugLoc, BRC) != getMacroName(TerminatorLoc, BRC)) {
+          BR.markInvalid("Suppress Macro IDC", CurLC);
+        }
         return nullptr;
       }
     }

diff --git a/test/Analysis/inlining/false-positive-suppression.c b/test/Analysis/inlining/false-positive-suppression.c
index 4931695ef12743aaf7abe742aeeb092615cd60e6..5f613e12c2fd0c455e13f3c7427a45df504efaeb 100644 (file)
--- a/test/Analysis/inlining/false-positive-suppression.c
+++ b/test/Analysis/inlining/false-positive-suppression.c
@@ -163,6 +163,7 @@ void testNestedDisjunctiveMacro2(int *p, int *q) {
 }
 
 
+
 // Here the check is entirely in non-macro code even though the code itself
 // is a macro argument.
 #define MACRO_DO_IT(a) (a)
@@ -171,6 +172,15 @@ void testErrorInArgument(int *p) {
   (void)i;
 }
 
+// No warning should be emitted if dereference is performed from a different
+// macro.
+#define MACRO_CHECK(a) if (a) {}
+#define MACRO_DEREF(a) (*a)
+int testDifferentMacro(int *p) {
+  MACRO_CHECK(p);
+  return MACRO_DEREF(p); // no-warning
+}
+
 // --------------------------
 // "Suppression suppression"
 // --------------------------