[cfe-commits] r71135 - in /cfe/trunk: include/clang/Analysis/PathSensitive/BugReporter.h lib/Analysis/BugReporter.cpp lib/Analysis/GRExprEngineInternalChecks.cpp
Ted Kremenek
kremenek at apple.com
Wed May 6 17:45:33 PDT 2009
Author: kremenek
Date: Wed May 6 19:45:33 2009
New Revision: 71135
URL: http://llvm.org/viewvc/llvm-project?rev=71135&view=rev
Log:
Add preliminary support for enhancing null-pointer dereference diagnostics.
Modified:
cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h
cfe/trunk/lib/Analysis/BugReporter.cpp
cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.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=71135&r1=71134&r2=71135&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h (original)
+++ cfe/trunk/include/clang/Analysis/PathSensitive/BugReporter.h Wed May 6 19:45:33 2009
@@ -50,7 +50,7 @@
virtual ~BugReporterVisitor();
virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
- BugReporterContext& BR) = 0;
+ BugReporterContext& BRC) = 0;
virtual bool isOwnedByReporterContext() { return true; }
};
@@ -115,7 +115,7 @@
}
// FIXME: Perhaps move this into a subclass.
- virtual PathDiagnosticPiece* getEndPath(BugReporterContext& BR,
+ virtual PathDiagnosticPiece* getEndPath(BugReporterContext& BRC,
const ExplodedNode<GRState>* N);
/// getLocation - Return the "definitive" location of the reported bug.
@@ -130,15 +130,10 @@
virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
const ExplodedNode<GRState>* PrevN,
- BugReporterContext& BR);
-
- /*
- virtual PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
- const ExplodedNode<GRState>* PrevN,
- const ExplodedGraph<GRState>& G,
- BugReporterContext& BR,
- NodeResolver& NR);
- */
+ BugReporterContext& BR);
+
+ virtual void registerInitialVisitors(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N) {}
};
//===----------------------------------------------------------------------===//
@@ -424,6 +419,10 @@
return BR.getStateManager();
}
+ ValueManager& getValueManager() {
+ return getStateManager().getValueManager();
+ }
+
ASTContext& getASTContext() {
return BR.getContext();
}
Modified: cfe/trunk/lib/Analysis/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BugReporter.cpp?rev=71135&r1=71134&r2=71135&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/BugReporter.cpp (original)
+++ cfe/trunk/lib/Analysis/BugReporter.cpp Wed May 6 19:45:33 2009
@@ -710,10 +710,12 @@
}
}
- for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
- E = PDB.visitor_end(); I!=E; ++I) {
- if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB))
- PD.push_front(p);
+ if (NextNode) {
+ for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
+ E = PDB.visitor_end(); I!=E; ++I) {
+ if (PathDiagnosticPiece* p = (*I)->VisitNode(N, NextNode, PDB))
+ PD.push_front(p);
+ }
}
if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
@@ -1052,53 +1054,58 @@
NextNode = GetPredecessorNode(N);
ProgramPoint P = N->getLocation();
- // Block edges.
- if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
- const CFGBlock &Blk = *BE->getSrc();
- const Stmt *Term = Blk.getTerminator();
-
- if (Term)
- EB.addContext(Term);
-
- // Are we jumping to the head of a loop? Add a special diagnostic.
- if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
+ do {
+ // Block edges.
+ if (const BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+ const CFGBlock &Blk = *BE->getSrc();
+ const Stmt *Term = Blk.getTerminator();
- PathDiagnosticLocation L(Loop, PDB.getSourceManager());
- PathDiagnosticEventPiece *p =
- new PathDiagnosticEventPiece(L,
- "Looping back to the head of the loop");
-
- EB.addEdge(p->getLocation(), true);
- PD.push_front(p);
-
- if (!Term) {
- const CompoundStmt *CS = NULL;
- if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))
- CS = dyn_cast<CompoundStmt>(FS->getBody());
- else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))
- CS = dyn_cast<CompoundStmt>(WS->getBody());
-
- if (CS)
- EB.rawAddEdge(PathDiagnosticLocation(CS->getRBracLoc(),
- PDB.getSourceManager()));
+ if (Term)
+ EB.addContext(Term);
+
+ // Are we jumping to the head of a loop? Add a special diagnostic.
+ if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
+
+ PathDiagnosticLocation L(Loop, PDB.getSourceManager());
+ PathDiagnosticEventPiece *p =
+ new PathDiagnosticEventPiece(L,
+ "Looping back to the head of the loop");
+
+ EB.addEdge(p->getLocation(), true);
+ PD.push_front(p);
+
+ if (!Term) {
+ const CompoundStmt *CS = NULL;
+ if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))
+ CS = dyn_cast<CompoundStmt>(FS->getBody());
+ else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))
+ CS = dyn_cast<CompoundStmt>(WS->getBody());
+
+ if (CS)
+ EB.rawAddEdge(PathDiagnosticLocation(CS->getRBracLoc(),
+ PDB.getSourceManager()));
+ }
}
+
+ break;
}
-
- continue;
- }
- if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {
- if (const Stmt* S = BE->getFirstStmt()) {
- if (IsControlFlowExpr(S)) {
- // Add the proper context for '&&', '||', and '?'.
- EB.addContext(S);
- }
- else
- EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
- }
+ if (const BlockEntrance *BE = dyn_cast<BlockEntrance>(&P)) {
+ if (const Stmt* S = BE->getFirstStmt()) {
+ if (IsControlFlowExpr(S)) {
+ // Add the proper context for '&&', '||', and '?'.
+ EB.addContext(S);
+ }
+ else
+ EB.addExtendedContext(PDB.getEnclosingStmtLocation(S).asStmt());
+ }
+ break;
+ }
+ } while (0);
+
+ if (!NextNode)
continue;
- }
for (BugReporterContext::visitor_iterator I = PDB.visitor_begin(),
E = PDB.visitor_end(); I!=E; ++I) {
@@ -1506,7 +1513,8 @@
PD.push_back(Piece);
else
return;
-
+
+ R->registerInitialVisitors(PDB, N);
switch (PDB.getGenerationScheme()) {
case PathDiagnosticClient::Extensive:
Modified: cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp?rev=71135&r1=71134&r2=71135&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp (original)
+++ cfe/trunk/lib/Analysis/GRExprEngineInternalChecks.cpp Wed May 6 19:45:33 2009
@@ -14,6 +14,7 @@
#include "clang/Analysis/PathSensitive/BugReporter.h"
#include "clang/Analysis/PathSensitive/GRExprEngine.h"
+#include "clang/Analysis/PathDiagnostic.h"
#include "clang/Basic/SourceManager.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/raw_ostream.h"
@@ -35,10 +36,28 @@
}
//===----------------------------------------------------------------------===//
+// Forward declarations for bug reporter visitors.
+//===----------------------------------------------------------------------===//
+
+static void registerTrackNullValue(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N);
+
+//===----------------------------------------------------------------------===//
// Bug Descriptions.
//===----------------------------------------------------------------------===//
namespace {
+
+class VISIBILITY_HIDDEN BuiltinBugReport : public BugReport {
+public:
+ BuiltinBugReport(BugType& bt, const char* desc,
+ const ExplodedNode<GRState> *n)
+ : BugReport(bt, desc, n) {}
+
+ void registerInitialVisitors(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N);
+};
+
class VISIBILITY_HIDDEN BuiltinBug : public BugType {
GRExprEngine &Eng;
protected:
@@ -54,13 +73,25 @@
void FlushReports(BugReporter& BR) { FlushReportsImpl(BR, Eng); }
- template <typename ITER>
- void Emit(BugReporter& BR, ITER I, ITER E) {
- for (; I != E; ++I) BR.EmitReport(new BugReport(*this, desc.c_str(),
- GetNode(I)));
- }
+ virtual void registerInitialVisitors(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N,
+ BuiltinBugReport *R) {}
+
+ template <typename ITER> void Emit(BugReporter& BR, ITER I, ITER E);
};
+
+template <typename ITER>
+void BuiltinBug::Emit(BugReporter& BR, ITER I, ITER E) {
+ for (; I != E; ++I) BR.EmitReport(new BuiltinBugReport(*this, desc.c_str(),
+ GetNode(I)));
+}
+
+void BuiltinBugReport::registerInitialVisitors(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N) {
+ static_cast<BuiltinBug&>(getBugType()).registerInitialVisitors(BRC, N, this);
+}
+
class VISIBILITY_HIDDEN NullDeref : public BuiltinBug {
public:
NullDeref(GRExprEngine* eng)
@@ -69,6 +100,12 @@
void FlushReportsImpl(BugReporter& BR, GRExprEngine& Eng) {
Emit(BR, Eng.null_derefs_begin(), Eng.null_derefs_end());
}
+
+ void registerInitialVisitors(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N,
+ BuiltinBugReport *R) {
+ registerTrackNullValue(BRC, N);
+ }
};
class VISIBILITY_HIDDEN NilReceiverStructRet : public BugType {
@@ -484,6 +521,124 @@
} // end anonymous namespace
//===----------------------------------------------------------------------===//
+// Definitions for bug reporter visitors.
+//===----------------------------------------------------------------------===//
+
+namespace {
+class VISIBILITY_HIDDEN TrackConstraintBRVisitor : public BugReporterVisitor {
+ SVal Constraint;
+ const bool Assumption;
+ bool isSatisfied;
+public:
+ TrackConstraintBRVisitor(SVal constraint, bool assumption)
+ : Constraint(constraint), Assumption(assumption), isSatisfied(false) {}
+
+ PathDiagnosticPiece* VisitNode(const ExplodedNode<GRState>* N,
+ const ExplodedNode<GRState>* PrevN,
+ BugReporterContext& BRC) {
+ if (isSatisfied)
+ return NULL;
+
+ // Check if in the previous state it was feasible for this constraint
+ // to *not* be true.
+
+ GRStateManager &StateMgr = BRC.getStateManager();
+ bool isFeasible = false;
+ if (StateMgr.Assume(PrevN->getState(), Constraint, !Assumption,
+ isFeasible)) {
+ assert(isFeasible); // Eventually we don't need 'isFeasible'.
+
+ isSatisfied = true;
+
+ // As a sanity check, make sure that the negation of the constraint
+ // was infeasible in the current state. If it is feasible, we somehow
+ // missed the transition point.
+ isFeasible = false;
+ if (StateMgr.Assume(N->getState(), Constraint, !Assumption,
+ isFeasible)) {
+ assert(isFeasible);
+ return NULL;
+ }
+
+ // We found the transition point for the constraint. We now need to
+ // pretty-print the constraint. (work-in-progress)
+ std::string sbuf;
+ llvm::raw_string_ostream os(sbuf);
+
+ if (isa<Loc>(Constraint)) {
+ os << "Assuming pointer value is ";
+ os << (Assumption ? "non-NULL" : "NULL");
+ }
+
+ if (os.str().empty())
+ return NULL;
+
+ // FIXME: Refactor this into BugReporterContext.
+ Stmt *S = 0;
+ ProgramPoint P = N->getLocation();
+
+ if (BlockEdge *BE = dyn_cast<BlockEdge>(&P)) {
+ CFGBlock *BSrc = BE->getSrc();
+ S = BSrc->getTerminatorCondition();
+ }
+ else if (PostStmt *PS = dyn_cast<PostStmt>(&P)) {
+ S = PS->getStmt();
+ }
+
+ if (!S)
+ return NULL;
+
+ // Construct a new PathDiagnosticPiece.
+ PathDiagnosticLocation L(S, BRC.getSourceManager());
+ return new PathDiagnosticEventPiece(L, os.str());
+ }
+
+ return NULL;
+ }
+};
+} // end anonymous namespace
+
+static void registerTrackConstraint(BugReporterContext& BRC, SVal Constraint,
+ bool Assumption) {
+ BRC.addVisitor(new TrackConstraintBRVisitor(Constraint, Assumption));
+}
+
+static void registerTrackNullValue(BugReporterContext& BRC,
+ const ExplodedNode<GRState>* N) {
+
+ ProgramPoint P = N->getLocation();
+ PostStmt *PS = dyn_cast<PostStmt>(&P);
+
+ if (!PS)
+ return;
+
+ Stmt *S = PS->getStmt();
+
+ if (ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(S)) {
+ S = AE->getBase();
+ }
+
+ SVal V = BRC.getStateManager().GetSValAsScalarOrLoc(N->getState(), S);
+
+ // Uncomment this to find cases where we aren't properly getting the
+ // base value that was dereferenced.
+ // assert(!V.isUnknownOrUndef());
+
+ // For now just track when a symbolic value became null.
+ if (loc::MemRegionVal *L = dyn_cast<loc::MemRegionVal>(&V)) {
+ const SubRegion *R = cast<SubRegion>(L->getRegion());
+ while (R && !isa<SymbolicRegion>(R)) {
+ R = dyn_cast<SubRegion>(R->getSuperRegion());
+ }
+
+ if (R) {
+ assert(isa<SymbolicRegion>(R));
+ registerTrackConstraint(BRC, loc::MemRegionVal(R), false);
+ }
+ }
+}
+
+//===----------------------------------------------------------------------===//
// Check registration.
//===----------------------------------------------------------------------===//
More information about the cfe-commits
mailing list