From: Ted Kremenek Date: Mon, 17 Sep 2007 20:49:30 +0000 (+0000) Subject: UninitialuzedValues now only tracks BlockVarDecls; obviating false positives with X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2bf55140b893c874ef16929bfe953f7cc7823425;p=clang UninitialuzedValues now only tracks BlockVarDecls; obviating false positives with globals and function parameters. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@42055 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/Analysis/DataflowSolver.h b/Analysis/DataflowSolver.h index de5f9a3ddd..4a089dbcdc 100644 --- a/Analysis/DataflowSolver.h +++ b/Analysis/DataflowSolver.h @@ -117,7 +117,15 @@ private: for (CFGBlock::const_succ_iterator I=B->succ_begin(), E=B->succ_end(); I != E; ++I) WorkList.enqueue(*I); - } + } + + // For blocks that have no associated dataflow value, instantiate a + // default value. + BlockDataMapTy& M = D.getBlockDataMap(); + + for (CFG::const_iterator I=cfg.begin(), E=cfg.end(); I!=E; ++I) + if (M.find(&*I) == M.end()) + M[&*I].resetValues(D.getAnalysisData()); } /// SolveDataflowEquations (BACKWARD ANALYSIS) - Perform the actual @@ -153,7 +161,7 @@ private: ValTy& V = TF.getVal(); // Merge dataflow values from all predecessors of this block. - V.resetValues(); + V.resetValues(D.getAnalysisData()); MergeOperatorTy Merge; for (CFGBlock::const_pred_iterator I=B->pred_begin(), @@ -175,7 +183,7 @@ private: ValTy& V = TF.getVal(); // Merge dataflow values from all predecessors of this block. - V.resetValues(); + V.resetValues(D.getAnalysisData()); MergeOperatorTy Merge; for (CFGBlock::const_succ_iterator I=B->succ_begin(), diff --git a/Analysis/UnintializedValues.cpp b/Analysis/UnintializedValues.cpp index 2640493095..ce9454d0cc 100644 --- a/Analysis/UnintializedValues.cpp +++ b/Analysis/UnintializedValues.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/UninitializedValues.h" -#include "clang/Analysis/CFGVarDeclVisitor.h" #include "clang/Analysis/CFGStmtVisitor.h" #include "clang/Analysis/LocalCheckers.h" #include "clang/Basic/Diagnostic.h" @@ -29,32 +28,55 @@ using namespace clang; namespace { -class RegisterDeclsAndExprs : public CFGVarDeclVisitor { +class RegisterDeclsAndExprs : public CFGStmtVisitor { UninitializedValues::AnalysisDataTy& AD; public: - RegisterDeclsAndExprs(const CFG& cfg, UninitializedValues::AnalysisDataTy& ad) - : CFGVarDeclVisitor(cfg), AD(ad) - {} + RegisterDeclsAndExprs(UninitializedValues::AnalysisDataTy& ad) : AD(ad) {} - void VisitVarDecl(VarDecl* D) { - if (AD.VMap.find(D) == AD.VMap.end()) - AD.VMap[D] = AD.NumDecls++; + void VisitBlockVarDecl(BlockVarDecl* VD) { + if (AD.VMap.find(VD) == AD.VMap.end()) + AD.VMap[VD] = AD.NumDecls++; + } + + void VisitDeclChain(ScopedDecl* D) { + for (; D != NULL; D = D->getNextDeclarator()) + if (BlockVarDecl* VD = dyn_cast(D)) + VisitBlockVarDecl(VD); } void BlockStmt_VisitExpr(Expr* E) { if (AD.EMap.find(E) == AD.EMap.end()) AD.EMap[E] = AD.NumBlockExprs++; - } + + Visit(E); + } + + void VisitDeclRefExpr(DeclRefExpr* DR) { + VisitDeclChain(DR->getDecl()); + } + + void VisitDeclStmt(DeclStmt* S) { + VisitDeclChain(S->getDecl()); + } + + void VisitStmt(Stmt* S) { + VisitChildren(S); + } + }; } // end anonymous namespace void UninitializedValues::InitializeValues(const CFG& cfg) { - RegisterDeclsAndExprs R(cfg,this->getAnalysisData()); - R.VisitAllDecls(); + RegisterDeclsAndExprs R(this->getAnalysisData()); + + for (CFG::const_iterator I=cfg.begin(), E=cfg.end(); I!=E; ++I) + for (CFGBlock::const_iterator BI=I->begin(), BE=I->end(); BI!=BE; ++BI) + R.BlockStmt_Visit(*BI); + + // Initialize the values of the last block. UninitializedValues::ValTy& V = getBlockDataMap()[&cfg.getEntry()]; - V.DeclBV.resize(getAnalysisData().NumDecls); - V.ExprBV.resize(getAnalysisData().NumBlockExprs); + V.resetValues(getAnalysisData()); } //===----------------------------------------------------------------------===// @@ -68,8 +90,7 @@ class TransferFuncs : public CFGStmtVisitor { UninitializedValues::AnalysisDataTy& AD; public: TransferFuncs(UninitializedValues::AnalysisDataTy& ad) : AD(ad) { - V.DeclBV.resize(AD.NumDecls); - V.ExprBV.resize(AD.NumBlockExprs); + V.resetValues(AD); } UninitializedValues::ValTy& getVal() { return V; } @@ -88,7 +109,7 @@ public: bool TransferFuncs::VisitDeclRefExpr(DeclRefExpr* DR) { - if (VarDecl* VD = dyn_cast(DR->getDecl())) { + if (BlockVarDecl* VD = dyn_cast(DR->getDecl())) { assert ( AD.VMap.find(VD) != AD.VMap.end() && "Unknown VarDecl."); if (AD.Observer) AD.Observer->ObserveDeclRefExpr(V,AD,DR,VD); @@ -111,7 +132,7 @@ bool TransferFuncs::VisitBinaryOperator(BinaryOperator* B) { if (ParenExpr* P = dyn_cast(S)) S = P->getSubExpr(); else if (DeclRefExpr* DR = dyn_cast(S)) - if (VarDecl* VD = dyn_cast(DR->getDecl())) { + if (BlockVarDecl* VD = dyn_cast(DR->getDecl())) { assert ( AD.VMap.find(VD) != AD.VMap.end() && "Unknown VarDecl."); return V.DeclBV[ AD.VMap[VD] ] = Visit(B->getRHS()); } @@ -127,10 +148,13 @@ bool TransferFuncs::VisitDeclStmt(DeclStmt* S) { bool x = Initialized(); for (ScopedDecl* D = S->getDecl(); D != NULL; D = D->getNextDeclarator()) - if (VarDecl* VD = dyn_cast(D)) + if (BlockVarDecl* VD = dyn_cast(D)) if (Stmt* I = VD->getInit()) { + assert ( AD.EMap.find(cast(I)) != + AD.EMap.end() && "Unknown Expr."); + assert ( AD.VMap.find(VD) != AD.VMap.end() && "Unknown VarDecl."); - x = V.DeclBV[ AD.VMap[VD] ] = Visit(I); + x = V.DeclBV[ AD.VMap[VD] ] = V.ExprBV[ AD.EMap[cast(I)] ]; } return x; @@ -150,7 +174,7 @@ bool TransferFuncs::VisitUnaryOperator(UnaryOperator* U) { if (ParenExpr* P = dyn_cast(S)) S = P->getSubExpr(); else if (DeclRefExpr* DR = dyn_cast(S)) { - if (VarDecl* VD = dyn_cast(DR->getDecl())) { + if (BlockVarDecl* VD = dyn_cast(DR->getDecl())) { assert ( AD.VMap.find(VD) != AD.VMap.end() && "Unknown VarDecl."); V.DeclBV[ AD.VMap[VD] ] = Initialized(); } @@ -222,7 +246,7 @@ struct Merge { assert (Dst.ExprBV.size() == Src.ExprBV.size() && "Bitvector sizes do not match."); - Dst.ExprBV &= Src.ExprBV; + Dst.ExprBV |= Src.ExprBV; } }; } // end anonymous namespace @@ -238,7 +262,7 @@ namespace { class UninitializedValuesChecker : public UninitializedValues::ObserverTy { ASTContext &Ctx; Diagnostic &Diags; - llvm::SmallPtrSet AlreadyWarned; + llvm::SmallPtrSet AlreadyWarned; public: UninitializedValuesChecker(ASTContext &ctx, Diagnostic &diags) @@ -246,7 +270,7 @@ public: virtual void ObserveDeclRefExpr(UninitializedValues::ValTy& V, UninitializedValues::AnalysisDataTy& AD, - DeclRefExpr* DR, VarDecl* VD) { + DeclRefExpr* DR, BlockVarDecl* VD) { assert ( AD.VMap.find(VD) != AD.VMap.end() && "Unknown VarDecl."); if (V.DeclBV[ AD.VMap[VD] ] == TransferFuncs::Uninitialized()) @@ -257,6 +281,7 @@ public: } // end anonymous namespace +namespace clang { void CheckUninitializedValues(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags) { @@ -274,3 +299,5 @@ void CheckUninitializedValues(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags) { for (CFG::iterator I=cfg.begin(), E=cfg.end(); I!=E; ++I) S.runOnBlock(&*I); } + +} diff --git a/Driver/ASTStreamers.cpp b/Driver/ASTStreamers.cpp index 124b9e2e51..cce19e795a 100644 --- a/Driver/ASTStreamers.cpp +++ b/Driver/ASTStreamers.cpp @@ -210,7 +210,7 @@ ASTConsumer *clang::CreateLiveVarAnalyzer() { } //===----------------------------------------------------------------------===// -// RunDeadStores - run checker to locate dead stores in a function +// DeadStores - run checker to locate dead stores in a function namespace { class DeadStoreVisitor : public CFGVisitor { @@ -231,6 +231,29 @@ ASTConsumer *clang::CreateDeadStoreChecker(Diagnostic &Diags) { return new DeadStoreVisitor(Diags); } +//===----------------------------------------------------------------------===// +// Unitialized Values - run checker to flag potential uses of uninitalized +// variables. + +namespace { + class UninitValsVisitor : public CFGVisitor { + Diagnostic &Diags; + ASTContext *Ctx; + public: + UninitValsVisitor(Diagnostic &diags) : Diags(diags) {} + virtual void Initialize(ASTContext &Context, unsigned MainFileID) { + Ctx = &Context; + } + + virtual void VisitCFG(CFG& C) { CheckUninitializedValues(C, *Ctx, Diags); } + virtual bool printFuncDeclStart() { return false; } + }; +} // end anonymous namespace + +ASTConsumer *clang::CreateUnitValsChecker(Diagnostic &Diags) { + return new UninitValsVisitor(Diags); +} + //===----------------------------------------------------------------------===// // LLVM Emitter diff --git a/Driver/ASTStreamers.h b/Driver/ASTStreamers.h index d9ea3bdcf3..2d4a18fdc2 100644 --- a/Driver/ASTStreamers.h +++ b/Driver/ASTStreamers.h @@ -24,6 +24,7 @@ ASTConsumer *CreateASTDumper(); ASTConsumer *CreateCFGDumper(bool ViewGraphs = false); ASTConsumer *CreateLiveVarAnalyzer(); ASTConsumer *CreateDeadStoreChecker(Diagnostic &Diags); +ASTConsumer *CreateUnitValsChecker(Diagnostic &Diags); ASTConsumer *CreateLLVMEmitter(Diagnostic &Diags); } // end clang namespace diff --git a/Driver/clang.cpp b/Driver/clang.cpp index 4051806718..a69041396e 100644 --- a/Driver/clang.cpp +++ b/Driver/clang.cpp @@ -58,6 +58,7 @@ enum ProgActions { ParseCFGView, // Parse ASTS. Build CFGs. View CFGs. AnalysisLiveVariables, // Print results of live-variable analysis. WarnDeadStores, // Run DeadStores checker on parsed ASTs. + WarnUninitVals, // Run UnitializedVariables checker. ParsePrintCallbacks, // Parse and print each callback. ParseSyntaxOnly, // Parse and perform semantic analysis. ParseNoop, // Parse with noop callbacks. @@ -98,6 +99,8 @@ ProgAction(llvm::cl::desc("Choose output type:"), llvm::cl::ZeroOrMore, "Print results of live variable analysis."), clEnumValN(WarnDeadStores, "check-dead-stores", "Flag warnings of stores to dead variables."), + clEnumValN(WarnUninitVals, "check-unit-vals", + "Flag warnings of uses of unitialized variables."), clEnumValN(EmitLLVM, "emit-llvm", "Build ASTs then convert to LLVM, emit .ll file"), clEnumValEnd)); @@ -870,6 +873,11 @@ static void ProcessInputFile(Preprocessor &PP, unsigned MainFileID, ParseAST(PP, MainFileID, *C.get(), Stats); break; } + case WarnUninitVals: { + std::auto_ptr C(CreateUnitValsChecker(PP.getDiagnostics())); + ParseAST(PP, MainFileID, *C.get(), Stats); + break; + } case EmitLLVM: { std::auto_ptr C(CreateLLVMEmitter(PP.getDiagnostics())); ParseAST(PP, MainFileID, *C.get(), Stats); diff --git a/include/clang/Analysis/CFGVarDeclVisitor.h b/include/clang/Analysis/CFGVarDeclVisitor.h index 7e806dde5e..cd8073c84d 100644 --- a/include/clang/Analysis/CFGVarDeclVisitor.h +++ b/include/clang/Analysis/CFGVarDeclVisitor.h @@ -29,8 +29,7 @@ public: CFGVarDeclVisitor(const CFG& c) : cfg(c) {} void VisitStmt(Stmt* S) { - for (Stmt::child_iterator I=S->child_begin(), E=S->child_end(); I!=E; ++I) - static_cast(this)->Visit(*I); + static_cast(this)->VisitChildren(S); } void VisitDeclRefExpr(DeclRefExpr* DR) { @@ -56,7 +55,7 @@ public: void VisitAllDecls() { for (CFG::const_iterator BI = cfg.begin(), BE = cfg.end(); BI != BE; ++BI) for (CFGBlock::const_iterator SI=BI->begin(),SE = BI->end();SI != SE;++SI) - static_cast(this)->Visit(const_cast(*SI)); + static_cast(this)->BlockStmt_Visit(const_cast(*SI)); } }; diff --git a/include/clang/Analysis/UninitializedValues.h b/include/clang/Analysis/UninitializedValues.h index 3fe1e17250..6fcee2dbf4 100644 --- a/include/clang/Analysis/UninitializedValues.h +++ b/include/clang/Analysis/UninitializedValues.h @@ -20,7 +20,7 @@ namespace clang { - class VarDecl; + class BlockVarDecl; class Expr; class DeclRefExpr; @@ -30,6 +30,23 @@ namespace clang { class UninitializedValues_ValueTypes { public: + //===--------------------------------------------------------------------===// + // AnalysisDataTy - Whole-function meta data used by the transfer function + // logic. + //===--------------------------------------------------------------------===// + + struct ObserverTy; + + struct AnalysisDataTy { + llvm::DenseMap VMap; + llvm::DenseMap EMap; + unsigned NumDecls; + unsigned NumBlockExprs; + ObserverTy* Observer; + + AnalysisDataTy() : NumDecls(0), NumBlockExprs(0), Observer(NULL) {} + }; + //===--------------------------------------------------------------------===// // ValTy - Dataflow value. //===--------------------------------------------------------------------===// @@ -38,9 +55,10 @@ public: llvm::BitVector DeclBV; llvm::BitVector ExprBV; - // Used by the solver. - void resetValues() { + void resetValues(AnalysisDataTy& AD) { + DeclBV.resize(AD.NumDecls); DeclBV.reset(); + ExprBV.resize(AD.NumBlockExprs); ExprBV.reset(); } @@ -52,24 +70,7 @@ public: DeclBV = RHS.DeclBV; ExprBV = RHS.ExprBV; } - }; - - //===--------------------------------------------------------------------===// - // AnalysisDataTy - Whole-function meta data used by the transfer function - // logic. - //===--------------------------------------------------------------------===// - - struct ObserverTy; - - struct AnalysisDataTy { - llvm::DenseMap VMap; - llvm::DenseMap EMap; - unsigned NumDecls; - unsigned NumBlockExprs; - ObserverTy* Observer; - - AnalysisDataTy() : NumDecls(0), NumBlockExprs(0), Observer(NULL) {} - }; + }; //===--------------------------------------------------------------------===// // ObserverTy - Observer for querying DeclRefExprs that use an uninitalized @@ -79,7 +80,7 @@ public: struct ObserverTy { virtual ~ObserverTy(); virtual void ObserveDeclRefExpr(ValTy& Val, AnalysisDataTy& AD, - DeclRefExpr* DR, VarDecl* VD) = 0; + DeclRefExpr* DR, BlockVarDecl* VD) = 0; }; };