[cfe-commits] r64881 - in /cfe/trunk: include/clang/Analysis/PathSensitive/BugReporter.h include/clang/Analysis/PathSensitive/ExplodedGraph.h lib/Analysis/BugReporter.cpp lib/Analysis/CFRefCount.cpp lib/Analysis/ExplodedGraph.cpp
Ted Kremenek
kremenek at apple.com
Tue Feb 17 19:48:15 PST 2009
Author: kremenek
Date: Tue Feb 17 21:48:14 2009
New Revision: 64881
URL: http://llvm.org/viewvc/llvm-project?rev=64881&view=rev
Log:
Hooked up the necessary machinery to allow the retain/release checker reference
back to the summary used when evaluating the statement associated with a
simulation node. This is now being used to help improve the checker's
diagnostics. To get things started, the checker now emits a path diagnostic
indicating that 'autorelease' is a no-op in GC mode.
Some of these changes are exposing further grossness in the interface between
BugReporter and the ExplodedGraph::Trim facilities. These really need to be
cleaned up one day.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h
cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h
cfe/trunk/lib/Analysis/BugReporter.cpp
cfe/trunk/lib/Analysis/CFRefCount.cpp
cfe/trunk/lib/Analysis/ExplodedGraph.cpp
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h?rev=64881&r1=64880&r2=64881&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h Tue Feb 17 21:48:14 2009
@@ -61,6 +61,13 @@
}
public:
+ class NodeResolver {
+ public:
+ virtual ~NodeResolver() {}
+ virtual const ExplodedNode<GRState>*
+ getOriginalNode(const ExplodedNode<GRState>* N) = 0;
+ };
+
BugReport(BugType& bt, const char* desc, const ExplodedNode<GRState> *n)
: BT(bt), Description(desc), EndNode(n) {}
@@ -101,7 +108,8 @@
virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
- BugReporter& BR);
+ BugReporter& BR,
+ NodeResolver& NR);
};
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h?rev=64881&r1=64880&r2=64881&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/ExplodedGraph.h Tue Feb 17 21:48:14 2009
@@ -300,7 +300,9 @@
ExplodedGraphImpl* Trim(const ExplodedNodeImpl* const * NBeg,
const ExplodedNodeImpl* const * NEnd,
- InterExplodedGraphMapImpl *M) const;
+ InterExplodedGraphMapImpl *M,
+ llvm::DenseMap<const void*, const void*> *InverseMap)
+ const;
};
class InterExplodedGraphMapImpl {
@@ -447,7 +449,8 @@
}
std::pair<ExplodedGraph*, InterExplodedGraphMap<STATE>*>
- Trim(const NodeTy* const* NBeg, const NodeTy* const* NEnd) const {
+ Trim(const NodeTy* const* NBeg, const NodeTy* const* NEnd,
+ llvm::DenseMap<const void*, const void*> *InverseMap = 0) const {
if (NBeg == NEnd)
return std::make_pair((ExplodedGraph*) 0,
@@ -463,7 +466,8 @@
llvm::OwningPtr<InterExplodedGraphMap<STATE> >
M(new InterExplodedGraphMap<STATE>());
- ExplodedGraphImpl* G = ExplodedGraphImpl::Trim(NBegImpl, NEndImpl, M.get());
+ ExplodedGraphImpl* G = ExplodedGraphImpl::Trim(NBegImpl, NEndImpl, M.get(),
+ InverseMap);
return std::make_pair(static_cast<ExplodedGraph*>(G), M.take());
}
Modified: cfe/trunk/lib/Analysis/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BugReporter.cpp?rev=64881&r1=64880&r2=64881&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BugReporter.cpp (original)
+++ cfe/trunk/lib/Analysis/BugReporter.cpp Tue Feb 17 21:48:14 2009
@@ -167,7 +167,8 @@
PathDiagnosticPiece* BugReport::VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
- BugReporter& BR) {
+ BugReporter& BR,
+ NodeResolver &NR) {
return NULL;
}
@@ -226,7 +227,10 @@
// PathDiagnostics generation.
//===----------------------------------------------------------------------===//
-static std::pair<ExplodedGraph<GRState>*,
+typedef llvm::DenseMap<const ExplodedNode<GRState>*,
+ const ExplodedNode<GRState>*> NodeBackMap;
+
+static std::pair<std::pair<ExplodedGraph<GRState>*, NodeBackMap*>,
std::pair<ExplodedNode<GRState>*, unsigned> >
MakeReportGraph(const ExplodedGraph<GRState>* G,
const ExplodedNode<GRState>** NStart,
@@ -238,7 +242,9 @@
// path length.
ExplodedGraph<GRState>* GTrim;
InterExplodedGraphMap<GRState>* NMap;
- llvm::tie(GTrim, NMap) = G->Trim(NStart, NEnd);
+
+ llvm::DenseMap<const void*, const void*> InverseMap;
+ llvm::tie(GTrim, NMap) = G->Trim(NStart, NEnd, &InverseMap);
// Create owning pointers for GTrim and NMap just to ensure that they are
// released when this function exists.
@@ -262,8 +268,8 @@
// Create a new (third!) graph with a single path. This is the graph
// that will be returned to the caller.
ExplodedGraph<GRState> *GNew =
- new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
- GTrim->getContext());
+ new ExplodedGraph<GRState>(GTrim->getCFG(), GTrim->getCodeDecl(),
+ GTrim->getContext());
// Sometimes the trimmed graph can contain a cycle. Perform a reverse DFS
// to the root node, and then construct a new graph that contains only
@@ -298,6 +304,7 @@
// Now walk from the root down the DFS path, always taking the successor
// with the lowest number.
ExplodedNode<GRState> *Last = 0, *First = 0;
+ NodeBackMap *BM = new NodeBackMap();
for ( N = Root ;;) {
// Lookup the number associated with the current node.
@@ -307,7 +314,12 @@
// Create the equivalent node in the new graph with the same state
// and location.
ExplodedNode<GRState>* NewN =
- GNew->getNode(N->getLocation(), N->getState());
+ GNew->getNode(N->getLocation(), N->getState());
+
+ // Store the mapping to the original node.
+ llvm::DenseMap<const void*, const void*>::iterator IMitr=InverseMap.find(N);
+ assert(IMitr != InverseMap.end() && "No mapping to original node.");
+ (*BM)[NewN] = (const ExplodedNode<GRState>*) IMitr->second;
// Link up the new node with the previous node.
if (Last)
@@ -344,7 +356,8 @@
}
assert (First);
- return std::make_pair(GNew, std::make_pair(First, NodeIndex));
+ return std::make_pair(std::make_pair(GNew, BM),
+ std::make_pair(First, NodeIndex));
}
static const VarDecl*
@@ -527,6 +540,20 @@
};
} // end anonymous namespace
+namespace {
+class VISIBILITY_HIDDEN NodeMapClosure : public BugReport::NodeResolver {
+ NodeBackMap& M;
+public:
+ NodeMapClosure(NodeBackMap *m) : M(*m) {}
+ ~NodeMapClosure() {}
+
+ const ExplodedNode<GRState>* getOriginalNode(const ExplodedNode<GRState>* N) {
+ NodeBackMap::iterator I = M.find(N);
+ return I == M.end() ? 0 : I->second;
+ }
+};
+}
+
void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
BugReportEquivClass& EQ) {
@@ -542,7 +569,7 @@
// Construct a new graph that contains only a single path from the error
// node to a root.
- const std::pair<ExplodedGraph<GRState>*,
+ const std::pair<std::pair<ExplodedGraph<GRState>*, NodeBackMap*>,
std::pair<ExplodedNode<GRState>*, unsigned> >&
GPair = MakeReportGraph(&getGraph(), &Nodes[0], &Nodes[0] + Nodes.size());
@@ -554,7 +581,8 @@
assert(R && "No original report found for sliced graph.");
- llvm::OwningPtr<ExplodedGraph<GRState> > ReportGraph(GPair.first);
+ llvm::OwningPtr<ExplodedGraph<GRState> > ReportGraph(GPair.first.first);
+ llvm::OwningPtr<NodeBackMap> BackMap(GPair.first.second);
const ExplodedNode<GRState> *N = GPair.second.first;
// Start building the path diagnostic...
@@ -568,6 +596,7 @@
ASTContext& Ctx = getContext();
SourceManager& SMgr = Ctx.getSourceManager();
+ NodeMapClosure NMC(BackMap.get());
while (NextNode) {
@@ -750,7 +779,8 @@
}
}
- if (PathDiagnosticPiece* p = R->VisitNode(N, NextNode, *ReportGraph, *this))
+ if (PathDiagnosticPiece* p = R->VisitNode(N, NextNode, *ReportGraph, *this,
+ NMC))
PD.push_front(p);
if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=64881&r1=64880&r2=64881&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Tue Feb 17 21:48:14 2009
@@ -1283,8 +1283,11 @@
};
private:
+ typedef llvm::DenseMap<const GRExprEngine::NodeTy*, const RetainSummary*>
+ SummaryLogTy;
+
RetainSummaryManager Summaries;
- llvm::DenseMap<const GRExprEngine::NodeTy*, const RetainSummary*> SummaryLog;
+ SummaryLogTy SummaryLog;
const LangOptions& LOpts;
BugType *useAfterRelease, *releaseNotOwned;
@@ -1332,6 +1335,11 @@
bool isGCEnabled() const { return Summaries.isGCEnabled(); }
const LangOptions& getLangOptions() const { return LOpts; }
+ const RetainSummary *getSummaryOfNode(const ExplodedNode<GRState> *N) const {
+ SummaryLogTy::const_iterator I = SummaryLog.find(N);
+ return I == SummaryLog.end() ? 0 : I->second;
+ }
+
// Calls.
void EvalSummary(ExplodedNodeSet<GRState>& Dst,
@@ -2087,9 +2095,11 @@
class VISIBILITY_HIDDEN CFRefReport : public RangedBugReport {
protected:
SymbolRef Sym;
+ const CFRefCount &TF;
public:
- CFRefReport(CFRefBug& D, ExplodedNode<GRState> *n, SymbolRef sym)
- : RangedBugReport(D, D.getDescription(), n), Sym(sym) {}
+ CFRefReport(CFRefBug& D, const CFRefCount &tf,
+ ExplodedNode<GRState> *n, SymbolRef sym)
+ : RangedBugReport(D, D.getDescription(), n), Sym(sym), TF(tf) {}
virtual ~CFRefReport() {}
@@ -2119,14 +2129,16 @@
PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
- BugReporter& BR);
+ BugReporter& BR,
+ NodeResolver& NR);
};
class VISIBILITY_HIDDEN CFRefLeakReport : public CFRefReport {
SourceLocation AllocSite;
const MemRegion* AllocBinding;
public:
- CFRefLeakReport(CFRefBug& D, ExplodedNode<GRState> *n, SymbolRef sym,
+ CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
+ ExplodedNode<GRState> *n, SymbolRef sym,
GRExprEngine& Eng);
PathDiagnosticPiece* getEndPath(BugReporter& BR,
@@ -2215,7 +2227,8 @@
PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
- BugReporter& BR) {
+ BugReporter& BR,
+ NodeResolver& NR) {
// Check if the type state has changed.
GRStateManager &StMgr = cast<GRBugReporter>(BR).getStateManager();
@@ -2228,6 +2241,8 @@
const RefVal& CurrV = *CurrT;
const RefVal* PrevT = PrevSt.get<RefBindings>(Sym);
+ // This is the allocation site since the previous node had no bindings
+ // for this symbol.
if (!PrevT) {
std::string sbuf;
llvm::raw_string_ostream os(sbuf);
@@ -2277,56 +2292,112 @@
return P;
}
+
+ // Create a string buffer to constain all the useful things we want
+ // to tell the user.
+ std::string sbuf;
+ llvm::raw_string_ostream os(sbuf);
+ // Consult the summary to see if there is something special we
+ // should tell the user.
+ if (const RetainSummary *Summ = TF.getSummaryOfNode(NR.getOriginalNode(N))) {
+ // We only have summaries attached to nodes after evaluating CallExpr and
+ // ObjCMessageExprs.
+ Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
+
+ llvm::SmallVector<ArgEffect, 2> AEffects;
+
+ if (CallExpr *CE = dyn_cast<CallExpr>(S)) {
+ // Iterate through the parameter expressions and see if the symbol
+ // was ever passed as an argument.
+ unsigned i = 0;
+
+ for (CallExpr::arg_iterator AI=CE->arg_begin(), AE=CE->arg_end();
+ AI!=AE; ++AI, ++i) {
+
+ // Retrieve the value of the arugment.
+ SVal X = CurrSt.GetSVal(*AI);
+
+ // Is it the symbol we're interested in?
+ if (!isa<loc::SymbolVal>(X) ||
+ Sym != cast<loc::SymbolVal>(X).getSymbol())
+ continue;
+
+ // We have an argument. Get the effect!
+ AEffects.push_back(Summ->getArg(i));
+ }
+ }
+ else if (ObjCMessageExpr *ME = dyn_cast<ObjCMessageExpr>(S)) {
+ if (Expr *receiver = ME->getReceiver()) {
+ SVal RetV = CurrSt.GetSVal(receiver);
+ if (isa<loc::SymbolVal>(RetV) &&
+ Sym == cast<loc::SymbolVal>(RetV).getSymbol()) {
+ // The symbol we are tracking is the receiver.
+ AEffects.push_back(Summ->getReceiverEffect());
+ }
+ }
+ }
+
+ // Emit diagnostics for the argument effects (if any).
+ // FIXME: The typestate logic below should also be folded into
+ // this block.
+ for (llvm::SmallVectorImpl<ArgEffect>::iterator I=AEffects.begin(),
+ E=AEffects.end(); I != E; ++I) {
+
+ // Did we do an 'autorelease' in GC mode?
+ if (TF.isGCEnabled() && *I == Autorelease) {
+ os << "In GC mode an 'autorelease' has no effect.";
+ continue;
+ }
+ }
+ }
+
// Determine if the typestate has changed.
RefVal PrevV = *PrevT;
- if (PrevV == CurrV)
- return NULL;
-
- // The typestate has changed.
- std::string sbuf;
- llvm::raw_string_ostream os(sbuf);
-
- switch (CurrV.getKind()) {
- case RefVal::Owned:
- case RefVal::NotOwned:
+ if (!(PrevV == CurrV)) // The typestate has changed.
+ switch (CurrV.getKind()) {
+ case RefVal::Owned:
+ case RefVal::NotOwned:
- if (PrevV.getCount() == CurrV.getCount())
- return 0;
-
- if (PrevV.getCount() > CurrV.getCount())
- os << "Reference count decremented.";
- else
- os << "Reference count incremented.";
-
- if (unsigned Count = CurrV.getCount()) {
- os << " Object has +" << Count;
+ if (PrevV.getCount() == CurrV.getCount())
+ return 0;
- if (Count > 1)
- os << " retain counts.";
+ if (PrevV.getCount() > CurrV.getCount())
+ os << "Reference count decremented.";
else
- os << " retain count.";
- }
-
- break;
-
- case RefVal::Released:
- os << "Object released.";
- break;
-
- case RefVal::ReturnedOwned:
- os << "Object returned to caller as an owning reference (single retain "
- "count transferred to caller).";
- break;
-
- case RefVal::ReturnedNotOwned:
- os << "Object returned to caller with a +0 (non-owning) retain count.";
- break;
+ os << "Reference count incremented.";
+
+ if (unsigned Count = CurrV.getCount()) {
+ os << " Object has +" << Count;
+
+ if (Count > 1)
+ os << " retain counts.";
+ else
+ os << " retain count.";
+ }
+
+ break;
+
+ case RefVal::Released:
+ os << "Object released.";
+ break;
+
+ case RefVal::ReturnedOwned:
+ os << "Object returned to caller as an owning reference (single retain "
+ "count transferred to caller).";
+ break;
+
+ case RefVal::ReturnedNotOwned:
+ os << "Object returned to caller with a +0 (non-owning) retain count.";
+ break;
- default:
- return NULL;
- }
+ default:
+ return NULL;
+ }
+
+ if (os.str().empty())
+ return 0; // We have nothing to say!
Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
FullSourceLoc Pos(S->getLocStart(), BR.getContext().getSourceManager());
@@ -2512,9 +2583,10 @@
}
-CFRefLeakReport::CFRefLeakReport(CFRefBug& D, ExplodedNode<GRState> *n,
+CFRefLeakReport::CFRefLeakReport(CFRefBug& D, const CFRefCount &tf,
+ ExplodedNode<GRState> *n,
SymbolRef sym, GRExprEngine& Eng)
- : CFRefReport(D, n, sym)
+ : CFRefReport(D, tf, n, sym)
{
// Most bug reports are cached at the location where they occured.
@@ -2584,7 +2656,7 @@
CFRefBug *BT = static_cast<CFRefBug*>(I->second ? leakAtReturn
: leakWithinFunction);
assert(BT && "BugType not initialized.");
- CFRefLeakReport* report = new CFRefLeakReport(*BT, N, I->first, Eng);
+ CFRefLeakReport* report = new CFRefLeakReport(*BT, *this, N, I->first, Eng);
BR->EmitReport(report);
}
}
@@ -2633,7 +2705,7 @@
CFRefBug *BT = static_cast<CFRefBug*>(I->second ? leakAtReturn
: leakWithinFunction);
assert(BT && "BugType not initialized.");
- CFRefLeakReport* report = new CFRefLeakReport(*BT, N, I->first, Eng);
+ CFRefLeakReport* report = new CFRefLeakReport(*BT, *this, N, I->first, Eng);
BR->EmitReport(report);
}
}
@@ -2658,7 +2730,7 @@
BT = static_cast<CFRefBug*>(releaseNotOwned);
}
- CFRefReport *report = new CFRefReport(*BT, N, Sym);
+ CFRefReport *report = new CFRefReport(*BT, *this, N, Sym);
report->addRange(ErrorExpr->getSourceRange());
BR->EmitReport(report);
}
Modified: cfe/trunk/lib/Analysis/ExplodedGraph.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExplodedGraph.cpp?rev=64881&r1=64880&r2=64881&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ExplodedGraph.cpp (original)
+++ cfe/trunk/lib/Analysis/ExplodedGraph.cpp Tue Feb 17 21:48:14 2009
@@ -123,7 +123,9 @@
ExplodedGraphImpl*
ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources,
const ExplodedNodeImpl* const* EndSources,
- InterExplodedGraphMapImpl* M) const {
+ InterExplodedGraphMapImpl* M,
+ llvm::DenseMap<const void*, const void*> *InverseMap)
+const {
typedef llvm::DenseMap<const ExplodedNodeImpl*,
const ExplodedNodeImpl*> Pass1Ty;
@@ -249,6 +251,7 @@
ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State, NULL);
Pass2[N] = NewN;
+ if (InverseMap) (*InverseMap)[NewN] = N;
if (N->Preds.empty())
G->addRoot(NewN);
More information about the cfe-commits
mailing list