r324161 - [analyzer] Do not infer nullability inside function-like macros, even when macro is explicitly returning NULL
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 2 16:55:21 PST 2018
Author: george.karpenkov
Date: Fri Feb 2 16:55:21 2018
New Revision: 324161
URL: http://llvm.org/viewvc/llvm-project?rev=324161&view=rev
Log:
[analyzer] Do not infer nullability inside function-like macros, even when macro is explicitly returning NULL
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
Added:
cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=324161&r1=324160&r2=324161&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Feb 2 16:55:21 2018
@@ -159,8 +159,108 @@ std::unique_ptr<PathDiagnosticPiece> Bug
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 *SuppressInlineDefensiveCheck
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(
// 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));
Added: cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp?rev=324161&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp (added)
+++ cfe/trunk/test/Analysis/diagnostics/macro-null-return-suppression.cpp Fri Feb 2 16:55:21 2018
@@ -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 at -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 at -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 at -1{{'p' initialized to a null}}
+ // expected-note at -2{{Dereference of null pointer}}
More information about the cfe-commits
mailing list