From caa8ca5fbfcf02a09117af69182cc21917118ac9 Mon Sep 17 00:00:00 2001
From: Devin Coughlin <dcoughlin@apple.com>
Date: Sat, 30 Jan 2016 01:59:33 +0000
Subject: [PATCH] [analyzer] Make suppression of macro defensive checks work
 with -analyzer-eagerly-assume.

This is the default for the analyzer but the flag is added by the driver so
our suppression tests didn't cover this case.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@259288 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Core/BugReporterVisitors.cpp              | 37 +++++++++++++++----
 .../inlining/false-positive-suppression.c     | 24 ++++++++----
 2 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index f1c3223fae..ae5cd546f8 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -14,6 +14,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -834,21 +835,41 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
     }
 
     // Treat defensive checks in function-like macros as if they were an inlined
-    // defensive check.
-    auto CurPoint = Succ->getLocation().getAs<BlockEdge>();
+    // defensive check. If the bug location is not in a macro and the
+    // terminator for the current location is in a macro then suppress the
+    // warning.
     auto BugPoint = BR.getErrorNode()->getLocation().getAs<StmtPoint>();
 
-    if (!CurPoint || !BugPoint)
+    if (!BugPoint)
       return nullptr;
 
-    SourceLocation CurLoc =
-        CurPoint->getSrc()->getTerminator().getStmt()->getLocStart();
     SourceLocation BugLoc = BugPoint->getStmt()->getLocStart();
+    if (BugLoc.isMacroID())
+      return nullptr;
+
+    ProgramPoint CurPoint = Succ->getLocation();
+    const Stmt *CurTerminatorStmt = nullptr;
+    if (auto BE = CurPoint.getAs<BlockEdge>()) {
+      CurTerminatorStmt = BE->getSrc()->getTerminator().getStmt();
+    } else if (auto SP = CurPoint.getAs<StmtPoint>()) {
+      const Stmt *CurStmt = SP->getStmt();
+      if (!CurStmt->getLocStart().isMacroID())
+        return nullptr;
+
+      CFGStmtMap *Map = CurLC->getAnalysisDeclContext()->getCFGStmtMap();
+      CurTerminatorStmt = Map->getBlock(CurStmt)->getTerminator();
+    } else {
+      return nullptr;
+    }
+
+    if (!CurTerminatorStmt)
+      return nullptr;
 
-    if (CurLoc.isMacroID() && !BugLoc.isMacroID()) {
+    SourceLocation TerminatorLoc = CurTerminatorStmt->getLocStart();
+    if (TerminatorLoc.isMacroID()) {
       const SourceManager &SMgr = BRC.getSourceManager();
-      std::pair<FileID, unsigned> CLInfo = SMgr.getDecomposedLoc(CurLoc);
-      SrcMgr::SLocEntry SE = SMgr.getSLocEntry(CLInfo.first);
+      std::pair<FileID, unsigned> TLInfo = SMgr.getDecomposedLoc(TerminatorLoc);
+      SrcMgr::SLocEntry SE = SMgr.getSLocEntry(TLInfo.first);
       const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion();
       if (EInfo.isFunctionMacroExpansion()) {
         BR.markInvalid("Suppress Macro IDC", CurLC);
diff --git a/test/Analysis/inlining/false-positive-suppression.c b/test/Analysis/inlining/false-positive-suppression.c
index 335940ab5f..a0bc3611fe 100644
--- a/test/Analysis/inlining/false-positive-suppression.c
+++ b/test/Analysis/inlining/false-positive-suppression.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -DSUPPRESSED=1 %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config avoid-suppressing-null-argument-paths=true -DSUPPRESSED=1 -DNULL_ARGS=1 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-eagerly-assume -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-eagerly-assume -analyzer-checker=core -verify -DSUPPRESSED=1 %s
+// RUN: %clang_cc1 -analyze -analyzer-eagerly-assume -analyzer-checker=core -analyzer-config avoid-suppressing-null-argument-paths=true -DSUPPRESSED=1 -DNULL_ARGS=1 -verify %s
 
 int opaquePropertyCheck(void *object);
 int coin();
@@ -145,14 +145,24 @@ int isEqual(int *p, int *q);
 #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
+    *p = 1; // no-warning
+    *q = 1; // no-warning
   }
 
-  (void)*p; // no-warning
-  (void)*q; // no-warning
+  *p = 1; // no-warning
+  *q = 1; // no-warning
+}
+
+void testNestedDisjunctiveMacro2(int *p, int *q) {
+  if (ISEQUAL(p,q)) {
+    return;
+  }
+
+  *p = 1; // no-warning
+  *q = 1; // 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)
-- 
2.40.0