[cfe-commits] r68195 - in /cfe/trunk: include/clang/AST/ParentMap.h include/clang/Analysis/PathSensitive/GRExprEngine.h lib/AST/ParentMap.cpp lib/Analysis/CheckDeadStores.cpp lib/Analysis/GRExprEngine.cpp test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
Ted Kremenek
kremenek at apple.com
Tue Mar 31 23:52:48 PDT 2009
Author: kremenek
Date: Wed Apr 1 01:52:48 2009
New Revision: 68195
URL: http://llvm.org/viewvc/llvm-project?rev=68195&view=rev
Log:
Fix: <rdar://problem/6740387>. Sending nil to an object that returns a struct
should only be an error if that value is consumed. This fix was largely
accomplished by moving 'isConsumedExpr' back to ParentMap.
Modified:
cfe/trunk/include/clang/AST/ParentMap.h
cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
cfe/trunk/lib/AST/ParentMap.cpp
cfe/trunk/lib/Analysis/CheckDeadStores.cpp
cfe/trunk/lib/Analysis/GRExprEngine.cpp
cfe/trunk/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
Modified: cfe/trunk/include/clang/AST/ParentMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ParentMap.h?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ParentMap.h (original)
+++ cfe/trunk/include/clang/AST/ParentMap.h Wed Apr 1 01:52:48 2009
@@ -16,6 +16,7 @@
namespace clang {
class Stmt;
+class Expr;
class ParentMap {
void* Impl;
@@ -32,6 +33,12 @@
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
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Wed Apr 1 01:52:48 2009
@@ -98,7 +98,7 @@
// 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;
Modified: cfe/trunk/lib/AST/ParentMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ParentMap.cpp?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ParentMap.cpp (original)
+++ cfe/trunk/lib/AST/ParentMap.cpp Wed Apr 1 01:52:48 2009
@@ -45,3 +45,43 @@
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;
+ }
+}
+
Modified: cfe/trunk/lib/Analysis/CheckDeadStores.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CheckDeadStores.cpp?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CheckDeadStores.cpp (original)
+++ cfe/trunk/lib/Analysis/CheckDeadStores.cpp Wed Apr 1 01:52:48 2009
@@ -38,10 +38,7 @@
: 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 @@
return;
// Otherwise, issue a warning.
- DeadStoreKind dsk = isConsumedExpr(B)
+ DeadStoreKind dsk = Parents.isConsumedExpr(B)
? Enclosing
: (isIncrement(VD,B) ? DeadIncrement : Standard);
@@ -163,7 +160,7 @@
// 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 @@
} // 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.
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Wed Apr 1 01:52:48 2009
@@ -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 @@
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
Modified: cfe/trunk/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m?rev=68195&r1=68194&r2=68195&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m (original)
+++ cfe/trunk/test/Analysis/rdar-6600344-nil-receiver-undefined-struct-ret.m Wed Apr 1 01:52:48 2009
@@ -17,3 +17,9 @@
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.}}
+}
+
More information about the cfe-commits
mailing list