Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -83,6 +83,36 @@ BugReport &BR); }; +namespace bugreporter { + +/// Attempts to add visitors to track expression value back to its point of +/// origin. +/// +/// \param N A node "downstream" from the evaluation of the statement. +/// \param E The expression value which we are tracking +/// \param R The bug report to which visitors should be attached. +/// \param EnableNullFPSuppression Whether we should employ false positive +/// suppression (inlined defensive checks, returned null). +/// +/// \return Whether or not the function was able to add visitors for this +/// statement. Note that returning \c true does not actually imply +/// that any visitors were added. +bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport &R, + bool EnableNullFPSuppression = true); + +/// Specifies the type of tracking for an expression. For instance, we'd like to +/// gather far more information about a variable found to be a cause of a null +/// pointer dereference, while tracking a condition to that extent would pollute +/// the bug report without much of an added value. +enum class TrackingKind { + ThoroughTracking, + ConditionTracking +}; + +const Expr *getDerefExpr(const Stmt *S); + +} // namespace bugreporter + /// Finds last store into the given region, /// which is different from a given symbolic value. class FindLastStoreBRVisitor final : public BugReporterVisitor { @@ -94,15 +124,28 @@ /// bug, we are going to employ false positive suppression. bool EnableNullFPSuppression; + using TrackingKind = bugreporter::TrackingKind; + TrackingKind TKind; + public: /// Creates a visitor for every VarDecl inside a Stmt and registers it with /// the BugReport. static void registerStatementVarDecls(BugReport &BR, const Stmt *S, - bool EnableNullFPSuppression); - + bool EnableNullFPSuppression, + TrackingKind TKind); + + /// \param V We're searching for the store where \c R received this value. + /// \param R The region we're tracking. + /// \param EnableNullFPSuppression Whether we should employ false positive + /// suppression (inlined defensive checks, returned null). + /// \param TKind May limit the amount of notes added to the bug report. FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R, - bool InEnableNullFPSuppression) - : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression) {} + bool InEnableNullFPSuppression, + TrackingKind TKind) + : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression), + TKind(TKind) { + assert(R); + } void Profile(llvm::FoldingSetNodeID &ID) const override; @@ -345,27 +388,6 @@ BugReport &R) override; }; -namespace bugreporter { - -/// Attempts to add visitors to track expression value back to its point of -/// origin. -/// -/// \param N A node "downstream" from the evaluation of the statement. -/// \param E The expression value which we are tracking -/// \param R The bug report to which visitors should be attached. -/// \param EnableNullFPSuppression Whether we should employ false positive -/// suppression (inlined defensive checks, returned null). -/// -/// \return Whether or not the function was able to add visitors for this -/// statement. Note that returning \c true does not actually imply -/// that any visitors were added. -bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport &R, - bool EnableNullFPSuppression = true); - -const Expr *getDerefExpr(const Stmt *S); - -} // namespace bugreporter - } // namespace ento } // namespace clang Index: clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -87,7 +87,8 @@ if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); R->addVisitor(llvm::make_unique( - *V, VR, /*EnableNullFPSuppression*/ false)); + *V, VR, /*EnableNullFPSuppression*/ false, + bugreporter::TrackingKind::ThoroughTracking)); R->disablePathPruning(); // need location of block C.emitReport(std::move(R)); Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -71,6 +71,20 @@ // Utility functions. //===----------------------------------------------------------------------===// +/// Implementation function for trackExpressionValue(). +static bool trackExpressionValue( + const ExplodedNode *InputNode, + const Expr *E, BugReport &report, + bool EnableNullFPSuppression, + bugreporter::TrackingKind TKind); + +bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, + const Expr *E, BugReport &report, + bool EnableNullFPSuppression) { + return ::trackExpressionValue(InputNode, E, report, EnableNullFPSuppression, + TrackingKind::ThoroughTracking); +} + static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) { if (B->isAdditiveOp() && B->getType()->isPointerType()) { if (B->getLHS()->getType()->isPointerType()) { @@ -1162,9 +1176,45 @@ ID.AddPointer(&tag); ID.AddPointer(R); ID.Add(V); + ID.AddInteger(static_cast(TKind)); ID.AddBoolean(EnableNullFPSuppression); } +void FindLastStoreBRVisitor::registerStatementVarDecls( + BugReport &BR, const Stmt *S, bool EnableNullFPSuppression, + TrackingKind TKind) { + + const ExplodedNode *N = BR.getErrorNode(); + std::deque WorkList; + WorkList.push_back(S); + + while (!WorkList.empty()) { + const Stmt *Head = WorkList.front(); + WorkList.pop_front(); + + ProgramStateManager &StateMgr = N->getState()->getStateManager(); + + if (const auto *DR = dyn_cast(Head)) { + if (const auto *VD = dyn_cast(DR->getDecl())) { + const VarRegion *R = + StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext()); + + // What did we load? + SVal V = N->getSVal(S); + + if (V.getAs() || V.getAs()) { + // Register a new visitor with the BugReport. + BR.addVisitor(llvm::make_unique( + V.castAs(), R, EnableNullFPSuppression, TKind)); + } + } + } + + for (const Stmt *SubStmt : Head->children()) + WorkList.push_back(SubStmt); + } +} + /// Returns true if \p N represents the DeclStmt declaring and initializing /// \p VR. static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) { @@ -1335,7 +1385,7 @@ // should track the initializer expression. if (Optional PIP = Pred->getLocationAs()) { const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue(); - if (FieldReg && FieldReg == R) { + if (FieldReg == R) { StoreSite = Pred; InitE = PIP->getInitializer()->getInit(); } @@ -1400,8 +1450,7 @@ if (!IsParam) InitE = InitE->IgnoreParenCasts(); - bugreporter::trackExpressionValue(StoreSite, InitE, BR, - EnableNullFPSuppression); + trackExpressionValue(StoreSite, InitE, BR, EnableNullFPSuppression, TKind); } // Okay, we've found the binding. Emit an appropriate message. @@ -1429,7 +1478,7 @@ if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (auto KV = State->getSVal(OriginalR).getAs()) BR.addVisitor(llvm::make_unique( - *KV, OriginalR, EnableNullFPSuppression)); + *KV, OriginalR, EnableNullFPSuppression, TKind)); } } } @@ -1730,8 +1779,9 @@ // isn't sufficient, because a new visitor is created for each tracked // expression, hence the BugReport level set. if (BR.addTrackedCondition(N)) { - bugreporter::trackExpressionValue( - N, Condition, BR, /*EnableNullFPSuppression=*/false); + trackExpressionValue( + N, Condition, BR, /*EnableNullFPSuppression=*/false, + bugreporter::TrackingKind::ConditionTracking); return constructDebugPieceForTrackedCondition(Condition, N, BRC); } } @@ -1843,9 +1893,14 @@ return N; } -bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, - const Expr *E, BugReport &report, - bool EnableNullFPSuppression) { +static bool trackExpressionValue( + const ExplodedNode *InputNode, + const Expr *E, BugReport &report, + bool EnableNullFPSuppression, + bugreporter::TrackingKind TKind /* = TrackingKind::ThoroughTracking*/) { + + using namespace bugreporter; + if (!E || !InputNode) return false; @@ -1890,7 +1945,7 @@ if (RR && !LVIsNull) if (auto KV = LVal.getAs()) report.addVisitor(llvm::make_unique( - *KV, RR, EnableNullFPSuppression)); + *KV, RR, EnableNullFPSuppression, TKind)); // In case of C++ references, we want to differentiate between a null // reference and reference to null pointer. @@ -1927,7 +1982,7 @@ if (auto KV = V.getAs()) report.addVisitor(llvm::make_unique( - *KV, R, EnableNullFPSuppression)); + *KV, R, EnableNullFPSuppression, TKind)); return true; } } @@ -1967,7 +2022,7 @@ if (CanDereference) if (auto KV = RVal.getAs()) report.addVisitor(llvm::make_unique( - *KV, L->getRegion(), EnableNullFPSuppression)); + *KV, L->getRegion(), EnableNullFPSuppression, TKind)); const MemRegion *RegionRVal = RVal.getAsRegion(); if (RegionRVal && isa(RegionRVal)) { @@ -2034,45 +2089,6 @@ } //===----------------------------------------------------------------------===// -// Implementation of FindLastStoreBRVisitor. -//===----------------------------------------------------------------------===// - -// Registers every VarDecl inside a Stmt with a last store visitor. -void FindLastStoreBRVisitor::registerStatementVarDecls(BugReport &BR, - const Stmt *S, - bool EnableNullFPSuppression) { - const ExplodedNode *N = BR.getErrorNode(); - std::deque WorkList; - WorkList.push_back(S); - - while (!WorkList.empty()) { - const Stmt *Head = WorkList.front(); - WorkList.pop_front(); - - ProgramStateManager &StateMgr = N->getState()->getStateManager(); - - if (const auto *DR = dyn_cast(Head)) { - if (const auto *VD = dyn_cast(DR->getDecl())) { - const VarRegion *R = - StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext()); - - // What did we load? - SVal V = N->getSVal(S); - - if (V.getAs() || V.getAs()) { - // Register a new visitor with the BugReport. - BR.addVisitor(llvm::make_unique( - V.castAs(), R, EnableNullFPSuppression)); - } - } - } - - for (const Stmt *SubStmt : Head->children()) - WorkList.push_back(SubStmt); - } -} - -//===----------------------------------------------------------------------===// // Visitor that tries to report interesting diagnostics from conditions. //===----------------------------------------------------------------------===//