From 7deed0c65b315cac037539401c49586283158d9f Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Tue, 15 Apr 2008 18:35:30 +0000 Subject: [PATCH] Fix bug in terminator processing for uninitialized-values: simply ignore the terminator, don't reprocess it. LiveVariables analysis now does a flow-insensitive analysis to determine what variables have their address taken; these variables are now always treated as being live. The DataflowSolver now uses "SetTopValue()" when getting the initial value for the entry/exit block. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@49734 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/LiveVariables.h | 8 ++- .../Analysis/FlowSensitive/DataflowSolver.h | 24 ++++++- lib/Analysis/LiveVariables.cpp | 71 +++++++++++++++++-- lib/Analysis/UninitializedValues.cpp | 19 +++-- 4 files changed, 100 insertions(+), 22 deletions(-) diff --git a/include/clang/Analysis/Analyses/LiveVariables.h b/include/clang/Analysis/Analyses/LiveVariables.h index f172df62e8..6571c73ab7 100644 --- a/include/clang/Analysis/Analyses/LiveVariables.h +++ b/include/clang/Analysis/Analyses/LiveVariables.h @@ -28,6 +28,8 @@ struct LiveVariables_ValueTypes { struct ObserverTy; + // We keep dataflow state for declarations and block-level expressions; + typedef ExprDeclBitVector_Types::ValTy ValTy; // We need to keep track of both declarations and CFGBlock-level expressions, // (so that we don't explore such expressions twice). We also want @@ -36,12 +38,10 @@ struct LiveVariables_ValueTypes { struct AnalysisDataTy : public ExprDeclBitVector_Types::AnalysisDataTy { ObserverTy* Observer; + ValTy AlwaysLive; AnalysisDataTy() : Observer(NULL) {} }; - - // We only keep actual dataflow state for declarations. - typedef ExprDeclBitVector_Types::ValTy ValTy; //===-----------------------------------------------------===// // ObserverTy - Observer for uninitialized values queries. @@ -61,6 +61,8 @@ struct LiveVariables_ValueTypes { class LiveVariables : public DataflowValues { + + public: typedef LiveVariables_ValueTypes::ObserverTy ObserverTy; diff --git a/include/clang/Analysis/FlowSensitive/DataflowSolver.h b/include/clang/Analysis/FlowSensitive/DataflowSolver.h index 7a19c19119..9cf77cf144 100644 --- a/include/clang/Analysis/FlowSensitive/DataflowSolver.h +++ b/include/clang/Analysis/FlowSensitive/DataflowSolver.h @@ -204,11 +204,31 @@ private: void EnqueueFirstBlock(const CFG& cfg, dataflow::backward_analysis_tag) { WorkList.enqueue(&cfg.getExit()); } + + void ResetValues(CFG& cfg, ValTy& V, const CFGBlock* B, + dataflow::forward_analysis_tag){ + + if (B == &cfg.getEntry()) + TF.SetTopValue(V); + else + V.resetValues(D.getAnalysisData()); + } + + void ResetValues(CFG& cfg, ValTy& V, const CFGBlock* B, + dataflow::backward_analysis_tag){ + + if (B == &cfg.getExit()) + TF.SetTopValue(V); + else + V.resetValues(D.getAnalysisData()); + } void ProcessMerge(CFG& cfg, const CFGBlock* B) { + + ValTy& V = TF.getVal(); + ResetValues(cfg, V, B, AnalysisDirTag()); + // Merge dataflow values from all predecessors of this block. - ValTy& V = TF.getVal(); - V.resetValues(D.getAnalysisData()); MergeOperatorTy Merge; EdgeDataMapTy& M = D.getEdgeDataMap(); diff --git a/lib/Analysis/LiveVariables.cpp b/lib/Analysis/LiveVariables.cpp index 4cef30cab8..7896bfcfb0 100644 --- a/lib/Analysis/LiveVariables.cpp +++ b/lib/Analysis/LiveVariables.cpp @@ -19,6 +19,7 @@ #include "clang/Analysis/Visitors/CFGRecStmtDeclVisitor.h" #include "clang/Analysis/FlowSensitive/DataflowSolver.h" #include "llvm/ADT/SmallPtrSet.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/Support/Compiler.h" #include @@ -26,6 +27,13 @@ using namespace clang; +//===----------------------------------------------------------------------===// +// Useful constants. +//===----------------------------------------------------------------------===// + +static const bool Alive = true; +static const bool Dead = false; + //===----------------------------------------------------------------------===// // Dataflow initialization logic. //===----------------------------------------------------------------------===// @@ -35,9 +43,49 @@ class VISIBILITY_HIDDEN RegisterDecls : public CFGRecStmtDeclVisitor { LiveVariables::AnalysisDataTy& AD; + + typedef llvm::SmallVector AlwaysLiveTy; + AlwaysLiveTy AlwaysLive; + + public: - RegisterDecls(LiveVariables::AnalysisDataTy& ad) : AD(ad) {} - void VisitVarDecl(VarDecl* VD) { AD.Register(VD); } + RegisterDecls(LiveVariables::AnalysisDataTy& ad) : AD(ad) {} + + ~RegisterDecls() { + + AD.AlwaysLive.resetValues(AD); + + for (AlwaysLiveTy::iterator I = AlwaysLive.begin(), E = AlwaysLive.end(); + I != E; ++ I) + AD.AlwaysLive(*I, AD) = Alive; + } + + void VisitVarDecl(VarDecl* VD) { + // Register the VarDecl for tracking. + AD.Register(VD); + + // Does the variable have global storage? If so, it is always live. + if (VD->hasGlobalStorage()) + AlwaysLive.push_back(VD); + } + + void VisitUnaryOperator(UnaryOperator* U) { + // Check for '&'. Any VarDecl whose value has its address-taken we + // treat as always being live (flow-insensitive). + + Expr* E = U->getSubExpr()->IgnoreParenCasts(); + + if (U->getOpcode() == UnaryOperator::AddrOf) + if (DeclRefExpr* DR = dyn_cast(E)) + if (VarDecl* VD = dyn_cast(DR->getDecl())) { + AD.Register(VD); + AlwaysLive.push_back(VD); + return; + } + + Visit(E); + } + CFG& getCFG() { return AD.getCFG(); } }; } // end anonymous namespace @@ -55,9 +103,6 @@ LiveVariables::LiveVariables(CFG& cfg) { //===----------------------------------------------------------------------===// namespace { - -static const bool Alive = true; -static const bool Dead = false; class VISIBILITY_HIDDEN TransferFuncs : public CFGRecStmtVisitor{ LiveVariables::AnalysisDataTy& AD; @@ -75,6 +120,11 @@ public: void VisitUnaryOperator(UnaryOperator* U); void Visit(Stmt *S); void VisitTerminator(Stmt* S); + + void SetTopValue(LiveVariables::ValTy& V) { + V = AD.AlwaysLive; + } + }; void TransferFuncs::Visit(Stmt *S) { @@ -151,7 +201,11 @@ void TransferFuncs::VisitAssign(BinaryOperator* B) { // Assigning to a variable? if (DeclRefExpr* DR = dyn_cast(LHS->IgnoreParens())) { - LiveState(DR->getDecl(), AD) = Dead; + + // Update liveness inforamtion. + unsigned bit = AD.getIdx(DR->getDecl()); + LiveState.getDeclBit(bit) = Dead | AD.AlwaysLive.getDeclBit(bit); + if (AD.Observer) { AD.Observer->ObserverKill(DR); } // Handle things like +=, etc., which also generate "uses" @@ -170,7 +224,10 @@ void TransferFuncs::VisitDeclStmt(DeclStmt* DS) { // possibly be live before they are declared. for (ScopedDecl* D = DS->getDecl(); D != NULL; D = D->getNextDeclarator()) if (VarDecl* VD = dyn_cast(D)) { - LiveState(D, AD) = Dead; + + // Update liveness information. + unsigned bit = AD.getIdx(VD); + LiveState.getDeclBit(bit) = Dead | AD.AlwaysLive.getDeclBit(bit); if (Expr* Init = VD->getInit()) Visit(Init); diff --git a/lib/Analysis/UninitializedValues.cpp b/lib/Analysis/UninitializedValues.cpp index d5c697aa1c..2116e505d5 100644 --- a/lib/Analysis/UninitializedValues.cpp +++ b/lib/Analysis/UninitializedValues.cpp @@ -58,13 +58,15 @@ class VISIBILITY_HIDDEN TransferFuncs UninitializedValues::ValTy V; UninitializedValues::AnalysisDataTy& AD; public: - TransferFuncs(UninitializedValues::AnalysisDataTy& ad) : AD(ad) { - V.resetValues(AD); - } + TransferFuncs(UninitializedValues::AnalysisDataTy& ad) : AD(ad) {} UninitializedValues::ValTy& getVal() { return V; } CFG& getCFG() { return AD.getCFG(); } + void SetTopValue(UninitializedValues::ValTy& X) { + X.resetValues(AD); + } + bool VisitDeclRefExpr(DeclRefExpr* DR); bool VisitBinaryOperator(BinaryOperator* B); bool VisitUnaryOperator(UnaryOperator* U); @@ -76,7 +78,7 @@ public: bool Visit(Stmt *S); bool BlockStmt_VisitExpr(Expr* E); - void VisitTerminator(Stmt* T) { Visit(T); } + void VisitTerminator(Stmt* T) { } BlockVarDecl* FindBlockVarDecl(Stmt* S); }; @@ -216,12 +218,9 @@ bool TransferFuncs::BlockStmt_VisitExpr(Expr* E) { // In our transfer functions we take the approach that any // combination of unintialized values, e.g. Unitialized + ___ = Unitialized. // -// Merges take the opposite approach. -// -// In the merge of dataflow values we prefer unsoundness, and -// prefer false negatives to false positives. At merges, if a value for a -// tracked Decl is EVER initialized in any of the predecessors we treat it as -// initialized at the confluence point. +// Merges take the same approach, preferring soundness. At a confluence point, +// if any predecessor has a variable marked uninitialized, the value is +// uninitialized at the confluence point. //===----------------------------------------------------------------------===// namespace {