[PATCH] D42404: [analyzer] Do not infer nullability inside function-like macros, even when macro is explicitly returning NULL

2018-02-02 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Woohoo even better.


Repository:
  rC Clang

https://reviews.llvm.org/D42404



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D42404: [analyzer] Do not infer nullability inside function-like macros, even when macro is explicitly returning NULL

2018-02-02 Thread George Karpenkov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324161: [analyzer] Do not infer nullability inside 
function-like macros, even when… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D42404

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/diagnostics/macro-null-return-suppression.cpp

Index: test/Analysis/diagnostics/macro-null-return-suppression.cpp
===
--- test/Analysis/diagnostics/macro-null-return-suppression.cpp
+++ 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}}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -159,8 +159,108 @@
   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 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 {
+
+  const SubRegion *RegionOfInterest;
+
+public:
+  MacroNullReturnSuppressionVisitor(const SubRegion *R) : RegionOfInterest(R) {}
+
+  static void *getTag() {
+static int Tag = 0;
+return static_cast(&Tag);
+  }
+
+  void Profile(llvm::FoldingSetNodeID &ID) const override {
+ID.AddPointer(getTag());
+  }
+
+  std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override {
+auto BugPoint = BR.getErrorNode()->getLocation().getAs();
+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())
+  BR.addVisitor(llvm::make_unique