From: George Karpenkov Date: Sat, 3 Feb 2018 00:55:21 +0000 (+0000) Subject: [analyzer] Do not infer nullability inside function-like macros, even when macro... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d4a7bc588f9f950e90e3fcc7186e3bc2e706b116;p=clang [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 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 7484fcd189..7a0e4d41be 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -159,8 +159,108 @@ std::unique_ptr 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 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( + R->getAs())); + } + +private: + /// \return Source location of right hand side of an assignment + /// into \c RegionOfInterest, empty optional if none found. + Optional 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(S)) { + if (const VarDecl *VD = dyn_cast(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(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 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(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 index 0000000000..9a14f03a32 --- /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}}