[cfe-commits] r112839 - in /cfe/trunk: include/clang/Checker/BugReporter/BugReporter.h lib/Checker/BugReporterVisitors.cpp lib/Checker/IdempotentOperationChecker.cpp
Tom Care
tcare at apple.com
Thu Sep 2 10:49:20 PDT 2010
Author: tcare
Date: Thu Sep 2 12:49:20 2010
New Revision: 112839
URL: http://llvm.org/viewvc/llvm-project?rev=112839&view=rev
Log:
Improved error reporting in IdempotentOperationChecker
- SourceRange highlighting is only given for the relevant side of the operator (assignments give both)
- Added PostVisitBinaryOperator hook to retrieve the ExplodedNode for an operator
- Added a BugReporterVisitor to display the last store to every VarDecl in a Stmt
- Changed bug reporting to use the new BugReporterVisitor
Modified:
cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h
cfe/trunk/lib/Checker/BugReporterVisitors.cpp
cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
Modified: cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h?rev=112839&r1=112838&r2=112839&view=diff
==============================================================================
--- cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h (original)
+++ cfe/trunk/include/clang/Checker/BugReporter/BugReporter.h Thu Sep 2 12:49:20 2010
@@ -471,6 +471,9 @@
void registerNilReceiverVisitor(BugReporterContext &BRC);
+void registerVarDeclsLastStore(BugReporterContext &BRC, const void *stmt,
+ const ExplodedNode *N);
+
} // end namespace clang::bugreporter
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/lib/Checker/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/BugReporterVisitors.cpp?rev=112839&r1=112838&r2=112839&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/Checker/BugReporterVisitors.cpp Thu Sep 2 12:49:20 2010
@@ -419,3 +419,40 @@
void clang::bugreporter::registerNilReceiverVisitor(BugReporterContext &BRC) {
BRC.addVisitor(new NilReceiverVisitor());
}
+
+// Registers every VarDecl inside a Stmt with a last store vistor.
+void clang::bugreporter::registerVarDeclsLastStore(BugReporterContext &BRC,
+ const void *stmt,
+ const ExplodedNode *N) {
+ const Stmt *S = static_cast<const Stmt *>(stmt);
+
+ std::deque<const Stmt *> WorkList;
+
+ WorkList.push_back(S);
+
+ while (!WorkList.empty()) {
+ const Stmt *Head = WorkList.front();
+ WorkList.pop_front();
+
+ GRStateManager &StateMgr = BRC.getStateManager();
+ const GRState *state = N->getState();
+
+ if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Head)) {
+ if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+ const VarRegion *R =
+ StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
+
+ // What did we load?
+ SVal V = state->getSVal(S);
+
+ if (isa<loc::ConcreteInt>(V) || isa<nonloc::ConcreteInt>(V)) {
+ ::registerFindLastStore(BRC, R, V);
+ }
+ }
+ }
+
+ for (Stmt::const_child_iterator I = Head->child_begin();
+ I != Head->child_end(); ++I)
+ WorkList.push_back(*I);
+ }
+}
Modified: cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp?rev=112839&r1=112838&r2=112839&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp (original)
+++ cfe/trunk/lib/Checker/IdempotentOperationChecker.cpp Thu Sep 2 12:49:20 2010
@@ -45,6 +45,7 @@
#include "GRExprEngineExperimentalChecks.h"
#include "clang/Analysis/CFGStmtMap.h"
#include "clang/Analysis/Analyses/PseudoConstantAnalysis.h"
+#include "clang/Checker/BugReporter/BugReporter.h"
#include "clang/Checker/BugReporter/BugType.h"
#include "clang/Checker/PathSensitive/CheckerHelpers.h"
#include "clang/Checker/PathSensitive/CheckerVisitor.h"
@@ -64,6 +65,7 @@
public:
static void *getTag();
void PreVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B);
+ void PostVisitBinaryOperator(CheckerContext &C, const BinaryOperator *B);
void VisitEndAnalysis(ExplodedGraph &G, BugReporter &B, GRExprEngine &Eng);
private:
@@ -87,10 +89,15 @@
AnalysisContext *AC);
static bool containsNonLocalVarDecl(const Stmt *S);
- // Hash table
- typedef llvm::DenseMap<const BinaryOperator *,
- std::pair<Assumption, AnalysisContext*> >
- AssumptionMap;
+ // Hash table and related data structures
+ typedef struct {
+ Assumption Assumption;
+ AnalysisContext *AnalysisContext;
+ ExplodedNodeSet ExplodedNodes; // Set of ExplodedNodes that refer to a
+ // BinaryOperator
+ } BinaryOperatorData;
+ typedef llvm::DenseMap<const BinaryOperator *, BinaryOperatorData>
+ AssumptionMap;
AssumptionMap hash;
};
}
@@ -109,11 +116,12 @@
const BinaryOperator *B) {
// Find or create an entry in the hash for this BinaryOperator instance.
// If we haven't done a lookup before, it will get default initialized to
- // 'Possible'.
- std::pair<Assumption, AnalysisContext *> &Data = hash[B];
- Assumption &A = Data.first;
+ // 'Possible'. At this stage we do not store the ExplodedNode, as it has not
+ // been created yet.
+ BinaryOperatorData &Data = hash[B];
+ Assumption &A = Data.Assumption;
AnalysisContext *AC = C.getCurrentAnalysisContext();
- Data.second = AC;
+ Data.AnalysisContext = AC;
// If we already have visited this node on a path that does not contain an
// idempotent operation, return immediately.
@@ -317,16 +325,29 @@
A = Impossible;
}
+// At the post visit stage, the predecessor ExplodedNode will be the
+// BinaryOperator that was just created. We use this hook to collect the
+// ExplodedNode.
+void IdempotentOperationChecker::PostVisitBinaryOperator(
+ CheckerContext &C,
+ const BinaryOperator *B) {
+ // Add the ExplodedNode we just visited
+ BinaryOperatorData &Data = hash[B];
+ Data.ExplodedNodes.Add(C.getPredecessor());
+}
+
void IdempotentOperationChecker::VisitEndAnalysis(ExplodedGraph &G,
BugReporter &BR,
GRExprEngine &Eng) {
+ BugType *BT = new BugType("Idempotent operation", "Dead code");
// Iterate over the hash to see if we have any paths with definite
// idempotent operations.
for (AssumptionMap::const_iterator i = hash.begin(); i != hash.end(); ++i) {
// Unpack the hash contents
- const std::pair<Assumption, AnalysisContext *> &Data = i->second;
- const Assumption &A = Data.first;
- AnalysisContext *AC = Data.second;
+ const BinaryOperatorData &Data = i->second;
+ const Assumption &A = Data.Assumption;
+ AnalysisContext *AC = Data.AnalysisContext;
+ const ExplodedNodeSet &ES = Data.ExplodedNodes;
const BinaryOperator *B = i->first;
@@ -348,11 +369,14 @@
delete CBM;
}
- // Select the error message.
+ // Select the error message and SourceRanges to report.
llvm::SmallString<128> buf;
llvm::raw_svector_ostream os(buf);
+ bool LHSRelevant = false, RHSRelevant = false;
switch (A) {
case Equal:
+ LHSRelevant = true;
+ RHSRelevant = true;
if (B->getOpcode() == BO_Assign)
os << "Assigned value is always the same as the existing value";
else
@@ -360,15 +384,19 @@
<< "' always have the same value";
break;
case LHSis1:
+ LHSRelevant = true;
os << "The left operand to '" << B->getOpcodeStr() << "' is always 1";
break;
case RHSis1:
+ RHSRelevant = true;
os << "The right operand to '" << B->getOpcodeStr() << "' is always 1";
break;
case LHSis0:
+ LHSRelevant = true;
os << "The left operand to '" << B->getOpcodeStr() << "' is always 0";
break;
case RHSis0:
+ RHSRelevant = true;
os << "The right operand to '" << B->getOpcodeStr() << "' is always 0";
break;
case Possible:
@@ -377,11 +405,24 @@
llvm_unreachable(0);
}
- // Create the SourceRange Arrays
- SourceRange S[2] = { i->first->getLHS()->getSourceRange(),
- i->first->getRHS()->getSourceRange() };
- BR.EmitBasicReport("Idempotent operation", "Dead code",
- os.str(), i->first->getOperatorLoc(), S, 2);
+ // Add a report for each ExplodedNode
+ for (ExplodedNodeSet::iterator I = ES.begin(), E = ES.end(); I != E; ++I) {
+ EnhancedBugReport *report = new EnhancedBugReport(*BT, os.str(), *I);
+
+ // Add source ranges and visitor hooks
+ if (LHSRelevant) {
+ const Expr *LHS = i->first->getLHS();
+ report->addRange(LHS->getSourceRange());
+ report->addVisitorCreator(bugreporter::registerVarDeclsLastStore, LHS);
+ }
+ if (RHSRelevant) {
+ const Expr *RHS = i->first->getRHS();
+ report->addRange(i->first->getRHS()->getSourceRange());
+ report->addVisitorCreator(bugreporter::registerVarDeclsLastStore, RHS);
+ }
+
+ BR.EmitReport(report);
+ }
}
}
More information about the cfe-commits
mailing list