[cfe-commits] r89745 - in /cfe/trunk: include/clang/Analysis/PathSensitive/Checker.h include/clang/Analysis/PathSensitive/GRExprEngine.h lib/Analysis/CFRefCount.cpp lib/Analysis/CallAndMessageChecker.cpp lib/Analysis/GRExprEngine.cpp lib/Analysis/GRExprEngineInternalChecks.cpp
Zhongxing Xu
xuzhongxing at gmail.com
Mon Nov 23 23:06:39 PST 2009
Author: zhongxingxu
Date: Tue Nov 24 01:06:39 2009
New Revision: 89745
URL: http://llvm.org/viewvc/llvm-project?rev=89745&view=rev
Log:
Refactor NilReceiverStructRet and NilReceiverLargerThanVoidPtrRet into
CallAndMessageChecker.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h
cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h
cfe/trunk/lib/Analysis/CFRefCount.cpp
cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp
cfe/trunk/lib/Analysis/GRExprEngine.cpp
cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h?rev=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/Checker.h Tue Nov 24 01:06:39 2009
@@ -81,6 +81,10 @@
return getBugReporter().getSourceManager();
}
+ ValueManager &getValueManager() {
+ return Eng.getValueManager();
+ }
+
ExplodedNode *GenerateNode(bool autoTransition = true) {
assert(statement && "Only transitions with statements currently supported");
ExplodedNode *N = GenerateNodeImpl(statement, getState(), false);
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=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/GRExprEngine.h Tue Nov 24 01:06:39 2009
@@ -90,38 +90,6 @@
public:
typedef llvm::SmallPtrSet<ExplodedNode*,2> ErrorNodes;
- /// NilReceiverStructRetExplicit - Nodes in the ExplodedGraph that resulted
- /// from [x ...] with 'x' definitely being nil and the result was a 'struct'
- // (an undefined value).
- ErrorNodes NilReceiverStructRetExplicit;
-
- /// NilReceiverStructRetImplicit - Nodes in the ExplodedGraph that resulted
- /// from [x ...] with 'x' possibly being nil and the result was a 'struct'
- // (an undefined value).
- ErrorNodes NilReceiverStructRetImplicit;
-
- /// NilReceiverLargerThanVoidPtrRetExplicit - Nodes in the ExplodedGraph that
- /// resulted from [x ...] with 'x' definitely being nil and the result's size
- // was larger than sizeof(void *) (an undefined value).
- ErrorNodes NilReceiverLargerThanVoidPtrRetExplicit;
-
- /// NilReceiverLargerThanVoidPtrRetImplicit - Nodes in the ExplodedGraph that
- /// resulted from [x ...] with 'x' possibly being nil and the result's size
- // was larger than sizeof(void *) (an undefined value).
- ErrorNodes NilReceiverLargerThanVoidPtrRetImplicit;
-
- /// UndefBranches - Nodes in the ExplodedGraph that result from
- /// taking a branch based on an undefined value.
- ErrorNodes UndefBranches;
-
- /// UndefStores - Sinks in the ExplodedGraph that result from
- /// making a store to an undefined lvalue.
- ErrorNodes UndefStores;
-
- /// NoReturnCalls - Sinks in the ExplodedGraph that result from
- // calling a function with the attribute "noreturn".
- ErrorNodes NoReturnCalls;
-
/// UndefResults - Nodes in the ExplodedGraph where the operands are defined
/// by the result is not. Excludes divide-by-zero errors.
ErrorNodes UndefResults;
@@ -185,36 +153,6 @@
return static_cast<CHECKER*>(lookupChecker(CHECKER::getTag()));
}
- bool isNoReturnCall(const ExplodedNode* N) const {
- return N->isSink() && NoReturnCalls.count(const_cast<ExplodedNode*>(N)) != 0;
- }
-
- typedef ErrorNodes::iterator undef_branch_iterator;
- undef_branch_iterator undef_branches_begin() { return UndefBranches.begin(); }
- undef_branch_iterator undef_branches_end() { return UndefBranches.end(); }
-
- typedef ErrorNodes::iterator nil_receiver_struct_ret_iterator;
-
- nil_receiver_struct_ret_iterator nil_receiver_struct_ret_begin() {
- return NilReceiverStructRetExplicit.begin();
- }
-
- nil_receiver_struct_ret_iterator nil_receiver_struct_ret_end() {
- return NilReceiverStructRetExplicit.end();
- }
-
- typedef ErrorNodes::iterator nil_receiver_larger_than_voidptr_ret_iterator;
-
- nil_receiver_larger_than_voidptr_ret_iterator
- nil_receiver_larger_than_voidptr_ret_begin() {
- return NilReceiverLargerThanVoidPtrRetExplicit.begin();
- }
-
- nil_receiver_larger_than_voidptr_ret_iterator
- nil_receiver_larger_than_voidptr_ret_end() {
- return NilReceiverLargerThanVoidPtrRetExplicit.end();
- }
-
typedef ErrorNodes::iterator undef_result_iterator;
undef_result_iterator undef_results_begin() { return UndefResults.begin(); }
undef_result_iterator undef_results_end() { return UndefResults.end(); }
Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Tue Nov 24 01:06:39 2009
@@ -3066,6 +3066,16 @@
GRStmtNodeBuilder& Builder,
ObjCMessageExpr* ME,
ExplodedNode* Pred) {
+ // FIXME: Since we moved the nil check into a checker, we could get nil
+ // receiver here. Need a better way to check such case.
+ if (Expr* Receiver = ME->getReceiver()) {
+ const GRState *state = Pred->getState();
+ DefinedOrUnknownSVal L=cast<DefinedOrUnknownSVal>(state->getSVal(Receiver));
+ if (!state->Assume(L, true)) {
+ Dst.Add(Pred);
+ return;
+ }
+ }
RetainSummary *Summ =
ME->getReceiver()
Modified: cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp?rev=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp (original)
+++ cfe/trunk/lib/Analysis/CallAndMessageChecker.cpp Tue Nov 24 01:06:39 2009
@@ -14,6 +14,7 @@
#include "clang/Analysis/PathSensitive/CheckerVisitor.h"
#include "clang/Analysis/PathSensitive/BugReporter.h"
+#include "clang/AST/ParentMap.h"
#include "GRExprEngineInternalChecks.h"
using namespace clang;
@@ -26,10 +27,13 @@
BugType *BT_call_arg;
BugType *BT_msg_undef;
BugType *BT_msg_arg;
+ BugType *BT_struct_ret;
+ BugType *BT_void_ptr;
public:
CallAndMessageChecker() :
BT_call_null(0), BT_call_undef(0), BT_call_arg(0),
- BT_msg_undef(0), BT_msg_arg(0) {}
+ BT_msg_undef(0), BT_msg_arg(0), BT_struct_ret(0), BT_void_ptr(0) {}
+
static void *getTag() {
static int x = 0;
return &x;
@@ -119,8 +123,8 @@
}
// Check for any arguments that are uninitialized/undefined.
- for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(), E = ME->arg_end();
- I != E; ++I) {
+ for (ObjCMessageExpr::const_arg_iterator I = ME->arg_begin(),
+ E = ME->arg_end(); I != E; ++I) {
if (state->getSVal(*I).isUndef()) {
if (ExplodedNode *N = C.GenerateSink()) {
if (!BT_msg_arg)
@@ -137,4 +141,112 @@
}
}
}
+
+ // Check if the receiver was nil and then return value a struct.
+ if (const Expr *Receiver = ME->getReceiver()) {
+ SVal L_untested = state->getSVal(Receiver);
+ // Assume that the receiver is not NULL.
+ DefinedOrUnknownSVal L = cast<DefinedOrUnknownSVal>(L_untested);
+ const GRState *StNotNull = state->Assume(L, true);
+
+ // Assume that the receiver is NULL.
+ const GRState *StNull = state->Assume(L, false);
+
+ if (StNull) {
+ QualType RetTy = ME->getType();
+ if (RetTy->isRecordType()) {
+ if (C.getPredecessor()->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
+ // this point.
+ // FIXME: We should bifurcate and fill the returned struct with
+ // garbage.
+ if (ExplodedNode* N = C.GenerateSink(StNull)) {
+ if (!StNotNull) {
+ if (!BT_struct_ret) {
+ std::string sbuf;
+ llvm::raw_string_ostream os(sbuf);
+ os << "The receiver in the message expression is 'nil' and "
+ "results in the returned value (of type '"
+ << ME->getType().getAsString()
+ << "') to be garbage or otherwise undefined";
+ BT_struct_ret = new BuiltinBug(os.str().c_str());
+ }
+
+ EnhancedBugReport *R = new EnhancedBugReport(*BT_struct_ret,
+ BT_struct_ret->getName(), N);
+ R->addRange(Receiver->getSourceRange());
+ R->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
+ Receiver);
+ C.EmitReport(R);
+ return;
+ }
+ else
+ // Do not report implicit bug.
+ return;
+ }
+ }
+ } else {
+ ASTContext &Ctx = C.getASTContext();
+ if (RetTy != Ctx.VoidTy) {
+ if (C.getPredecessor()->getParentMap().isConsumedExpr(ME)) {
+ // sizeof(void *)
+ const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
+ // sizeof(return type)
+ const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
+
+ if (voidPtrSize < returnTypeSize) {
+ if (ExplodedNode* N = C.GenerateSink(StNull)) {
+ if (!StNotNull) {
+ if (!BT_struct_ret) {
+ std::string sbuf;
+ llvm::raw_string_ostream os(sbuf);
+ os << "The receiver in the message expression is 'nil' and "
+ "results in the returned value (of type '"
+ << ME->getType().getAsString()
+ << "' and of size "
+ << returnTypeSize / 8
+ << " bytes) to be garbage or otherwise undefined";
+ BT_void_ptr = new BuiltinBug(os.str().c_str());
+ }
+
+ EnhancedBugReport *R = new EnhancedBugReport(*BT_void_ptr,
+ BT_void_ptr->getName(), N);
+ R->addRange(Receiver->getSourceRange());
+ R->addVisitorCreator(
+ bugreporter::registerTrackNullOrUndefValue, Receiver);
+ C.EmitReport(R);
+ return;
+ } else
+ // Do not report implicit bug.
+ return;
+ }
+ }
+ else if (!StNotNull) {
+ // Handle the safe cases where the return value is 0 if the
+ // receiver is nil.
+ //
+ // FIXME: For now take the conservative approach that we only
+ // return null values if we *know* that the receiver is nil.
+ // This is because we can have surprises like:
+ //
+ // ... = [[NSScreens screens] objectAtIndex:0];
+ //
+ // What can happen is that [... screens] could return nil, but
+ // it most likely isn't nil. We should assume the semantics
+ // of this case unless we have *a lot* more knowledge.
+ //
+ SVal V = C.getValueManager().makeZeroVal(ME->getType());
+ C.GenerateNode(StNull->BindExpr(ME, V));
+ return;
+ }
+ }
+ }
+ }
+ }
+ // Do not propagate null state.
+ if (StNotNull)
+ C.GenerateNode(StNotNull);
+ }
}
Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Tue Nov 24 01:06:39 2009
@@ -860,8 +860,9 @@
if (isa<loc::ConcreteInt>(V) || isa<UndefinedVal>(V)) {
// Dispatch to the first target and mark it as a sink.
- ExplodedNode* N = builder.generateNode(builder.begin(), state, true);
- UndefBranches.insert(N);
+ //ExplodedNode* N = builder.generateNode(builder.begin(), state, true);
+ // FIXME: add checker visit.
+ // UndefBranches.insert(N);
return;
}
@@ -912,8 +913,10 @@
SVal CondV_untested = state->getSVal(CondE);
if (CondV_untested.isUndef()) {
- ExplodedNode* N = builder.generateDefaultCaseNode(state, true);
- UndefBranches.insert(N);
+ //ExplodedNode* N = builder.generateDefaultCaseNode(state, true);
+ // FIXME: add checker
+ //UndefBranches.insert(N);
+
return;
}
DefinedOrUnknownSVal CondV = cast<DefinedOrUnknownSVal>(CondV_untested);
@@ -1858,88 +1861,9 @@
for (ExplodedNodeSet::iterator DI = DstTmp.begin(), DE = DstTmp.end();
DI!=DE; ++DI) {
Pred = *DI;
- // FIXME: More logic for the processing the method call.
- const GRState* state = GetState(Pred);
bool RaisesException = false;
- if (Expr* Receiver = ME->getReceiver()) {
- SVal L_untested = state->getSVal(Receiver);
-
- // "Assume" that the receiver is not NULL.
- DefinedOrUnknownSVal L = cast<DefinedOrUnknownSVal>(L_untested);
- const GRState *StNotNull = state->Assume(L, true);
-
- // "Assume" that the receiver is NULL.
- const GRState *StNull = state->Assume(L, false);
-
- if (StNull) {
- QualType RetTy = ME->getType();
-
- // Check if the receiver was nil and the return value a struct.
- if (RetTy->isRecordType()) {
- if (Pred->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
- // this point.
- // FIXME: We should bifurcate and fill the returned struct with
- // garbage.
- if (ExplodedNode* N = Builder->generateNode(ME, StNull, Pred)) {
- N->markAsSink();
- if (StNotNull)
- NilReceiverStructRetImplicit.insert(N);
- else
- NilReceiverStructRetExplicit.insert(N);
- }
- }
- }
- else {
- ASTContext& Ctx = getContext();
- if (RetTy != Ctx.VoidTy) {
- if (Pred->getParentMap().isConsumedExpr(ME)) {
- // sizeof(void *)
- const uint64_t voidPtrSize = Ctx.getTypeSize(Ctx.VoidPtrTy);
- // sizeof(return type)
- const uint64_t returnTypeSize = Ctx.getTypeSize(ME->getType());
-
- if (voidPtrSize < returnTypeSize) {
- if (ExplodedNode* N = Builder->generateNode(ME, StNull, Pred)) {
- N->markAsSink();
- if (StNotNull)
- NilReceiverLargerThanVoidPtrRetImplicit.insert(N);
- else
- NilReceiverLargerThanVoidPtrRetExplicit.insert(N);
- }
- }
- else if (!StNotNull) {
- // Handle the safe cases where the return value is 0 if the
- // receiver is nil.
- //
- // FIXME: For now take the conservative approach that we only
- // return null values if we *know* that the receiver is nil.
- // This is because we can have surprises like:
- //
- // ... = [[NSScreens screens] objectAtIndex:0];
- //
- // What can happen is that [... screens] could return nil, but
- // it most likely isn't nil. We should assume the semantics
- // of this case unless we have *a lot* more knowledge.
- //
- SVal V = ValMgr.makeZeroVal(ME->getType());
- MakeNode(Dst, ME, Pred, StNull->BindExpr(ME, V));
- return;
- }
- }
- }
- }
- // We have handled the cases where the receiver is nil. The remainder
- // of this method should assume that the receiver is not nil.
- if (!StNotNull)
- return;
-
- state = StNotNull;
- }
-
+ if (ME->getReceiver()) {
// Check if the "raise" message was sent.
if (ME->getSelector() == RaiseSel)
RaisesException = true;
@@ -2840,11 +2764,10 @@
GraphPrintCheckerState->isBadCall(N) ||
GraphPrintCheckerState->isUndefArg(N))
return "color=\"red\",style=\"filled\"";
-#endif
if (GraphPrintCheckerState->isNoReturnCall(N))
return "color=\"blue\",style=\"filled\"";
-
+#endif
return "";
}
Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=89745&r1=89744&r2=89745&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Tue Nov 24 01:06:39 2009
@@ -62,72 +62,6 @@
GetNode(I)));
}
-class VISIBILITY_HIDDEN NilReceiverStructRet : public BuiltinBug {
-public:
- NilReceiverStructRet(GRExprEngine* eng) :
- BuiltinBug(eng, "'nil' receiver with struct return type") {}
-
- void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
- for (GRExprEngine::nil_receiver_struct_ret_iterator
- I=Eng.nil_receiver_struct_ret_begin(),
- E=Eng.nil_receiver_struct_ret_end(); I!=E; ++I) {
-
- std::string sbuf;
- llvm::raw_string_ostream os(sbuf);
- PostStmt P = cast<PostStmt>((*I)->getLocation());
- const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(P.getStmt());
- os << "The receiver in the message expression is 'nil' and results in the"
- " returned value (of type '"
- << ME->getType().getAsString()
- << "') to be garbage or otherwise undefined";
-
- BuiltinBugReport *R = new BuiltinBugReport(*this, os.str().c_str(), *I);
- R->addRange(ME->getReceiver()->getSourceRange());
- BR.EmitReport(R);
- }
- }
-
- void registerInitialVisitors(BugReporterContext& BRC,
- const ExplodedNode* N,
- BuiltinBugReport *R) {
- registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N);
- }
-};
-
-class VISIBILITY_HIDDEN NilReceiverLargerThanVoidPtrRet : public BuiltinBug {
-public:
- NilReceiverLargerThanVoidPtrRet(GRExprEngine* eng) :
- BuiltinBug(eng,
- "'nil' receiver with return type larger than sizeof(void *)") {}
-
- void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
- for (GRExprEngine::nil_receiver_larger_than_voidptr_ret_iterator
- I=Eng.nil_receiver_larger_than_voidptr_ret_begin(),
- E=Eng.nil_receiver_larger_than_voidptr_ret_end(); I!=E; ++I) {
-
- std::string sbuf;
- llvm::raw_string_ostream os(sbuf);
- PostStmt P = cast<PostStmt>((*I)->getLocation());
- const ObjCMessageExpr *ME = cast<ObjCMessageExpr>(P.getStmt());
- os << "The receiver in the message expression is 'nil' and results in the"
- " returned value (of type '"
- << ME->getType().getAsString()
- << "' and of size "
- << Eng.getContext().getTypeSize(ME->getType()) / 8
- << " bytes) to be garbage or otherwise undefined";
-
- BuiltinBugReport *R = new BuiltinBugReport(*this, os.str().c_str(), *I);
- R->addRange(ME->getReceiver()->getSourceRange());
- BR.EmitReport(R);
- }
- }
- void registerInitialVisitors(BugReporterContext& BRC,
- const ExplodedNode* N,
- BuiltinBugReport *R) {
- registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N);
- }
-};
-
class VISIBILITY_HIDDEN UndefResult : public BuiltinBug {
public:
UndefResult(GRExprEngine* eng)
@@ -217,8 +151,6 @@
// analyzing a function. Generation of BugReport objects is done via a call
// to 'FlushReports' from BugReporter.
BR.Register(new UndefResult(this));
- BR.Register(new NilReceiverStructRet(this));
- BR.Register(new NilReceiverLargerThanVoidPtrRet(this));
// The following checks do not need to have their associated BugTypes
// explicitly registered with the BugReporter. If they issue any BugReports,
More information about the cfe-commits
mailing list