From 85ac9349ea8b72cb342d1f2adf4f0fd7005d29c7 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Fri, 15 May 2009 05:25:09 +0000 Subject: [PATCH] Cleanup internal checks bug reporting, allowing intermediate diagnostics to be generated for bad argument warnings, bad branches, etc. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@71838 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Analysis/GRExprEngineInternalChecks.cpp | 205 +++++++++++++------- 1 file changed, 134 insertions(+), 71 deletions(-) diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index 7690aa1a25..a298629529 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -39,8 +39,15 @@ ExplodedNode* GetNode(GRExprEngine::undef_arg_iterator I) { // Forward declarations for bug reporter visitors. //===----------------------------------------------------------------------===// +static const Stmt *GetDerefExpr(const ExplodedNode *N); +static const Stmt *GetReceiverExpr(const ExplodedNode *N); +static const Stmt *GetDenomExpr(const ExplodedNode *N); +static const Stmt *GetCalleeExpr(const ExplodedNode *N); +static const Stmt *GetRetValExpr(const ExplodedNode *N); + static void registerTrackNullOrUndefValue(BugReporterContext& BRC, - const ExplodedNode* N); + const Stmt *ValExpr, + const ExplodedNode* N); //===----------------------------------------------------------------------===// // Bug Descriptions. @@ -54,6 +61,10 @@ public: ExplodedNode *n) : RangedBugReport(bt, desc, n) {} + BuiltinBugReport(BugType& bt, const char *shortDesc, const char *desc, + ExplodedNode *n) + : RangedBugReport(bt, shortDesc, desc, n) {} + void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N); }; @@ -104,7 +115,7 @@ public: void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); + registerTrackNullOrUndefValue(BRC, GetDerefExpr(N), N); } }; @@ -136,7 +147,7 @@ public: void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); + registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N); } }; @@ -170,7 +181,7 @@ public: void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); + registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N); } }; @@ -186,7 +197,7 @@ public: void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); + registerTrackNullOrUndefValue(BRC, GetDerefExpr(N), N); } }; @@ -199,6 +210,12 @@ public: void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.explicit_bad_divides_begin(), Eng.explicit_bad_divides_end()); } + + void registerInitialVisitors(BugReporterContext& BRC, + const ExplodedNode* N, + BuiltinBugReport *R) { + registerTrackNullOrUndefValue(BRC, GetDenomExpr(N), N); + } }; class VISIBILITY_HIDDEN UndefResult : public BuiltinBug { @@ -220,10 +237,31 @@ public: void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { Emit(BR, Eng.bad_calls_begin(), Eng.bad_calls_end()); } + + void registerInitialVisitors(BugReporterContext& BRC, + const ExplodedNode* N, + BuiltinBugReport *R) { + registerTrackNullOrUndefValue(BRC, GetCalleeExpr(N), N); + } }; -class VISIBILITY_HIDDEN BadArg : public BuiltinBug { + +class VISIBILITY_HIDDEN ArgReport : public BuiltinBugReport { + const Stmt *Arg; public: + ArgReport(BugType& bt, const char* desc, ExplodedNode *n, + const Stmt *arg) + : BuiltinBugReport(bt, desc, n), Arg(arg) {} + + ArgReport(BugType& bt, const char *shortDesc, const char *desc, + ExplodedNode *n, const Stmt *arg) + : BuiltinBugReport(bt, shortDesc, desc, n), Arg(arg) {} + + const Stmt *getArg() const { return Arg; } +}; + +class VISIBILITY_HIDDEN BadArg : public BuiltinBug { +public: BadArg(GRExprEngine* eng) : BuiltinBug(eng,"Uninitialized argument", "Pass-by-value argument in function call is undefined.") {} @@ -234,12 +272,19 @@ public: for (GRExprEngine::UndefArgsTy::iterator I = Eng.undef_arg_begin(), E = Eng.undef_arg_end(); I!=E; ++I) { // Generate a report for this bug. - RangedBugReport *report = new RangedBugReport(*this, desc.c_str(), - I->first); + ArgReport *report = new ArgReport(*this, desc.c_str(), I->first, + I->second); report->addRange(I->second->getSourceRange()); BR.EmitReport(report); } } + + void registerInitialVisitors(BugReporterContext& BRC, + const ExplodedNode* N, + BuiltinBugReport *R) { + registerTrackNullOrUndefValue(BRC, static_cast(R)->getArg(), + N); + } }; class VISIBILITY_HIDDEN BadMsgExprArg : public BadArg { @@ -251,12 +296,12 @@ public: for (GRExprEngine::UndefArgsTy::iterator I=Eng.msg_expr_undef_arg_begin(), E = Eng.msg_expr_undef_arg_end(); I!=E; ++I) { // Generate a report for this bug. - BuiltinBugReport *report = new BuiltinBugReport(*this, desc.c_str(), - I->first); + ArgReport *report = new ArgReport(*this, desc.c_str(), I->first, + I->second); report->addRange(I->second->getSourceRange()); BR.EmitReport(report); } - } + } }; class VISIBILITY_HIDDEN BadReceiver : public BuiltinBug { @@ -279,12 +324,12 @@ public: BR.EmitReport(report); } } - + void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); - } + registerTrackNullOrUndefValue(BRC, GetReceiverExpr(N), N); + } }; class VISIBILITY_HIDDEN RetStack : public BuiltinBug { @@ -358,8 +403,8 @@ public: void registerInitialVisitors(BugReporterContext& BRC, const ExplodedNode* N, BuiltinBugReport *R) { - registerTrackNullOrUndefValue(BRC, N); - } + registerTrackNullOrUndefValue(BRC, GetRetValExpr(N), N); + } }; class VISIBILITY_HIDDEN UndefBranch : public BuiltinBug { @@ -423,11 +468,18 @@ public: FindUndefExpr FindIt(Eng.getStateManager(), St); Ex = FindIt.FindExpr(Ex); - RangedBugReport *R = new RangedBugReport(*this, desc.c_str(), *I); + ArgReport *R = new ArgReport(*this, desc.c_str(), *I, Ex); R->addRange(Ex->getSourceRange()); BR.EmitReport(R); } } + + void registerInitialVisitors(BugReporterContext& BRC, + const ExplodedNode* N, + BuiltinBugReport *R) { + registerTrackNullOrUndefValue(BRC, static_cast(R)->getArg(), + N); + } }; class VISIBILITY_HIDDEN OutOfBoundMemoryAccess : public BuiltinBug { @@ -481,13 +533,20 @@ public: << (isUndefined ? "garbage value for array size" : "has zero elements (undefined behavior)"); - RangedBugReport *report = new RangedBugReport(*this, - os_short.str().c_str(), - os.str().c_str(), N); + ArgReport *report = new ArgReport(*this, os_short.str().c_str(), + os.str().c_str(), N, SizeExpr); + report->addRange(SizeExpr->getSourceRange()); BR.EmitReport(report); } } + + void registerInitialVisitors(BugReporterContext& BRC, + const ExplodedNode* N, + BuiltinBugReport *R) { + registerTrackNullOrUndefValue(BRC, static_cast(R)->getArg(), + N); + } }; //===----------------------------------------------------------------------===// @@ -549,6 +608,47 @@ public: // Definitions for bug reporter visitors. //===----------------------------------------------------------------------===// +static const Stmt *GetDerefExpr(const ExplodedNode *N) { + // Pattern match for a few useful cases (do something smarter later): + // a[0], p->f, *p + const Stmt *S = N->getLocationAs()->getStmt(); + + if (const UnaryOperator *U = dyn_cast(S)) { + if (U->getOpcode() == UnaryOperator::Deref) + return U->getSubExpr()->IgnoreParenCasts(); + } + else if (const MemberExpr *ME = dyn_cast(S)) { + return ME->getBase()->IgnoreParenCasts(); + } + else if (const ArraySubscriptExpr *AE = dyn_cast(S)) { + // Retrieve the base for arrays since BasicStoreManager doesn't know how + // to reason about them. + return AE->getBase(); + } + + return NULL; +} + +static const Stmt *GetReceiverExpr(const ExplodedNode *N) { + const Stmt *S = N->getLocationAs()->getStmt(); + return cast(S)->getReceiver(); +} + +static const Stmt *GetDenomExpr(const ExplodedNode *N) { + const Stmt *S = N->getLocationAs()->getStmt(); + return cast(S)->getRHS(); +} + +static const Stmt *GetCalleeExpr(const ExplodedNode *N) { + const Stmt *S = N->getLocationAs()->getStmt(); + return cast(S)->getCallee(); +} + +static const Stmt *GetRetValExpr(const ExplodedNode *N) { + const Stmt *S = N->getLocationAs()->getStmt(); + return cast(S)->getRetValue(); +} + namespace { class VISIBILITY_HIDDEN FindLastStoreBRVisitor : public BugReporterVisitor { const MemRegion *R; @@ -624,6 +724,9 @@ public: if (!b) os << "initialized to a null pointer value"; } + else if (isa(V)) { + os << "initialized to " << cast(V).getValue(); + } else if (V.isUndef()) { if (isa(R)) { const VarDecl *VD = cast(DS->getSingleDecl()); @@ -635,7 +738,7 @@ public: } } } - + if (os.str().empty()) { if (isa(V)) { bool b = false; @@ -734,7 +837,7 @@ public: if (isa(Constraint)) { os << "Assuming pointer value is "; - os << (Assumption ? "non-NULL" : "NULL"); + os << (Assumption ? "non-null" : "null"); } if (os.str().empty()) @@ -771,65 +874,29 @@ static void registerTrackConstraint(BugReporterContext& BRC, SVal Constraint, } static void registerTrackNullOrUndefValue(BugReporterContext& BRC, - const ExplodedNode* N) { + const Stmt *S, + const ExplodedNode* N) { - ProgramPoint P = N->getLocation(); - PostStmt *PS = dyn_cast(&P); - - if (!PS) + if (!S) return; - - Stmt *S = PS->getStmt(); + GRStateManager &StateMgr = BRC.getStateManager(); - const GRState *state = N->getState(); + const GRState *state = N->getState(); - // Pattern match for a few useful cases (do something smarter later): - // a[0], p->f, *p - const DeclRefExpr *DR = 0; - - if (const UnaryOperator *U = dyn_cast(S)) { - if (U->getOpcode() == UnaryOperator::Deref) - DR = dyn_cast(U->getSubExpr()->IgnoreParenCasts()); - } - else if (const MemberExpr *ME = dyn_cast(S)) { - DR = dyn_cast(ME->getBase()->IgnoreParenCasts()); - } - else if (const ObjCMessageExpr *MSE = dyn_cast(S)) { - // FIXME: We should probably distinguish between cases where we had - // a nil receiver and null dereferences. - const Expr *Receiver = MSE->getReceiver(); - if (Receiver) { - DR = dyn_cast(Receiver->IgnoreParenCasts()); - } - } - else if (const ReturnStmt *RS = dyn_cast(S)) { - if (const Expr *Ret = RS->getRetValue()) - DR = dyn_cast(Ret->IgnoreParenCasts()); - } - else if (const Expr *Ex = dyn_cast(S)) { - // Keep this case last. - DR = dyn_cast(Ex->IgnoreParenCasts()); - } - - if (DR) { + if (const DeclRefExpr *DR = dyn_cast(S)) { if (const VarDecl *VD = dyn_cast(DR->getDecl())) { const VarRegion *R = StateMgr.getRegionManager().getVarRegion(VD); // What did we load? - SVal V = StateMgr.GetSVal(state, R); + SVal V = StateMgr.GetSVal(state, S); - if (isa(V) || V.isUndef()) { + if (isa(V) || isa(V) + || V.isUndef()) { registerFindLastStore(BRC, R, V); } } } - - // Retrieve the base for arrays since BasicStoreManager doesn't know how - // to reason about them. - if (ArraySubscriptExpr *AE = dyn_cast(S)) { - S = AE->getBase(); - } SVal V = StateMgr.GetSValAsScalarOrLoc(state, S); @@ -849,10 +916,6 @@ static void registerTrackNullOrUndefValue(BugReporterContext& BRC, registerTrackConstraint(BRC, loc::MemRegionVal(R), false); } } - - // Was it a hard integer? -// if (isa(V)) -// registerTrackValue(BRC, S, V, N); } //===----------------------------------------------------------------------===// -- 2.40.0