diff options
| -rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h | 18 | ||||
| -rw-r--r-- | lib/StaticAnalyzer/Core/BugReporterVisitors.cpp | 39 | ||||
| -rw-r--r-- | test/Analysis/inlining/inline-defensive-checks.m | 16 |
3 files changed, 38 insertions, 35 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index 6eb5f259b3..2e5f207f4b 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -160,25 +160,11 @@ private: /// \brief Prints path notes when a message is sent to a nil receiver. class NilReceiverBRVisitor : public BugReporterVisitorImpl<NilReceiverBRVisitor> { - - /// \brief The reciever to track. If null, all receivers should be tracked. - const Expr *TrackedReceiver; - - /// If the visitor is tracking the value directly responsible for the - /// bug, we are going to employ false positive suppression. - bool EnableNullFPSuppression; - public: - NilReceiverBRVisitor(const Expr *InTrackedReceiver = 0, - bool InEnableNullFPSuppression = false): - TrackedReceiver(InTrackedReceiver), - EnableNullFPSuppression(InEnableNullFPSuppression) {} void Profile(llvm::FoldingSetNodeID &ID) const { static int x = 0; ID.AddPointer(&x); - ID.AddPointer(TrackedReceiver); - ID.AddBoolean(EnableNullFPSuppression); } PathDiagnosticPiece *VisitNode(const ExplodedNode *N, @@ -186,7 +172,9 @@ public: BugReporterContext &BRC, BugReport &BR); - static const Expr *getReceiver(const Stmt *S); + /// If the statement is a message send expression with nil receiver, returns + /// the receiver expression. Returns NULL otherwise. + static const Expr *getNilReceiver(const Stmt *S, const ExplodedNode *N); }; /// Visitor that tries to report interesting diagnostics from conditions. diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 11218c086b..241388d18f 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -824,12 +824,6 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, S = Ex; } - // The message send could be null if the receiver is null. - if (const Expr *Receiver = NilReceiverBRVisitor::getReceiver(S)) { - report.addVisitor(new NilReceiverBRVisitor(Receiver, - EnableNullFPSuppression)); - } - const Expr *Inner = 0; if (const Expr *Ex = dyn_cast<Expr>(S)) { Ex = Ex->IgnoreParenCasts(); @@ -862,7 +856,14 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, ProgramStateRef state = N->getState(); - // See if the expression we're interested refers to a variable. + // The message send could be nil due to the receiver being nil. + // 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(S, N)) + trackNullOrUndefValue(N, Receiver, report, IsArg, EnableNullFPSuppression); + + + // See if the expression we're interested refers to a variable. // If so, we can track both its contents and constraints on its value. if (Inner && ExplodedGraph::isInterestingLValueExpr(Inner)) { const MemRegion *R = 0; @@ -985,11 +986,18 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, return true; } -const Expr *NilReceiverBRVisitor::getReceiver(const Stmt *S) { +const Expr *NilReceiverBRVisitor::getNilReceiver(const Stmt *S, + const ExplodedNode *N) { const ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S); if (!ME) return 0; - return ME->getInstanceReceiver(); + if (const Expr *Receiver = ME->getInstanceReceiver()) { + ProgramStateRef state = N->getState(); + SVal V = state->getSVal(Receiver, N->getLocationContext()); + if (state->isNull(V).isConstrainedTrue()) + return Receiver; + } + return 0; } PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, @@ -1000,24 +1008,15 @@ PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N, if (!P) return 0; - const Expr *Receiver = getReceiver(P->getStmt()); + const Expr *Receiver = getNilReceiver(P->getStmt(), N); if (!Receiver) return 0; - // Are we tracking a different reciever? - if (TrackedReceiver && TrackedReceiver != Receiver) - return 0; - - ProgramStateRef state = N->getState(); - SVal V = state->getSVal(Receiver, N->getLocationContext()); - if (!state->isNull(V).isConstrainedTrue()) - return 0; - // 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::trackNullOrUndefValue(N, Receiver, BR, /*IsArg*/ false, - EnableNullFPSuppression); + /*EnableNullFPSuppression*/ false); // Issue a message saying that the method was skipped. PathDiagnosticLocation L(Receiver, BRC.getSourceManager(), N->getLocationContext()); diff --git a/test/Analysis/inlining/inline-defensive-checks.m b/test/Analysis/inlining/inline-defensive-checks.m index a757af3d2f..0404ee6df8 100644 --- a/test/Analysis/inlining/inline-defensive-checks.m +++ b/test/Analysis/inlining/inline-defensive-checks.m @@ -76,6 +76,22 @@ int testNilReceiver(Foo* fPtr) { return mem->x; // expected-warning {{Access to instance variable 'x' results in a dereference of a null pointer}} } +int suppressNilReceiverRetNullCond(Foo* fPtr) { + unsigned zero = 0; + fPtr = retInputOrNil(fPtr); + // On a path where fPtr is nzil, mem should be nil. + Foo *mem = [fPtr getFooPtr]; + return mem->x; +} + +int suppressNilReceiverRetNullCondCast(id fPtr) { + unsigned zero = 0; + fPtr = retInputOrNil(fPtr); + // On a path where fPtr is nzil, mem should be nil. + Foo *mem = ((id)([(Foo*)(fPtr) getFooPtr])); + return mem->x; +} + int dontSuppressNilReceiverRetNullCond(Foo* fPtr) { unsigned zero = 0; fPtr = retInputOrNil(fPtr); |