[analyzer] Do not infer nullability inside function-like macros, even when macro...

We already suppress such reports for inlined functions, we should then
get the same behavior for macros.
The underlying reason is that the same macro, can be called from many
different contexts, and nullability can only be expected in _some_ of
them.
Assuming that the macro can return null in _all_ of them sometimes leads
to a large number of false positives.

E.g. consider the test case for the dynamic cast implementation in
macro: in such cases, the bug report is unwanted.

Tracked in rdar://36304776

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

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

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 7484fcd18925533ea377137087e090c6205473e6..7a0e4d41be72e8c6c6bda139baef7336e067dc3f 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -159,8 +159,108 @@ std::unique_ptr<PathDiagnosticPiece> BugReporterVisitor::getDefaultEndPath(
   return std::move(P);
 }
 
+/// \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());
+}
+
+/// \return Whether given spelling location corresponds to an expansion
+/// of a function-like macro.
+static bool isFunctionMacroExpansion(SourceLocation Loc,
+                                const SourceManager &SM) {
+  if (!Loc.isMacroID())
+    return false;
+  while (SM.isMacroArgExpansion(Loc))
+    Loc = SM.getImmediateExpansionRange(Loc).first;
+  std::pair<FileID, unsigned> TLInfo = SM.getDecomposedLoc(Loc);
+  SrcMgr::SLocEntry SE = SM.getSLocEntry(TLInfo.first);
+  const SrcMgr::ExpansionInfo &EInfo = SE.getExpansion();
+  return EInfo.isFunctionMacroExpansion();
+}
 
 namespace {
+
+class MacroNullReturnSuppressionVisitor final
+    : public BugReporterVisitorImpl<MacroNullReturnSuppressionVisitor> {
+
+  const SubRegion *RegionOfInterest;
+
+public:
+  MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) {}
+
+  static void *getTag() {
+    static int Tag = 0;
+    return static_cast<void *>(&Tag);
+  }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+    ID.AddPointer(getTag());
+  }
+
+  std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+                                                 const ExplodedNode *PrevN,
+                                                 BugReporterContext &BRC,
+                                                 BugReport &BR) override {
+    auto BugPoint = BR.getErrorNode()->getLocation().getAs<StmtPoint>();
+    if (!BugPoint)
+      return nullptr;
+
+    const SourceManager &SMgr = BRC.getSourceManager();
+    if (auto Loc = matchAssignment(N, BRC)) {
+      if (isFunctionMacroExpansion(*Loc, SMgr)) {
+        std::string MacroName = getMacroName(*Loc, BRC);
+        SourceLocation BugLoc = BugPoint->getStmt()->getLocStart();
+        if (!BugLoc.isMacroID() || getMacroName(BugLoc, BRC) != MacroName)
+          BR.markInvalid(getTag(), MacroName.c_str());
+      }
+    }
+    return nullptr;
+  }
+
+  static void addMacroVisitorIfNecessary(
+        const ExplodedNode *N, const MemRegion *R,
+        bool EnableNullFPSuppression, BugReport &BR,
+        const SVal V) {
+    AnalyzerOptions &Options = N->getState()->getStateManager()
+        .getOwningEngine()->getAnalysisManager().options;
+    if (EnableNullFPSuppression && Options.shouldSuppressNullReturnPaths()
+          && V.getAs<Loc>())
+      BR.addVisitor(llvm::make_unique<MacroNullReturnSuppressionVisitor>(
+              R->getAs<SubRegion>()));
+  }
+
+private:
+  /// \return Source location of right hand side of an assignment
+  /// into \c RegionOfInterest, empty optional if none found.
+  Optional<SourceLocation> matchAssignment(const ExplodedNode *N,
+                                           BugReporterContext &BRC) {
+    const Stmt *S = PathDiagnosticLocation::getStmt(N);
+    ProgramStateRef State = N->getState();
+    auto *LCtx = N->getLocationContext();
+    if (!S)
+      return None;
+
+    if (auto *DS = dyn_cast<DeclStmt>(S)) {
+      if (const VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl()))
+        if (const Expr *RHS = VD->getInit())
+          if (RegionOfInterest->isSubRegionOf(
+                  State->getLValue(VD, LCtx).getAsRegion()))
+            return RHS->getLocStart();
+    } else if (auto *BO = dyn_cast<BinaryOperator>(S)) {
+      const MemRegion *R = N->getSVal(BO->getLHS()).getAsRegion();
+      const Expr *RHS = BO->getRHS();
+      if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(R)) {
+        return RHS->getLocStart();
+      }
+    }
+    return None;
+  }
+};
+
 /// Emits an extra note at the return statement of an interesting stack frame.
 ///
 /// The returned value is marked as an interesting value, and if it's null,
@@ -854,13 +954,6 @@ 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,
@@ -1123,6 +1216,9 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
       // Mark both the variable region and its contents as interesting.
       SVal V = LVState->getRawSVal(loc::MemRegionVal(R));
 
+      MacroNullReturnSuppressionVisitor::addMacroVisitorIfNecessary(
+          N, R, EnableNullFPSuppression, report, V);
+
       report.markInteresting(R);
       report.markInteresting(V);
       report.addVisitor(llvm::make_unique<UndefOrNullArgVisitor>(R));

diff --git a/test/Analysis/diagnostics/macro-null-return-suppression.cpp b/test/Analysis/diagnostics/macro-null-return-suppression.cpp
new file mode 100644 (file)
index 0000000..9a14f03
--- /dev/null
+++ b/test/Analysis/diagnostics/macro-null-return-suppression.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_analyze_cc1 -x c -analyzer-checker=core -analyzer-output=text -verify %s
+
+#define NULL 0
+
+int test_noparammacro() {
+  int *x = NULL; // expected-note{{'x' initialized to a null pointer value}}
+  return *x; // expected-warning{{Dereference of null pointer (loaded from variable 'x')}}
+             // expected-note@-1{{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+#define DYN_CAST(X) (X ? (char*)X : 0)
+#define GENERATE_NUMBER(X) (0)
+
+char test_assignment(int *param) {
+  char *param2;
+  param2 = DYN_CAST(param);
+  return *param2;
+}
+
+char test_declaration(int *param) {
+  char *param2 = DYN_CAST(param);
+  return *param2;
+}
+
+int coin();
+
+int test_multi_decl(int *paramA, int *paramB) {
+  char *param1 = DYN_CAST(paramA), *param2 = DYN_CAST(paramB);
+  if (coin())
+    return *param1;
+  return *param2;
+}
+
+int testDivision(int a) {
+  int divider = GENERATE_NUMBER(2); // expected-note{{'divider' initialized to 0}}
+  return 1/divider; // expected-warning{{Division by zero}}
+                    // expected-note@-1{{Division by zero}}
+}
+
+// Warning should not be suppressed if it happens in the same macro.
+#define DEREF_IN_MACRO(X) int fn() {int *p = 0; return *p; }
+
+DEREF_IN_MACRO(0) // expected-warning{{Dereference of null pointer}}
+                  // expected-note@-1{{'p' initialized to a null}}
+                  // expected-note@-2{{Dereference of null pointer}}