diff options
-rw-r--r-- | include/clang/AST/ParentMap.h | 7 | ||||
-rw-r--r-- | include/clang/Analysis/PathSensitive/GRExprEngine.h | 2 | ||||
-rw-r--r-- | lib/AST/ParentMap.cpp | 40 | ||||
-rw-r--r-- | lib/Analysis/CheckDeadStores.cpp | 46 | ||||
-rw-r--r-- | lib/Analysis/GRExprEngine.cpp | 5 | ||||
-rw-r--r-- | test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m | 6 |
6 files changed, 60 insertions, 46 deletions
diff --git a/include/clang/AST/ParentMap.h b/include/clang/AST/ParentMap.h index 94890e3feb..199a360ca0 100644 --- a/include/clang/AST/ParentMap.h +++ b/include/clang/AST/ParentMap.h @@ -16,6 +16,7 @@ namespace clang { class Stmt; +class Expr; class ParentMap { void* Impl; @@ -32,6 +33,12 @@ public: bool hasParent(Stmt* S) const { return getParent(S) != 0; } + + bool isConsumedExpr(Expr *E) const; + + bool isConsumedExpr(const Expr *E) const { + return isConsumedExpr(const_cast<Expr*>(E)); + } }; } // end clang namespace diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index 1dee5d267a..f53a9abaad 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -98,7 +98,7 @@ protected: // to lazily evaluate such logic. The downside is that it eagerly // bifurcates paths. const bool EagerlyAssume; - + public: typedef llvm::SmallPtrSet<NodeTy*,2> ErrorNodes; typedef llvm::DenseMap<NodeTy*, Expr*> UndefArgsTy; diff --git a/lib/AST/ParentMap.cpp b/lib/AST/ParentMap.cpp index 54472ecb96..877b43b6de 100644 --- a/lib/AST/ParentMap.cpp +++ b/lib/AST/ParentMap.cpp @@ -45,3 +45,43 @@ Stmt* ParentMap::getParent(Stmt* S) const { MapTy::iterator I = M->find(S); return I == M->end() ? 0 : I->second; } + +bool ParentMap::isConsumedExpr(Expr* E) const { + Stmt *P = getParent(E); + Stmt *DirectChild = E; + + // Ignore parents that are parentheses or casts. + while (P && (isa<ParenExpr>(E) || isa<CastExpr>(E))) { + DirectChild = P; + P = getParent(P); + } + + if (!P) + return false; + + switch (P->getStmtClass()) { + default: + return isa<Expr>(P); + case Stmt::DeclStmtClass: + return true; + case Stmt::BinaryOperatorClass: { + BinaryOperator *BE = cast<BinaryOperator>(P); + return BE->getOpcode()==BinaryOperator::Comma && DirectChild==BE->getLHS(); + } + case Stmt::ForStmtClass: + return DirectChild == cast<ForStmt>(P)->getCond(); + case Stmt::WhileStmtClass: + return DirectChild == cast<WhileStmt>(P)->getCond(); + case Stmt::DoStmtClass: + return DirectChild == cast<DoStmt>(P)->getCond(); + case Stmt::IfStmtClass: + return DirectChild == cast<IfStmt>(P)->getCond(); + case Stmt::IndirectGotoStmtClass: + return DirectChild == cast<IndirectGotoStmt>(P)->getTarget(); + case Stmt::SwitchStmtClass: + return DirectChild == cast<SwitchStmt>(P)->getCond(); + case Stmt::ReturnStmtClass: + return true; + } +} + diff --git a/lib/Analysis/CheckDeadStores.cpp b/lib/Analysis/CheckDeadStores.cpp index 504de3c97d..d1b3be511a 100644 --- a/lib/Analysis/CheckDeadStores.cpp +++ b/lib/Analysis/CheckDeadStores.cpp @@ -38,10 +38,7 @@ public: : Ctx(ctx), BR(br), Parents(parents) {} virtual ~DeadStoreObs() {} - - bool isConsumedExpr(Expr* E) const; - - + void Report(VarDecl* V, DeadStoreKind dsk, SourceLocation L, SourceRange R) { std::string name = V->getNameAsString(); @@ -148,7 +145,7 @@ public: return; // Otherwise, issue a warning. - DeadStoreKind dsk = isConsumedExpr(B) + DeadStoreKind dsk = Parents.isConsumedExpr(B) ? Enclosing : (isIncrement(VD,B) ? DeadIncrement : Standard); @@ -163,7 +160,7 @@ public: // about preincrements to dead variables when the preincrement occurs // as a subexpression. This can lead to false negatives, e.g. "(++x);" // A generalized dead code checker should find such issues. - if (U->isPrefix() && isConsumedExpr(U)) + if (U->isPrefix() && Parents.isConsumedExpr(U)) return; Expr *Ex = U->getSubExpr()->IgnoreParenCasts(); @@ -218,43 +215,6 @@ public: } // end anonymous namespace -bool DeadStoreObs::isConsumedExpr(Expr* E) const { - Stmt *P = Parents.getParent(E); - Stmt *DirectChild = E; - - // Ignore parents that are parentheses or casts. - while (P && (isa<ParenExpr>(E) || isa<CastExpr>(E))) { - DirectChild = P; - P = Parents.getParent(P); - } - - if (!P) - return false; - - switch (P->getStmtClass()) { - default: - return isa<Expr>(P); - case Stmt::BinaryOperatorClass: { - BinaryOperator *BE = cast<BinaryOperator>(P); - return BE->getOpcode()==BinaryOperator::Comma && DirectChild==BE->getLHS(); - } - case Stmt::ForStmtClass: - return DirectChild == cast<ForStmt>(P)->getCond(); - case Stmt::WhileStmtClass: - return DirectChild == cast<WhileStmt>(P)->getCond(); - case Stmt::DoStmtClass: - return DirectChild == cast<DoStmt>(P)->getCond(); - case Stmt::IfStmtClass: - return DirectChild == cast<IfStmt>(P)->getCond(); - case Stmt::IndirectGotoStmtClass: - return DirectChild == cast<IndirectGotoStmt>(P)->getTarget(); - case Stmt::SwitchStmtClass: - return DirectChild == cast<SwitchStmt>(P)->getCond(); - case Stmt::ReturnStmtClass: - return true; - } -} - //===----------------------------------------------------------------------===// // Driver function to invoke the Dead-Stores checker on a CFG. //===----------------------------------------------------------------------===// diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 2b5808f562..0e3d3ad2d4 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -13,9 +13,9 @@ // //===----------------------------------------------------------------------===// +#include "clang/AST/ParentMap.h" #include "clang/Analysis/PathSensitive/GRExprEngine.h" #include "clang/Analysis/PathSensitive/GRExprEngineBuilders.h" - #include "clang/Analysis/PathSensitive/BugReporter.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/PrettyStackTrace.h" @@ -1690,7 +1690,8 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME, if (isFeasibleNull) { // Check if the receiver was nil and the return value a struct. - if (ME->getType()->isRecordType()) { + if (ME->getType()->isRecordType() && + BR.getParentMap().isConsumedExpr(ME)) { // The [0 ...] expressions will return garbage. Flag either an // explicit or implicit error. Because of the structure of this // function we currently do not bifurfacte the state graph at diff --git a/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m b/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m index d1f0802abf..5d1fa37c46 100644 --- a/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m +++ b/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m @@ -17,3 +17,9 @@ void createFoo() { Bar f = [obj foo]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value (of type 'Bar') to be garbage or otherwise undefined.}} } +void createFoo2() { + MyClass *obj = 0; + [obj foo]; // no-warning + Bar f = [obj foo]; // expected-warning{{The receiver in the message expression is 'nil' and results in the returned value (of type 'Bar') to be garbage or otherwise undefined.}} +} + |