From: Zhongxing Xu Date: Tue, 24 Nov 2009 08:24:26 +0000 (+0000) Subject: Refactor undefined result checker. This is the last one. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=668399b23cf18124c8c8c41a14a7712212f6fa45;p=clang Refactor undefined result checker. This is the last one. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@89750 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/PathSensitive/CheckerVisitor.def b/include/clang/Analysis/PathSensitive/CheckerVisitor.def index 090a5d3975..96e0d0c97b 100644 --- a/include/clang/Analysis/PathSensitive/CheckerVisitor.def +++ b/include/clang/Analysis/PathSensitive/CheckerVisitor.def @@ -24,6 +24,7 @@ PREVISIT(ReturnStmt) #ifdef POSTVISIT POSTVISIT(CallExpr) +POSTVISIT(BinaryOperator) #undef POSTVISIT #endif diff --git a/lib/Analysis/CMakeLists.txt b/lib/Analysis/CMakeLists.txt index d2ba5421b4..45fa9f40dd 100644 --- a/lib/Analysis/CMakeLists.txt +++ b/lib/Analysis/CMakeLists.txt @@ -56,6 +56,7 @@ add_clang_library(clangAnalysis Store.cpp SymbolManager.cpp UndefBranchChecker.cpp + UndefResultChecker.cpp UndefinedArraySubscriptChecker.cpp UndefinedAssignmentChecker.cpp UninitializedValues.cpp diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 2f7ea944e7..a301f2549d 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -13,6 +13,7 @@ // //===----------------------------------------------------------------------===// +#include "GRExprEngineInternalChecks.h" #include "clang/Analysis/PathSensitive/GRExprEngine.h" #include "clang/Analysis/PathSensitive/GRExprEngineBuilders.h" #include "clang/Analysis/PathSensitive/Checker.h" @@ -2576,6 +2577,8 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, else Visit(LHS, Pred, Tmp1); + ExplodedNodeSet Tmp3; + for (ExplodedNodeSet::iterator I1=Tmp1.begin(), E1=Tmp1.end(); I1!=E1; ++I1) { SVal LeftV = (*I1)->getState()->getSVal(LHS); ExplodedNodeSet Tmp2; @@ -2610,7 +2613,7 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, // Simulate the effects of a "store": bind the value of the RHS // to the L-Value represented by the LHS. - EvalStore(Dst, B, LHS, *I2, state->BindExpr(B, ExprVal), LeftV, RightV); + EvalStore(Tmp3, B, LHS, *I2, state->BindExpr(B, ExprVal), LeftV, RightV); continue; } @@ -2622,26 +2625,17 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, if (Result.isUnknown()) { if (OldSt != state) { // Generate a new node if we have already created a new state. - MakeNode(Dst, B, *I2, state); + MakeNode(Tmp3, B, *I2, state); } else - Dst.Add(*I2); + Tmp3.Add(*I2); continue; } state = state->BindExpr(B, Result); - if (Result.isUndef()) { - if (ExplodedNode *UndefNode = Builder->generateNode(B, state, *I2)){ - UndefNode->markAsSink(); - UndefResults.insert(UndefNode); - } - continue; - } - - // Otherwise, create a new node. - MakeNode(Dst, B, *I2, state); + MakeNode(Tmp3, B, *I2, state); continue; } @@ -2720,11 +2714,13 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, llvm::tie(state, LHSVal) = SVator.EvalCast(Result, state, LTy, CTy); } - EvalStore(Dst, B, LHS, *I3, state->BindExpr(B, Result), + EvalStore(Tmp3, B, LHS, *I3, state->BindExpr(B, Result), location, LHSVal); } } } + + CheckerVisit(B, Dst, Tmp3, false); } //===----------------------------------------------------------------------===// @@ -2980,3 +2976,27 @@ void GRExprEngine::ViewGraph(ExplodedNode** Beg, ExplodedNode** End) { GraphPrintSourceManager = NULL; #endif } + +void GRExprEngine::RegisterInternalChecks() { + // Register internal "built-in" BugTypes with the BugReporter. These BugTypes + // are different than what probably many checks will do since they don't + // create BugReports on-the-fly but instead wait until GRExprEngine finishes + // analyzing a function. Generation of BugReport objects is done via a call + // to 'FlushReports' from BugReporter. + // The following checks do not need to have their associated BugTypes + // explicitly registered with the BugReporter. If they issue any BugReports, + // their associated BugType will get registered with the BugReporter + // automatically. Note that the check itself is owned by the GRExprEngine + // object. + RegisterAttrNonNullChecker(*this); + RegisterCallAndMessageChecker(*this); + RegisterDereferenceChecker(*this); + RegisterVLASizeChecker(*this); + RegisterDivZeroChecker(*this); + RegisterReturnStackAddressChecker(*this); + RegisterReturnUndefChecker(*this); + RegisterUndefinedArraySubscriptChecker(*this); + RegisterUndefinedAssignmentChecker(*this); + RegisterUndefBranchChecker(*this); + RegisterUndefResultChecker(*this); +} diff --git a/lib/Analysis/GRExprEngineInternalChecks.cpp b/lib/Analysis/GRExprEngineInternalChecks.cpp index 93ade9d208..6153a45860 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.cpp +++ b/lib/Analysis/GRExprEngineInternalChecks.cpp @@ -24,147 +24,7 @@ using namespace clang; using namespace clang::bugreporter; -//===----------------------------------------------------------------------===// -// Utility functions. -//===----------------------------------------------------------------------===// - -template inline -ExplodedNode* GetNode(ITERATOR I) { - return *I; -} - -//===----------------------------------------------------------------------===// -// Bug Descriptions. -//===----------------------------------------------------------------------===// -namespace clang { -class BuiltinBugReport : public RangedBugReport { -public: - BuiltinBugReport(BugType& bt, const char* desc, - 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); -}; - -void BuiltinBugReport::registerInitialVisitors(BugReporterContext& BRC, - const ExplodedNode* N) { - static_cast(getBugType()).registerInitialVisitors(BRC, N, this); -} - -template -void BuiltinBug::Emit(BugReporter& BR, ITER I, ITER E) { - for (; I != E; ++I) BR.EmitReport(new BuiltinBugReport(*this, desc.c_str(), - GetNode(I))); -} - -class VISIBILITY_HIDDEN UndefResult : public BuiltinBug { -public: - UndefResult(GRExprEngine* eng) - : BuiltinBug(eng,"Undefined or garbage result", - "Result of operation is garbage or undefined") {} - - void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) { - for (GRExprEngine::undef_result_iterator I=Eng.undef_results_begin(), - E = Eng.undef_results_end(); I!=E; ++I) { - - ExplodedNode *N = *I; - const Stmt *S = N->getLocationAs()->getStmt(); - BuiltinBugReport *report = NULL; - - if (const BinaryOperator *B = dyn_cast(S)) { - llvm::SmallString<256> sbuf; - llvm::raw_svector_ostream OS(sbuf); - const GRState *ST = N->getState(); - const Expr *Ex = NULL; - bool isLeft = true; - - if (ST->getSVal(B->getLHS()).isUndef()) { - Ex = B->getLHS()->IgnoreParenCasts(); - isLeft = true; - } - else if (ST->getSVal(B->getRHS()).isUndef()) { - Ex = B->getRHS()->IgnoreParenCasts(); - isLeft = false; - } - - if (Ex) { - OS << "The " << (isLeft ? "left" : "right") - << " operand of '" - << BinaryOperator::getOpcodeStr(B->getOpcode()) - << "' is a garbage value"; - } - else { - // Neither operand was undefined, but the result is undefined. - OS << "The result of the '" - << BinaryOperator::getOpcodeStr(B->getOpcode()) - << "' expression is undefined"; - } - - // FIXME: Use StringRefs to pass string information. - report = new BuiltinBugReport(*this, OS.str().str().c_str(), N); - if (Ex) report->addRange(Ex->getSourceRange()); - } - else { - report = new BuiltinBugReport(*this, - "Expression evaluates to an uninitialized" - " or undefined value", N); - } - - BR.EmitReport(report); - } - } - - void registerInitialVisitors(BugReporterContext& BRC, - const ExplodedNode* N, - BuiltinBugReport *R) { - - const Stmt *S = N->getLocationAs()->getStmt(); - const Stmt *X = S; - - if (const BinaryOperator *B = dyn_cast(S)) { - const GRState *ST = N->getState(); - if (ST->getSVal(B->getLHS()).isUndef()) - X = B->getLHS(); - else if (ST->getSVal(B->getRHS()).isUndef()) - X = B->getRHS(); - } - - registerTrackNullOrUndefValue(BRC, X, N); - } -}; - -} // end clang namespace - //===----------------------------------------------------------------------===// // Check registration. //===----------------------------------------------------------------------===// -void GRExprEngine::RegisterInternalChecks() { - // Register internal "built-in" BugTypes with the BugReporter. These BugTypes - // are different than what probably many checks will do since they don't - // create BugReports on-the-fly but instead wait until GRExprEngine finishes - // analyzing a function. Generation of BugReport objects is done via a call - // to 'FlushReports' from BugReporter. - BR.Register(new UndefResult(this)); - - // The following checks do not need to have their associated BugTypes - // explicitly registered with the BugReporter. If they issue any BugReports, - // their associated BugType will get registered with the BugReporter - // automatically. Note that the check itself is owned by the GRExprEngine - // object. - RegisterAttrNonNullChecker(*this); - RegisterCallAndMessageChecker(*this); - RegisterDereferenceChecker(*this); - RegisterVLASizeChecker(*this); - RegisterDivZeroChecker(*this); - RegisterReturnStackAddressChecker(*this); - RegisterReturnUndefChecker(*this); - RegisterUndefinedArraySubscriptChecker(*this); - RegisterUndefinedAssignmentChecker(*this); - RegisterUndefBranchChecker(*this); -} diff --git a/lib/Analysis/GRExprEngineInternalChecks.h b/lib/Analysis/GRExprEngineInternalChecks.h index f691190df2..5b7a7572ed 100644 --- a/lib/Analysis/GRExprEngineInternalChecks.h +++ b/lib/Analysis/GRExprEngineInternalChecks.h @@ -35,6 +35,7 @@ void RegisterArrayBoundChecker(GRExprEngine &Eng); void RegisterUndefinedArraySubscriptChecker(GRExprEngine &Eng); void RegisterUndefinedAssignmentChecker(GRExprEngine &Eng); void RegisterUndefBranchChecker(GRExprEngine &Eng); +void RegisterUndefResultChecker(GRExprEngine &Eng); } // end clang namespace #endif diff --git a/lib/Analysis/UndefResultChecker.cpp b/lib/Analysis/UndefResultChecker.cpp new file mode 100644 index 0000000000..191148029b --- /dev/null +++ b/lib/Analysis/UndefResultChecker.cpp @@ -0,0 +1,86 @@ +//=== UndefResultChecker.cpp ------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This defines UndefResultChecker, a builtin check in GRExprEngine that +// performs checks for undefined results of non-assignment binary operators. +// +//===----------------------------------------------------------------------===// + +#include "GRExprEngineInternalChecks.h" +#include "clang/Analysis/PathSensitive/CheckerVisitor.h" +#include "clang/Analysis/PathSensitive/GRExprEngine.h" +#include "clang/Analysis/PathSensitive/BugReporter.h" + +using namespace clang; + +namespace { +class VISIBILITY_HIDDEN UndefResultChecker + : public CheckerVisitor { + + BugType *BT; + +public: + UndefResultChecker() : BT(0) {} + static void *getTag() { static int tag = 0; return &tag; } + void PostVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B); +}; +} // end anonymous namespace + +void clang::RegisterUndefResultChecker(GRExprEngine &Eng) { + Eng.registerCheck(new UndefResultChecker()); +} + +void UndefResultChecker::PostVisitBinaryOperator(CheckerContext &C, + const BinaryOperator *B) { + const GRState *state = C.getState(); + if (state->getSVal(B).isUndef()) { + // Generate an error node. + ExplodedNode *N = C.GenerateSink(); + if (!N) + return; + + if (!BT) + BT = new BuiltinBug("Result of operation is garbage or undefined"); + + llvm::SmallString<256> sbuf; + llvm::raw_svector_ostream OS(sbuf); + const Expr *Ex = NULL; + bool isLeft = true; + + if (state->getSVal(B->getLHS()).isUndef()) { + Ex = B->getLHS()->IgnoreParenCasts(); + isLeft = true; + } + else if (state->getSVal(B->getRHS()).isUndef()) { + Ex = B->getRHS()->IgnoreParenCasts(); + isLeft = false; + } + + if (Ex) { + OS << "The " << (isLeft ? "left" : "right") + << " operand of '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' is a garbage value"; + } + else { + // Neither operand was undefined, but the result is undefined. + OS << "The result of the '" + << BinaryOperator::getOpcodeStr(B->getOpcode()) + << "' expression is undefined"; + } + EnhancedBugReport *report = new EnhancedBugReport(*BT, + OS.str().str().c_str(), N); + report->addRange(Ex->getSourceRange()); + if (Ex) + report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, Ex); + else + report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, B); + C.EmitReport(report); + } +}