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.

llvm-svn: 64881
This commit is contained in:
Ted Kremenek 2009-02-18 03:48:14 +00:00
parent 617e89231d
commit 48d1645179
5 changed files with 184 additions and 67 deletions

View File

@ -61,6 +61,13 @@ protected:
}
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 @@ public:
virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
const ExplodedGraph<GRState>& G,
BugReporter& BR);
BugReporter& BR,
NodeResolver& NR);
};
//===----------------------------------------------------------------------===//

View File

@ -300,7 +300,9 @@ public:
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 @@ public:
}
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 @@ public:
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());
}

View File

@ -167,7 +167,8 @@ SourceLocation BugReport::getLocation() const {
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 @@ void BugReporter::FlushReports() {
// 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 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
// 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 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
// 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 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
// 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 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
// 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 @@ MakeReportGraph(const ExplodedGraph<GRState>* G,
}
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 @@ public:
};
} // 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 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
// 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 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
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 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
ASTContext& Ctx = getContext();
SourceManager& SMgr = Ctx.getSourceManager();
NodeMapClosure NMC(BackMap.get());
while (NextNode) {
@ -750,7 +779,8 @@ void GRBugReporter::GeneratePathDiagnostic(PathDiagnostic& PD,
}
}
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)) {

View File

@ -1283,8 +1283,11 @@ public:
};
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 @@ public:
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 @@ namespace {
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 @@ namespace {
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 @@ std::pair<const char**,const char**> CFRefReport::getExtraDescriptiveText() {
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 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
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 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode<GRState>* N,
return P;
}
// Determine if the typestate has changed.
RefVal PrevV = *PrevT;
if (PrevV == CurrV)
return NULL;
// The typestate has changed.
// 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);
switch (CurrV.getKind()) {
case RefVal::Owned:
case RefVal::NotOwned:
// 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) {
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;
// 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;
if (Count > 1)
os << " retain counts.";
else
os << " retain count.";
// 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());
}
}
}
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;
// 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) {
default:
return NULL;
// 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)) // 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 (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;
}
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::getEndPath(BugReporter& br, const ExplodedNode<GRState>* EndN){
}
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 @@ void CFRefCount::EvalEndPath(GRExprEngine& Eng,
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 @@ void CFRefCount::EvalDeadSymbols(ExplodedNodeSet<GRState>& Dst,
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 @@ void CFRefCount::ProcessNonLeakError(ExplodedNodeSet<GRState>& Dst,
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);
}

View File

@ -123,7 +123,9 @@ ExplodedNodeImpl::NodeGroup::~NodeGroup() {
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 @@ ExplodedGraphImpl::Trim(const ExplodedNodeImpl* const* BeginSources,
ExplodedNodeImpl* NewN = G->getNodeImpl(N->getLocation(), N->State, NULL);
Pass2[N] = NewN;
if (InverseMap) (*InverseMap)[NewN] = N;
if (N->Preds.empty())
G->addRoot(NewN);