From: Ted Kremenek Date: Fri, 7 Mar 2008 22:58:01 +0000 (+0000) Subject: Added --trim-path-graph to the driver to trim paths from the ExplodedGraph X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ffe0f43806d4823271c2406c1fccc2373115c36a;p=clang Added --trim-path-graph to the driver to trim paths from the ExplodedGraph that are not related to error nodes. Fixed bug where we did not detect some NULL dereferences. Added "ExplodedGraph::Trim" to trim all nodes that cannot transitively reach a set of provided nodes. Fixed subtle bug in ExplodedNodeImpl where we could create predecessor iterators that included the mangled "sink" bit. The better fix is to integrate this bit into the void* for the wrapped State, not the NodeGroups representing a node's predecessors and successors. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@48036 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/Analysis/ExplodedGraph.cpp b/Analysis/ExplodedGraph.cpp index 274565bf6c..421b9e3846 100644 --- a/Analysis/ExplodedGraph.cpp +++ b/Analysis/ExplodedGraph.cpp @@ -13,6 +13,9 @@ //===----------------------------------------------------------------------===// #include "clang/Analysis/PathSensitive/ExplodedGraph.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" #include using namespace clang; @@ -25,6 +28,7 @@ static inline std::vector& getVector(void* P) { void ExplodedNodeImpl::NodeGroup::addNode(ExplodedNodeImpl* N) { assert ((reinterpret_cast(N) & Mask) == 0x0); + assert (!getFlag()); if (getKind() == Size1) { if (ExplodedNodeImpl* NOld = getNode()) { @@ -47,14 +51,11 @@ void ExplodedNodeImpl::NodeGroup::addNode(ExplodedNodeImpl* N) { } } -bool ExplodedNodeImpl::NodeGroup::empty() const { - if (getKind() == Size1) - return getNode() ? false : true; - else - return getVector(getPtr()).empty(); -} unsigned ExplodedNodeImpl::NodeGroup::size() const { + if (getFlag()) + return 0; + if (getKind() == Size1) return getNode() ? 1 : 0; else @@ -62,15 +63,21 @@ unsigned ExplodedNodeImpl::NodeGroup::size() const { } ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::begin() const { + if (getFlag()) + return NULL; + if (getKind() == Size1) - return (ExplodedNodeImpl**) &P; + return (ExplodedNodeImpl**) (getPtr() ? &P : NULL); else return const_cast(&*(getVector(getPtr()).begin())); } ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::end() const { + if (getFlag()) + return NULL; + if (getKind() == Size1) - return (ExplodedNodeImpl**) (P ? &P+1 : &P); + return (ExplodedNodeImpl**) (getPtr() ? &P+1 : NULL); else return const_cast(&*(getVector(getPtr()).end())); } @@ -78,3 +85,110 @@ ExplodedNodeImpl** ExplodedNodeImpl::NodeGroup::end() const { ExplodedNodeImpl::NodeGroup::~NodeGroup() { if (getKind() == SizeOther) delete &getVector(getPtr()); } + +ExplodedGraphImpl* ExplodedGraphImpl::Trim(ExplodedNodeImpl** BeginSources, + ExplodedNodeImpl** EndSources) const{ + + typedef llvm::DenseSet Pass1Ty; + typedef llvm::DenseMap Pass2Ty; + + Pass1Ty Pass1; + Pass2Ty Pass2; + + llvm::SmallVector WL2; + + { // ===- Pass 1 (reverse BFS) -=== + + // Enqueue the source nodes to the first worklist. + + llvm::SmallVector WL1; + + for (ExplodedNodeImpl** I = BeginSources; I != EndSources; ++I) + WL1.push_back(*I); + + // Process the worklist. + + while (!WL1.empty()) { + + ExplodedNodeImpl* N = WL1.back(); + WL1.pop_back(); + + if (Pass1.count(N)) + continue; + + Pass1.insert(N); + + if (N->Preds.empty()) { + WL2.push_back(N); + continue; + } + + for (ExplodedNodeImpl** I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I) + WL1.push_back(*I); + } + } + + if (WL2.empty()) + return NULL; + + ExplodedGraphImpl* G = MakeEmptyGraph(); + + // ===- Pass 2 (forward DFS to construct the new graph) -=== + + while (!WL2.empty()) { + + ExplodedNodeImpl* N = WL2.back(); + WL2.pop_back(); + + // Skip this node if we have already processed it. + + if (Pass2.find(N) != Pass2.end()) + continue; + + // Create the corresponding node in the new graph. + + ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State, NULL); + Pass2[N] = NewN; + + if (N->Preds.empty()) + G->addRoot(NewN); + + // In the case that some of the intended predecessors of NewN have already + // been created, we should hook them up as predecessors. + + for (ExplodedNodeImpl **I=N->Preds.begin(), **E=N->Preds.end(); I!=E; ++I) { + + Pass2Ty::iterator PI = Pass2.find(*I); + + if (PI == Pass2.end()) + continue; + + NewN->addPredecessor(PI->second); + } + + // In the case that some of the intended successors of NewN have already + // been created, we should hook them up as successors. Otherwise, enqueue + // the new nodes from the original graph that should have nodes created + // in the new graph. + + for (ExplodedNodeImpl **I=N->Succs.begin(), **E=N->Succs.end(); I!=E; ++I) { + + Pass2Ty::iterator PI = Pass2.find(*I); + + if (PI != Pass2.end()) { + PI->second->addPredecessor(NewN); + continue; + } + + // Enqueue nodes to the worklist that were marked during pass 1. + + if (Pass1.count(*I)) + WL2.push_back(*I); + } + + if (N->isSink()) + NewN->markAsSink(); + } + + return G; +} diff --git a/Analysis/GRExprEngine.cpp b/Analysis/GRExprEngine.cpp index aaef2f8cb0..3bf0151180 100644 --- a/Analysis/GRExprEngine.cpp +++ b/Analysis/GRExprEngine.cpp @@ -732,7 +732,8 @@ void GRExprEngine::VisitSizeOfAlignOfTypeExpr(SizeOfAlignOfTypeExpr* Ex, } -void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst) { +void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred, + NodeSet& Dst, bool GetLVal) { Expr* Ex = U->getSubExpr()->IgnoreParens(); @@ -787,12 +788,16 @@ void GRExprEngine::VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst) { ValueState* StNotNull = Assume(St, LV, true, isFeasibleNotNull); if (isFeasibleNotNull) { + + if (GetLVal) Nodify(Dst, U, N, SetRVal(StNotNull, U, LV)); + else { + + // FIXME: Currently symbolic analysis "generates" new symbols + // for the contents of values. We need a better approach. - // FIXME: Currently symbolic analysis "generates" new symbols - // for the contents of values. We need a better approach. - - Nodify(Dst, U, N, SetRVal(StNotNull, U, - GetRVal(StNotNull, LV, U->getType()))); + Nodify(Dst, U, N, SetRVal(StNotNull, U, + GetRVal(StNotNull, LV, U->getType()))); + } } bool isFeasibleNull; @@ -975,18 +980,11 @@ void GRExprEngine::VisitLVal(Expr* Ex, NodeTy* Pred, NodeSet& Dst) { return; } - if (UnaryOperator* U = dyn_cast(Ex)) { + if (UnaryOperator* U = dyn_cast(Ex)) if (U->getOpcode() == UnaryOperator::Deref) { - Ex = U->getSubExpr()->IgnoreParens(); - - if (isa(Ex)) - Dst.Add(Pred); - else - Visit(Ex, Pred, Dst); - + VisitDeref(U, Pred, Dst, true); return; } - } Visit(Ex, Pred, Dst); } @@ -1810,11 +1808,40 @@ struct VISIBILITY_HIDDEN DOTGraphTraits : } // end llvm namespace #endif -void GRExprEngine::ViewGraph() { +#ifndef NDEBUG +template +static void AddSources(llvm::SmallVector& Sources, + ITERATOR I, ITERATOR E) { + + for ( ; I != E; ++I ) + Sources.push_back(*I); +} +#endif + +void GRExprEngine::ViewGraph(bool trim) { #ifndef NDEBUG GraphPrintCheckerState = this; GraphPrintSourceManager = &getContext().getSourceManager(); - llvm::ViewGraph(*G.roots_begin(), "GRExprEngine"); + + if (trim) { + llvm::SmallVector Sources; + AddSources(Sources, null_derefs_begin(), null_derefs_end()); + AddSources(Sources, undef_derefs_begin(), undef_derefs_end()); + + GRExprEngine::GraphTy* TrimmedG = G.Trim(&Sources[0], + &Sources[0]+Sources.size()); + + if (!TrimmedG) + llvm::cerr << "warning: Trimmed ExplodedGraph is empty.\n"; + else { + llvm::ViewGraph(*TrimmedG->roots_begin(), "TrimmedGRExprEngine"); + delete TrimmedG; + } + } + else + llvm::ViewGraph(*G.roots_begin(), "GRExprEngine"); + + GraphPrintCheckerState = NULL; GraphPrintSourceManager = NULL; #endif diff --git a/Analysis/GRSimpleVals.cpp b/Analysis/GRSimpleVals.cpp index c18610d924..efe435b884 100644 --- a/Analysis/GRSimpleVals.cpp +++ b/Analysis/GRSimpleVals.cpp @@ -90,7 +90,7 @@ static void EmitWarning(Diagnostic& Diag, SourceManager& SrcMgr, } unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx, - Diagnostic& Diag, bool Visualize) { + Diagnostic& Diag, bool Visualize, bool TrimGraph) { if (Diag.hasErrorOccurred()) return 0; @@ -141,7 +141,7 @@ unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx, "Pass-by-value argument in function or message expression is undefined."); #ifndef NDEBUG - if (Visualize) CheckerState->ViewGraph(); + if (Visualize) CheckerState->ViewGraph(TrimGraph); #endif return Engine.getGraph().size(); diff --git a/Driver/ASTConsumers.cpp b/Driver/ASTConsumers.cpp index f8fbdc9173..a24cd0478e 100644 --- a/Driver/ASTConsumers.cpp +++ b/Driver/ASTConsumers.cpp @@ -593,10 +593,11 @@ namespace { Diagnostic &Diags; ASTContext* Ctx; bool Visualize; + bool TrimGraph; public: GRSimpleValsVisitor(Diagnostic &diags, const std::string& fname, - bool visualize) - : CFGVisitor(fname), Diags(diags), Visualize(visualize) {} + bool visualize, bool trim) + : CFGVisitor(fname), Diags(diags), Visualize(visualize), TrimGraph(trim){} virtual void Initialize(ASTContext &Context) { Ctx = &Context; } virtual void VisitCFG(CFG& C, FunctionDecl&); @@ -606,9 +607,9 @@ namespace { ASTConsumer* clang::CreateGRSimpleVals(Diagnostic &Diags, const std::string& FunctionName, - bool Visualize) { + bool Visualize, bool TrimGraph) { - return new GRSimpleValsVisitor(Diags, FunctionName, Visualize); + return new GRSimpleValsVisitor(Diags, FunctionName, Visualize, TrimGraph); } void GRSimpleValsVisitor::VisitCFG(CFG& C, FunctionDecl& FD) { @@ -626,13 +627,13 @@ void GRSimpleValsVisitor::VisitCFG(CFG& C, FunctionDecl& FD) { llvm::Timer T("GRSimpleVals"); T.startTimer(); - unsigned size = RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize); + unsigned size = RunGRSimpleVals(C, FD, *Ctx, Diags, false, false); T.stopTimer(); llvm::cerr << size << ' ' << T.getWallTime() << '\n'; } else { llvm::cerr << '\n'; - RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize); + RunGRSimpleVals(C, FD, *Ctx, Diags, Visualize, TrimGraph); } } diff --git a/Driver/ASTConsumers.h b/Driver/ASTConsumers.h index 3d2f9495fa..63f769f06a 100644 --- a/Driver/ASTConsumers.h +++ b/Driver/ASTConsumers.h @@ -44,7 +44,7 @@ ASTConsumer *CreateUnitValsChecker(Diagnostic &Diags); ASTConsumer *CreateGRSimpleVals(Diagnostic &Diags, const std::string& Function, - bool Visualize = false); + bool Visualize = false, bool TrimGraph = false); ASTConsumer* CreateCFRefChecker(Diagnostic &Diags, const std::string& FunctionName); diff --git a/Driver/clang.cpp b/Driver/clang.cpp index 216e669ccc..6807075912 100644 --- a/Driver/clang.cpp +++ b/Driver/clang.cpp @@ -461,6 +461,11 @@ static llvm::cl::opt AnalyzeSpecificFunction("analyze-function", llvm::cl::desc("Run analysis on specific function.")); +static llvm::cl::opt +TrimGraph("trim-path-graph", + llvm::cl::desc("Only show error-related paths in the analysis graph.")); + + //===----------------------------------------------------------------------===// // Target Triple Processing. //===----------------------------------------------------------------------===// @@ -1039,7 +1044,7 @@ static ASTConsumer* CreateASTConsumer(const std::string& InFile, return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction); case AnalysisGRSimpleValsView: - return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction, true); + return CreateGRSimpleVals(Diag, AnalyzeSpecificFunction, true, TrimGraph); case CheckerCFRef: return CreateCFRefChecker(Diag, AnalyzeSpecificFunction); diff --git a/include/clang/Analysis/Analyses/GRSimpleVals.h b/include/clang/Analysis/Analyses/GRSimpleVals.h index 81de2ac2e9..c8054ff253 100644 --- a/include/clang/Analysis/Analyses/GRSimpleVals.h +++ b/include/clang/Analysis/Analyses/GRSimpleVals.h @@ -25,7 +25,7 @@ namespace clang { /// something more elaborate as the requirements on the interface become /// clearer. The value returned is the number of nodes in the ExplodedGraph. unsigned RunGRSimpleVals(CFG& cfg, FunctionDecl& FD, ASTContext& Ctx, - Diagnostic& Diag, bool Visualize); + Diagnostic& Diag, bool Visualize, bool TrimGraph); } // end clang namespace diff --git a/include/clang/Analysis/PathSensitive/ExplodedGraph.h b/include/clang/Analysis/PathSensitive/ExplodedGraph.h index 65936399b3..b8c66368da 100644 --- a/include/clang/Analysis/PathSensitive/ExplodedGraph.h +++ b/include/clang/Analysis/PathSensitive/ExplodedGraph.h @@ -55,6 +55,7 @@ protected: } void* getPtr() const { + assert (!getFlag()); return reinterpret_cast(P & ~Mask); } @@ -73,12 +74,13 @@ protected: unsigned size() const; - bool empty() const; + bool empty() const { return size() == 0; } void addNode(ExplodedNodeImpl* N); void setFlag() { - P |= AuxFlag; + assert (P == 0); + P = AuxFlag; } bool getFlag() const { @@ -123,8 +125,8 @@ public: bool succ_empty() const { return Succs.empty(); } bool pred_empty() const { return Preds.size(); } - bool isSink() const { return Preds.getFlag(); } - void markAsSink() { Preds.setFlag(); } + bool isSink() const { return Succs.getFlag(); } + void markAsSink() { Succs.setFlag(); } }; @@ -232,6 +234,8 @@ protected: /// is intended to be used only by GRCoreEngineImpl. virtual ExplodedNodeImpl* getNodeImpl(const ProgramPoint& L, void* State, bool* IsNew) = 0; + + virtual ExplodedGraphImpl* MakeEmptyGraph() const = 0; /// addRoot - Add an untyped node to the set of roots. ExplodedNodeImpl* addRoot(ExplodedNodeImpl* V) { @@ -262,6 +266,10 @@ public: CFG& getCFG() { return cfg; } ASTContext& getContext() { return Ctx; } FunctionDecl& getFunctionDecl() { return FD; } + + ExplodedGraphImpl* Trim(ExplodedNodeImpl** NBeg, + ExplodedNodeImpl** NEnd) const; + }; template @@ -279,10 +287,15 @@ protected: AllNodesTy Nodes; protected: - virtual ExplodedNodeImpl* - getNodeImpl(const ProgramPoint& L, void* State, bool* IsNew) { + virtual ExplodedNodeImpl* getNodeImpl(const ProgramPoint& L, + void* State, bool* IsNew) { + return getNode(L, static_cast(State), IsNew); } + + virtual ExplodedGraphImpl* MakeEmptyGraph() const { + return new ExplodedGraph(cfg, FD, Ctx); + } public: ExplodedGraph(CFG& c, FunctionDecl& fd, ASTContext& ctx) @@ -362,6 +375,22 @@ public: const_eop_iterator eop_end() const { return const_cast(this)->eop_end(); } + + // Utility. + + ExplodedGraph* Trim(NodeTy** NBeg, NodeTy** NEnd) const { + + if (NBeg == NEnd) + return NULL; + + assert (NBeg < NEnd); + + ExplodedNodeImpl** NBegImpl = (ExplodedNodeImpl**) NBeg; + ExplodedNodeImpl** NEndImpl = (ExplodedNodeImpl**) NEnd; + + return static_cast(ExplodedGraphImpl::Trim(NBegImpl, + NEndImpl)); + } }; diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index a1de93e8e2..520c3732c8 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -149,7 +149,7 @@ public: /// ViewGraph - Visualize the ExplodedGraph created by executing the /// simulation. - void ViewGraph(); + void ViewGraph(bool trim = false); /// getInitialState - Return the initial state used for the root vertex /// in the ExplodedGraph. @@ -392,7 +392,8 @@ protected: /// VisitUnaryOperator - Transfer function logic for unary operators. void VisitUnaryOperator(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst); - void VisitDeref(UnaryOperator* B, NodeTy* Pred, NodeSet& Dst); + void VisitDeref(UnaryOperator* U, NodeTy* Pred, NodeSet& Dst, + bool GetLVal = false); RVal EvalCast(RVal X, QualType CastT) { if (X.isUnknownOrUndef())