diff options
5 files changed, 121 insertions, 88 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h index b2927a08b61..4ca38079d02 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -85,6 +85,40 @@ public: const BugReport &BR); }; +namespace bugreporter { + +/// Specifies the type of tracking for an expression. +enum class TrackingKind { + /// Default tracking kind -- specifies that as much information should be + /// gathered about the tracked expression value as possible. + Thorough, + /// Specifies that a more moderate tracking should be used for the expression + /// value. This will essentially make sure that functions relevant to the it + /// aren't pruned, but otherwise relies on the user reading the code or + /// following the arrows. + Condition +}; + +/// 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, + TrackingKind TKind = TrackingKind::Thorough, + bool EnableNullFPSuppression = true); + +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 { @@ -96,15 +130,28 @@ class FindLastStoreBRVisitor final : public BugReporterVisitor { /// 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; @@ -347,27 +394,6 @@ public: 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 diff --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp index 6e7776bb484..9cdc7612b29 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -282,7 +282,8 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const { N); R->addRange(RS->getSourceRange()); - bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false); + bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, + bugreporter::TrackingKind::Thorough, false); C.emitReport(std::move(R)); } diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp index f69a3944a56..98d2a9941da 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp @@ -146,8 +146,8 @@ void ObjCContainersChecker::checkPreStmt(const CallExpr *CE, initBugType(); auto R = llvm::make_unique<BugReport>(*BT, "Index is out of bounds", N); R->addRange(IdxExpr->getSourceRange()); - bugreporter::trackExpressionValue(N, IdxExpr, *R, - /*EnableNullFPSuppression=*/false); + bugreporter::trackExpressionValue( + N, IdxExpr, *R, bugreporter::TrackingKind::Thorough, false); C.emitReport(std::move(R)); return; } diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp index c787ef58660..1b6f19c01c2 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp @@ -87,7 +87,8 @@ UndefCapturedBlockVarChecker::checkPostStmt(const BlockExpr *BE, if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD)) R->addRange(Ex->getSourceRange()); R->addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *V, VR, /*EnableNullFPSuppression*/ false)); + *V, VR, /*EnableNullFPSuppression*/ false, + bugreporter::TrackingKind::Thorough)); R->disablePathPruning(); // need location of block C.emitReport(std::move(R)); diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index bd3296f7eae..21e488ff853 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -851,12 +851,13 @@ class ReturnVisitor : public BugReporterVisitor { bool EnableNullFPSuppression; bool ShouldInvalidate = true; AnalyzerOptions& Options; + bugreporter::TrackingKind TKind; public: ReturnVisitor(const StackFrameContext *Frame, bool Suppressed, - AnalyzerOptions &Options) + AnalyzerOptions &Options, bugreporter::TrackingKind TKind) : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed), - Options(Options) {} + Options(Options), TKind(TKind) {} static void *getTag() { static int Tag = 0; @@ -878,7 +879,8 @@ public: /// bug report, so it can print a note later. static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S, BugReport &BR, - bool InEnableNullFPSuppression) { + bool InEnableNullFPSuppression, + bugreporter::TrackingKind TKind) { if (!CallEvent::isCallStmt(S)) return; @@ -951,7 +953,7 @@ public: BR.addVisitor(llvm::make_unique<ReturnVisitor>(CalleeContext, EnableNullFPSuppression, - Options)); + Options, TKind)); } PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N, @@ -999,8 +1001,9 @@ public: RetE = RetE->IgnoreParenCasts(); - // If we're returning 0, we should track where that 0 came from. - bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression); + // Let's track the return value. + bugreporter::trackExpressionValue( + N, RetE, BR, TKind, EnableNullFPSuppression); // Build an appropriate message based on the return value. SmallString<64> Msg; @@ -1115,7 +1118,7 @@ public: if (!State->isNull(*ArgV).isConstrainedTrue()) continue; - if (bugreporter::trackExpressionValue(N, ArgE, BR, EnableNullFPSuppression)) + if (trackExpressionValue(N, ArgE, BR, TKind, EnableNullFPSuppression)) ShouldInvalidate = false; // If we /can't/ track the null pointer, we should err on the side of @@ -1159,9 +1162,45 @@ void FindLastStoreBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const { ID.AddPointer(&tag); ID.AddPointer(R); ID.Add(V); + ID.AddInteger(static_cast<int>(TKind)); ID.AddBoolean(EnableNullFPSuppression); } +void FindLastStoreBRVisitor::registerStatementVarDecls( + BugReport &BR, const Stmt *S, bool EnableNullFPSuppression, + TrackingKind TKind) { + + const ExplodedNode *N = BR.getErrorNode(); + std::deque<const Stmt *> 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<DeclRefExpr>(Head)) { + if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) { + const VarRegion *R = + StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext()); + + // What did we load? + SVal V = N->getSVal(S); + + if (V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) { + // Register a new visitor with the BugReport. + BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( + V.castAs<KnownSVal>(), 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) { @@ -1332,7 +1371,7 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, // should track the initializer expression. if (Optional<PostInitializer> PIP = Pred->getLocationAs<PostInitializer>()) { const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue(); - if (FieldReg && FieldReg == R) { + if (FieldReg == R) { StoreSite = Pred; InitE = PIP->getInitializer()->getInit(); } @@ -1397,8 +1436,8 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, if (!IsParam) InitE = InitE->IgnoreParenCasts(); - bugreporter::trackExpressionValue(StoreSite, InitE, BR, - EnableNullFPSuppression); + bugreporter::trackExpressionValue( + StoreSite, InitE, BR, TKind, EnableNullFPSuppression); } // Okay, we've found the binding. Emit an appropriate message. @@ -1426,7 +1465,7 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ, if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>()) BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *KV, OriginalR, EnableNullFPSuppression)); + *KV, OriginalR, EnableNullFPSuppression, TKind)); } } } @@ -1724,7 +1763,8 @@ PathDiagnosticPieceRef TrackControlDependencyCondBRVisitor::VisitNode( // expression, hence the BugReport level set. if (BR.addTrackedCondition(N)) { bugreporter::trackExpressionValue( - N, Condition, BR, /*EnableNullFPSuppression=*/false); + N, Condition, BR, bugreporter::TrackingKind::Condition, + /*EnableNullFPSuppression=*/false); return constructDebugPieceForTrackedCondition(Condition, N, BRC); } } @@ -1838,7 +1878,9 @@ static const ExplodedNode* findNodeForExpression(const ExplodedNode *N, bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, const Expr *E, BugReport &report, + bugreporter::TrackingKind TKind, bool EnableNullFPSuppression) { + if (!E || !InputNode) return false; @@ -1862,12 +1904,13 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, // At this point in the path, the receiver should be live since we are at the // message send expr. If it is nil, start tracking it. if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(Inner, LVNode)) - trackExpressionValue(LVNode, Receiver, report, EnableNullFPSuppression); + trackExpressionValue( + LVNode, Receiver, report, TKind, EnableNullFPSuppression); // Track the index if this is an array subscript. if (const auto *Arr = dyn_cast<ArraySubscriptExpr>(Inner)) trackExpressionValue( - LVNode, Arr->getIdx(), report, /*EnableNullFPSuppression*/ false); + LVNode, Arr->getIdx(), report, TKind, /*EnableNullFPSuppression*/false); // See if the expression we're interested refers to a variable. // If so, we can track both its contents and constraints on its value. @@ -1883,7 +1926,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, if (RR && !LVIsNull) if (auto KV = LVal.getAs<KnownSVal>()) report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *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. @@ -1920,7 +1963,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, if (auto KV = V.getAs<KnownSVal>()) report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *KV, R, EnableNullFPSuppression)); + *KV, R, EnableNullFPSuppression, TKind)); return true; } } @@ -1930,7 +1973,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, SVal V = LVState->getSValAsScalarOrLoc(Inner, LVNode->getLocationContext()); ReturnVisitor::addVisitorIfNecessary( - LVNode, Inner, report, EnableNullFPSuppression); + LVNode, Inner, report, EnableNullFPSuppression, TKind); // Is it a symbolic value? if (auto L = V.getAs<loc::MemRegionVal>()) { @@ -1959,7 +2002,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode, if (CanDereference) if (auto KV = RVal.getAs<KnownSVal>()) report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - *KV, L->getRegion(), EnableNullFPSuppression)); + *KV, L->getRegion(), EnableNullFPSuppression, TKind)); const MemRegion *RegionRVal = RVal.getAsRegion(); if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) { @@ -2017,8 +2060,9 @@ PathDiagnosticPieceRef NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, // The receiver was nil, and hence the method was skipped. // Register a BugReporterVisitor to issue a message telling us how // the receiver was null. - bugreporter::trackExpressionValue(N, Receiver, BR, - /*EnableNullFPSuppression*/ false); + bugreporter::trackExpressionValue( + N, Receiver, BR, bugreporter::TrackingKind::Thorough, + /*EnableNullFPSuppression*/ false); // Issue a message saying that the method was skipped. PathDiagnosticLocation L(Receiver, BRC.getSourceManager(), N->getLocationContext()); @@ -2026,45 +2070,6 @@ PathDiagnosticPieceRef NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, } //===----------------------------------------------------------------------===// -// 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<const Stmt *> 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<DeclRefExpr>(Head)) { - if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) { - const VarRegion *R = - StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext()); - - // What did we load? - SVal V = N->getSVal(S); - - if (V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) { - // Register a new visitor with the BugReport. - BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>( - V.castAs<KnownSVal>(), R, EnableNullFPSuppression)); - } - } - } - - for (const Stmt *SubStmt : Head->children()) - WorkList.push_back(SubStmt); - } -} - -//===----------------------------------------------------------------------===// // Visitor that tries to report interesting diagnostics from conditions. //===----------------------------------------------------------------------===// |
