[cfe-commits] [patch]interfile analysis
Ted Kremenek
kremenek at apple.com
Mon Jul 12 22:58:00 PDT 2010
On Jul 8, 2010, at 2:09 AM, Zhongxing Xu wrote:
> This patch did the following things.
> * add a Indexer to AnalysisManager
> * add a InlineCallInAnotherTU() to GRExprEngine, which tries to find
> the definition of the callee in other translation units.
> * add a new ProgramPoint: FarCallEnter which indicates we are going to
> enter a function in another translation unit. And associated builder
> classes.
> * now stop at GRFarCallEnterNodeBuilder::GenerateNode(). Here I plan
> to create a new GRExprEngine with a input state and an output state
> set. After it finishes running, add all output nodes into the WList.
Hi Zhongxing,
Thanks for your patience. I've been thinking about the design, and have included my comments inline! Let me know what you think.
Index: include/clang/Analysis/ProgramPoint.h
===================================================================
--- include/clang/Analysis/ProgramPoint.h (版本 107853)
+++ include/clang/Analysis/ProgramPoint.h (工作副本)
@@ -28,11 +28,14 @@
class LocationContext;
class FunctionDecl;
+namespace idx { class TranslationUnit; }
+
class ProgramPoint {
public:
enum Kind { BlockEdgeKind,
BlockEntranceKind,
BlockExitKind,
+ FarCallEnterKind,
PreStmtKind,
PostStmtKind,
PreLoadKind,
@@ -341,7 +344,21 @@
}
};
+class FarCallEnter : public ProgramPoint {
+public:
+ FarCallEnter(const FunctionDecl *FD, idx::TranslationUnit *TU,
+ const LocationContext *L)
+ : ProgramPoint(FD, TU, FarCallEnterKind, L, 0) {}
+ const FunctionDecl *getCallee() const {
+ return static_cast<const FunctionDecl *>(getData1());
+ }
+
+ static bool classof(const ProgramPoint *Location) {
+ return Location->getKind() == FarCallEnterKind;
+ }
+};
+
} // end namespace clang
After looking at the ProgramPoints we already have, I don't think the addition of FarCallEnter is necessary. I also think the restriction to work just on FunctionDecls isn't necessary since we have AnalysisContexts (which allow us to transparently handle BlockDecls and ObjCMethodDecls as well).
Since we already have CallEnter/CallExt program points, why not extend those to be more generic? One easy way to do this is to generalize AnalysisContext. Instead of passing a FunctionDecl to the constructor of CallEnter, why not pass an AnalysisContext object instead? This would handle the case of FunctionDecls, ObjCMethodDecls, and BlockDecls. Further, if AnalysisContext also contained a reference to the TranslationUnit, we would get the generalization we seek without the need to add another ProgramPoint. Then all future StackFrames just used the new AnalysisContext.
Index: include/clang/Checker/PathSensitive/GRExprEngine.h
===================================================================
--- include/clang/Checker/PathSensitive/GRExprEngine.h (版本 107853)
+++ include/clang/Checker/PathSensitive/GRExprEngine.h (工作副本)
@@ -196,6 +196,8 @@
/// Generate the first post callsite node.
void ProcessCallExit(GRCallExitNodeBuilder &builder);
+ void ProcessFarCallEnter(GRFarCallEnterNodeBuilder &builder);
+
/// Called by GRCoreEngine when the analysis worklist has terminated.
void ProcessEndWorklist(bool hasWorkRemaining);
@@ -473,6 +475,8 @@
const void *tag, bool isLoad);
bool InlineCall(ExplodedNodeSet &Dst, const CallExpr *CE, ExplodedNode *Pred);
+ bool InlineCallInAnotherTU(ExplodedNodeSet &Dst, const CallExpr *CE,
+ ExplodedNode *Pred);
};
If we unify CallEnter and FallCallEnter, is InlineCallInAnotherTU necessary?
} // end clang namespace
Index: include/clang/Checker/PathSensitive/GRCoreEngine.h
===================================================================
--- include/clang/Checker/PathSensitive/GRCoreEngine.h (版本 107853)
+++ include/clang/Checker/PathSensitive/GRCoreEngine.h (工作副本)
@@ -42,6 +42,7 @@
friend class GREndPathNodeBuilder;
friend class GRCallEnterNodeBuilder;
friend class GRCallExitNodeBuilder;
+ friend class GRFarCallEnterNodeBuilder;
GRSubEngine& SubEngine;
@@ -76,6 +77,8 @@
void HandleCallEnter(const CallEnter &L, const CFGBlock *Block,
unsigned Index, ExplodedNode *Pred);
void HandleCallExit(const CallExit &L, ExplodedNode *Pred);
+ void HandleFarCallEnter(const FarCallEnter &L, const CFGBlock *Block,
+ unsigned Index, ExplodedNode *Pred);
/// Get the initial state from the subengine.
const GRState* getInitialState(const LocationContext *InitLoc) {
@@ -101,6 +104,7 @@
void ProcessCallEnter(GRCallEnterNodeBuilder &Builder);
void ProcessCallExit(GRCallExitNodeBuilder &Builder);
+ void ProcessFarCallEnter(GRFarCallEnterNodeBuilder &Builder);
private:
GRCoreEngine(const GRCoreEngine&); // Do not implement.
@@ -474,6 +478,35 @@
void GenerateNode(const GRState *state, const LocationContext *LocCtx);
};
+class GRFarCallEnterNodeBuilder {
+ GRCoreEngine &Eng;
+
+ const ExplodedNode *Pred;
+
+ // The definition of callee.
+ const FunctionDecl *FD;
+
+ // The parent block of the CallExpr.
+ const CFGBlock *Block;
+
+ // The CFGBlock index of the CallExpr.
+ unsigned Index;
+
+public:
+ GRFarCallEnterNodeBuilder(GRCoreEngine &eng, const ExplodedNode *pred,
+ const FunctionDecl *fd, const CFGBlock *blk,
+ unsigned idx)
+ : Eng(eng), Pred(pred), FD(fd), Block(blk), Index(idx) {}
+ const GRState *getState() const { return Pred->getState(); }
+ const LocationContext *getLocationContext() const {
+ return Pred->getLocationContext();
+ }
+ const FunctionDecl *getCallee() const { return FD; }
+ const CFGBlock *getBlock() const { return Block; }
+ unsigned getIndex() const { return Index; }
+ void GenerateNode(const GRState *state, const LocationContext *LocCtx);
+};
+
If we unify CallEnter and FarCallEnter, can we just have one builder?
class GRCallExitNodeBuilder {
GRCoreEngine &Eng;
const ExplodedNode *Pred;
Index: include/clang/Checker/PathSensitive/GRSubEngine.h
===================================================================
--- include/clang/Checker/PathSensitive/GRSubEngine.h (版本 107853)
+++ include/clang/Checker/PathSensitive/GRSubEngine.h (工作副本)
@@ -31,6 +31,7 @@
class GREndPathNodeBuilder;
class GRCallEnterNodeBuilder;
class GRCallExitNodeBuilder;
+class GRFarCallEnterNodeBuilder;
class LocationContext;
class GRSubEngine {
@@ -73,6 +74,8 @@
// Generate the first post callsite node.
virtual void ProcessCallExit(GRCallExitNodeBuilder &builder) = 0;
+
+ virtual void ProcessFarCallEnter(GRFarCallEnterNodeBuilder &builder) = 0;
Same comment. Is this needed if have one builder?
/// Called by ConstraintManager. Used to call checker-specific
/// logic for handling assumptions on symbolic values.
Index: include/clang/Checker/PathSensitive/AnalysisManager.h
===================================================================
--- include/clang/Checker/PathSensitive/AnalysisManager.h (版本 107853)
+++ include/clang/Checker/PathSensitive/AnalysisManager.h (工作副本)
@@ -18,9 +18,15 @@
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Checker/BugReporter/BugReporter.h"
#include "clang/Checker/BugReporter/PathDiagnostic.h"
+#include "clang/Index/Program.h"
+#include "clang/Index/Indexer.h"
namespace clang {
+namespace idx {
+class TranslationUnit;
+}
+
class AnalysisManager : public BugReporterData {
AnalysisContextManager AnaCtxMgr;
LocationContextManager LocCtxMgr;
@@ -35,6 +41,9 @@
StoreManagerCreator CreateStoreMgr;
ConstraintManagerCreator CreateConstraintMgr;
+ // Provide function definitions in other translation units.
+ idx::Indexer *Idxer;
+
Seems reasonable. Maybe eventually we'll reference count idx::Indexer to make the memory management clearer.
enum AnalysisScope { ScopeTU, ScopeDecl } AScope;
// The maximum number of exploded nodes the analyzer will generate.
@@ -62,13 +71,14 @@
AnalysisManager(ASTContext &ctx, Diagnostic &diags,
const LangOptions &lang, PathDiagnosticClient *pd,
StoreManagerCreator storemgr,
- ConstraintManagerCreator constraintmgr, unsigned maxnodes,
- unsigned maxloop,
+ ConstraintManagerCreator constraintmgr,
+ idx::Indexer *idxer,
+ unsigned maxnodes, unsigned maxloop,
bool vizdot, bool vizubi, bool purge, bool eager, bool trim,
bool inlinecall)
We should add a comment that 'idxer' can be NULL.
: Ctx(ctx), Diags(diags), LangInfo(lang), PD(pd),
- CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr),
+ CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr),Idxer(idxer),
AScope(ScopeDecl), MaxNodes(maxnodes), MaxLoop(maxloop),
VisualizeEGDot(vizdot), VisualizeEGUbi(vizubi), PurgeDead(purge),
EagerlyAssume(eager), TrimGraph(trim), InlineCall(inlinecall) {}
@@ -157,6 +167,17 @@
unsigned Idx) {
return LocCtxMgr.getStackFrame(AnaCtxMgr.getContext(D), Parent, S, Blk,Idx);
}
+
+ bool isInterfileAnalysis() const {
+ return Idxer != 0;
+ }
Please add a doxygen comment. I know the rest of the methods don't have one, but it's time we started since this beast is getting more complicated.
+
+ std::pair<FunctionDecl *, idx::TranslationUnit *>
+ getFunctionDefinition(const FunctionDecl *D) {
+ idx::Entity Ent = idx::Entity::get(const_cast<FunctionDecl *>(D),
+ Idxer->getProgram());
+ return Idxer->getDefinitionFor(Ent);
+ }
};
If we have AnalysisContext contain the idx::TranslationUnit, how about "getAnalysisContext" instead?
}
Index: tools/driver/Makefile
===================================================================
--- tools/driver/Makefile (版本 107853)
+++ tools/driver/Makefile (工作副本)
@@ -28,8 +28,8 @@
LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader bitwriter codegen \
ipo selectiondag
USEDLIBS = clangFrontend.a clangDriver.a clangCodeGen.a clangSema.a \
- clangChecker.a clangAnalysis.a clangRewrite.a clangAST.a \
- clangParse.a clangLex.a clangBasic.a
+ clangChecker.a clangAnalysis.a clangIndex.a clangRewrite.a \
+ clangAST.a clangParse.a clangLex.a clangBasic.a
The CMake build system may also need to get modified.
include $(CLANG_LEVEL)/Makefile
Index: lib/Checker/AnalysisConsumer.cpp
===================================================================
--- lib/Checker/AnalysisConsumer.cpp (版本 107853)
+++ lib/Checker/AnalysisConsumer.cpp (工作副本)
@@ -172,7 +172,7 @@
Ctx = &Context;
Mgr.reset(new AnalysisManager(*Ctx, PP.getDiagnostics(),
PP.getLangOptions(), PD,
- CreateStoreMgr, CreateConstraintMgr,
+ CreateStoreMgr, CreateConstraintMgr, 0,
Opts.MaxNodes, Opts.MaxLoop,
Opts.VisualizeEGDot, Opts.VisualizeEGUbi,
Opts.PurgeDead, Opts.EagerlyAssume,
Please add a comment that the '0' is for the indexer argument, e.g. /* idexer = */ 0
Index: lib/Checker/GRCoreEngine.cpp
===================================================================
--- lib/Checker/GRCoreEngine.cpp (版本 107853)
+++ lib/Checker/GRCoreEngine.cpp (工作副本)
@@ -148,6 +148,10 @@
SubEngine.ProcessCallEnter(Builder);
}
+void GRCoreEngine::ProcessFarCallEnter(GRFarCallEnterNodeBuilder &Builder) {
+ SubEngine.ProcessFarCallEnter(Builder);
+}
+
void GRCoreEngine::ProcessCallExit(GRCallExitNodeBuilder &Builder) {
SubEngine.ProcessCallExit(Builder);
}
@@ -209,6 +213,11 @@
WU.getIndex(), Node);
break;
+ case ProgramPoint::FarCallEnterKind:
+ HandleFarCallEnter(cast<FarCallEnter>(Node->getLocation()),
+ WU.getBlock(), WU.getIndex(), Node);
+ break;
+
case ProgramPoint::CallExitKind:
HandleCallExit(cast<CallExit>(Node->getLocation()), Node);
break;
@@ -232,6 +241,13 @@
ProcessCallEnter(Builder);
}
+void GRCoreEngine::HandleFarCallEnter(const FarCallEnter &L,
+ const CFGBlock *Block,
+ unsigned Index, ExplodedNode *Pred) {
+ GRFarCallEnterNodeBuilder Builder(*this, Pred, L.getCallee(), Block, Index);
+ ProcessFarCallEnter(Builder);
+}
+
Again I don't think these are needed if we generalize AnalysisContext.
void GRCoreEngine::HandleCallExit(const CallExit &L, ExplodedNode *Pred) {
GRCallExitNodeBuilder Builder(*this, Pred);
ProcessCallExit(Builder);
@@ -435,7 +451,7 @@
assert (!N->isSink());
// Check if this node entered a callee.
- if (isa<CallEnter>(N->getLocation())) {
+ if (isa<CallEnter>(N->getLocation()) || isa<FarCallEnter>(N->getLocation())) {
Same comment.
// Still use the index of the CallExpr. It's needed to create the callee
// StackFrameContext.
Eng.WList->Enqueue(N, B, Idx);
@@ -711,6 +727,11 @@
Eng.WList->Enqueue(Node);
}
+void GRFarCallEnterNodeBuilder::GenerateNode(const GRState *state,
+ const LocationContext *LocCtx) {
+ assert(0 && "Okay, we get here. Create a new GRExprEngine with the initial state.");
+}
+
Same comment.
void GRCallExitNodeBuilder::GenerateNode(const GRState *state) {
// Get the callee's location context.
const StackFrameContext *LocCtx
Index: lib/Checker/GRExprEngine.cpp
===================================================================
--- lib/Checker/GRExprEngine.cpp (版本 107853)
+++ lib/Checker/GRExprEngine.cpp (工作副本)
@@ -18,6 +18,7 @@
#include "clang/Checker/PathSensitive/GRExprEngine.h"
#include "clang/Checker/PathSensitive/GRExprEngineBuilders.h"
#include "clang/Checker/PathSensitive/Checker.h"
+#include "clang/Index/TranslationUnit.h"
#include "clang/AST/CharUnits.h"
#include "clang/AST/ParentMap.h"
#include "clang/AST/StmtObjC.h"
@@ -1453,6 +1454,20 @@
B.GenerateNode(state, LocCtx);
}
+void GRExprEngine::ProcessFarCallEnter(GRFarCallEnterNodeBuilder &B) {
+ const FunctionDecl *FD = B.getCallee();
+ // Get the call expr.
+ const CFGBlock &Block = *B.getBlock();
+ unsigned Idx = B.getIndex();
+ Stmt *S = Block[Idx].getStmt();
+ // Create the stack frame for the callee.
+ const StackFrameContext *LocCtx = AMgr.getStackFrame(FD,
+ B.getLocationContext(),
+ S, &Block, Idx);
+ // FIXME: Marshal information to get the new state. Use the old state for now.
+ B.GenerateNode(B.getState(), LocCtx);
+}
Most of this is the same as ProcessCallEnter. Moreover, if ProcessCallEnter is generalized to take an AnalysisContext, it works for more than just FunctionDecls.
+
void GRExprEngine::ProcessCallExit(GRCallExitNodeBuilder &B) {
const GRState *state = B.getState();
const ExplodedNode *Pred = B.getPredecessor();
@@ -1897,6 +1912,29 @@
return true;
}
+bool GRExprEngine::InlineCallInAnotherTU(ExplodedNodeSet &Dst,
+ const CallExpr *CE, ExplodedNode *Pred) {
+ const GRState *state = GetState(Pred);
+ const Expr *Callee = CE->getCallee();
+ SVal L = state->getSVal(Callee);
+ const FunctionDecl *FD = L.getAsFunctionDecl();
+ if (!FD)
+ return false;
+
+ // Get the definition and translation unit of the callee.
+ FunctionDecl *FuncDef;
+ idx::TranslationUnit *TU;
+ llvm::tie(FuncDef, TU) = AMgr.getFunctionDefinition(FD);
+
+ if (!FD)
+ return false;
+ FarCallEnter Loc(FD, TU, Pred->getLocationContext());
+ ExplodedNode *N = Builder->generateNode(Loc, state, Pred);
+ if (N)
+ Dst.Add(N);
+ return true;
+}
+
This is basically the same as GRExprEngine::InlineCall(), so just generalizing AnalysisContext should make this extra logic unnecessary.
void GRExprEngine::VisitCall(CallExpr* CE, ExplodedNode* Pred,
CallExpr::arg_iterator AI,
CallExpr::arg_iterator AE,
@@ -1979,8 +2017,10 @@
else if (AMgr.shouldInlineCall() && InlineCall(Dst, CE, *DI)) {
// Callee is inlined. We shouldn't do post call checking.
return;
- }
- else {
+ } else if (AMgr.isInterfileAnalysis() &&
+ InlineCallInAnotherTU(Dst, CE, *DI)) {
+ return;
+ } else {
If we generalize AnalysosContext, this condition would be folded into the previous 'else if'.
for (ExplodedNodeSet::iterator DI_Checker = DstChecker.begin(),
DE_Checker = DstChecker.end();
DI_Checker != DE_Checker; ++DI_Checker) {
Index: examples/wpa/clang-wpa.cpp
===================================================================
--- examples/wpa/clang-wpa.cpp (版本 107853)
+++ examples/wpa/clang-wpa.cpp (工作副本)
@@ -129,7 +129,7 @@
AnalysisManager AMgr(TU->getASTContext(), PP.getDiagnostics(),
PP.getLangOptions(), /* PathDiagnostic */ 0,
CreateRegionStoreManager,
- CreateRangeConstraintManager,
+ CreateRangeConstraintManager, &Idxer,
/* MaxNodes */ 300000, /* MaxLoop */ 3,
/* VisualizeEG */ false, /* VisualizeEGUbi */ false,
/* PurgeDead */ true, /* EagerlyAssume */ false,
More information about the cfe-commits
mailing list