diff options
3 files changed, 80 insertions, 19 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index c1b5594209..17c1009853 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -55,8 +55,8 @@ public: /// /// The last parameter can be used to register a new visitor with the given /// BugReport while processing a node. - virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *N, - const ExplodedNode *PrevN, + virtual PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, BugReporterContext &BRC, BugReport &BR) = 0; @@ -283,13 +283,20 @@ public: class SuppressInlineDefensiveChecksVisitor : public BugReporterVisitorImpl<SuppressInlineDefensiveChecksVisitor> { - // The symbolic value for which we are tracking constraints. - // This value is constrained to null in the end of path. + /// The symbolic value for which we are tracking constraints. + /// This value is constrained to null in the end of path. DefinedSVal V; - // Track if we found the node where the constraint was first added. + /// Track if we found the node where the constraint was first added. bool IsSatisfied; + /// \brief The node from which we should start tracking the value. + /// Note: Since the visitors can be registered on nodes previous to the last + /// node in the BugReport, but the path traversal always starts with the last + /// node, the visitor invariant (that we start with a node in which V is null) + /// might not hold when node visitation starts. + const ExplodedNode *StartN; + public: SuppressInlineDefensiveChecksVisitor(DefinedSVal Val, const ExplodedNode *N); @@ -299,8 +306,12 @@ public: /// to make all PathDiagnosticPieces created by this visitor. static const char *getTag(); - PathDiagnosticPiece *VisitNode(const ExplodedNode *N, - const ExplodedNode *PrevN, + PathDiagnosticPiece *getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR); + + PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, BugReporterContext &BRC, BugReport &BR); }; diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 4b6dd09021..88e4ba5876 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -695,7 +695,7 @@ TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N, SuppressInlineDefensiveChecksVisitor:: SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N) - : V(Value), IsSatisfied(false) { + : V(Value), IsSatisfied(false), StartN(N) { assert(N->getState()->isNull(V).isConstrainedTrue() && "The visitor only tracks the cases where V is constrained to 0"); @@ -704,6 +704,7 @@ SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N) void SuppressInlineDefensiveChecksVisitor::Profile(FoldingSetNodeID &ID) const { static int id = 0; ID.AddPointer(&id); + ID.AddPointer(StartN); ID.Add(V); } @@ -712,13 +713,28 @@ const char *SuppressInlineDefensiveChecksVisitor::getTag() { } PathDiagnosticPiece * -SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *N, - const ExplodedNode *PrevN, +SuppressInlineDefensiveChecksVisitor::getEndPath(BugReporterContext &BRC, + const ExplodedNode *N, + BugReport &BR) { + if (StartN == BR.getErrorNode()) + StartN = 0; + return 0; +} + +PathDiagnosticPiece * +SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ, + const ExplodedNode *Pred, BugReporterContext &BRC, BugReport &BR) { if (IsSatisfied) return 0; + // Start tracking after we see node StartN. + if (StartN == Succ) + StartN = 0; + if (StartN) + return 0; + AnalyzerOptions &Options = BRC.getBugReporter().getEngine().getAnalysisManager().options; if (!Options.shouldSuppressInlinedDefensiveChecks()) @@ -726,14 +742,13 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *N, // Check if in the previous state it was feasible for this value // to *not* be null. - if (PrevN->getState()->assume(V, true)) { + if (Pred->getState()->assume(V, true)) { IsSatisfied = true; - // TODO: Investigate if missing the transition point, where V - // is non-null in N could lead to false negatives. + assert(!Succ->getState()->assume(V, true)); - // Check if this is inline defensive checks. - const LocationContext *CurLC = N->getLocationContext(); + // Check if this is inlined defensive checks. + const LocationContext *CurLC =Succ->getLocationContext(); const LocationContext *ReportLC = BR.getErrorNode()->getLocationContext(); if (CurLC != ReportLC && !CurLC->isParentOf(ReportLC)) BR.markInvalid("Suppress IDC", CurLC); @@ -741,14 +756,17 @@ SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *N, return 0; } -bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, +bool bugreporter::trackNullOrUndefValue(const ExplodedNode *ErrorNode, + const Stmt *S, BugReport &report, bool IsArg) { - if (!S || !N) + if (!S || !ErrorNode) return false; if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(S)) S = OVE->getSourceExpr(); + const ExplodedNode *N = ErrorNode; + const Expr *LValue = 0; if (const Expr *Ex = dyn_cast<Expr>(S)) { Ex = Ex->IgnoreParenCasts(); @@ -850,10 +868,10 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, const Stmt *S, report.addVisitor(ConstraintTracker); // Add visitor, which will suppress inline defensive checks. - if (N->getState()->isNull(V).isConstrainedTrue()) { + if (ErrorNode->getState()->isNull(V).isConstrainedTrue()) { BugReporterVisitor *IDCSuppressor = new SuppressInlineDefensiveChecksVisitor(V.castAs<DefinedSVal>(), - N); + ErrorNode); report.addVisitor(IDCSuppressor); } } diff --git a/test/Analysis/inlining/inline-defensive-checks.cpp b/test/Analysis/inlining/inline-defensive-checks.cpp new file mode 100644 index 0000000000..c77961d03c --- /dev/null +++ b/test/Analysis/inlining/inline-defensive-checks.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s +// expected-no-diagnostics + +class ButterFly { +private: + ButterFly() { } +public: + int triggerderef() { + return 0; + } +}; +ButterFly *getInP(); +class X{ + ButterFly *p; + void setP(ButterFly *inP) { + if(inP) + ; + p = inP; + }; + void subtest1() { + ButterFly *inP = getInP(); + setP(inP); + } + int subtest2() { + int c = p->triggerderef(); // no-warning + return c; + } + int test() { + subtest1(); + return subtest2(); + } +};
\ No newline at end of file |