aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/Analysis/LocalCheckers.h4
-rw-r--r--include/clang/Analysis/PathSensitive/BugReporter.h3
-rw-r--r--include/clang/Analysis/PathSensitive/GRCoreEngine.h11
-rw-r--r--include/clang/Analysis/PathSensitive/GRExprEngine.h8
-rw-r--r--lib/Analysis/BugReporter.cpp4
-rw-r--r--lib/Analysis/DeadStores.cpp73
-rw-r--r--lib/Analysis/GRCoreEngine.cpp13
-rw-r--r--lib/Analysis/GRExprEngine.cpp15
-rw-r--r--test/Analysis/dead-stores.c23
9 files changed, 104 insertions, 50 deletions
diff --git a/include/clang/Analysis/LocalCheckers.h b/include/clang/Analysis/LocalCheckers.h
index 530bf90e7e..2369196b0b 100644
--- a/include/clang/Analysis/LocalCheckers.h
+++ b/include/clang/Analysis/LocalCheckers.h
@@ -25,8 +25,10 @@ class PathDiagnosticClient;
class GRTransferFuncs;
class BugType;
class LangOptions;
+class ParentMap;
-void CheckDeadStores(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags);
+void CheckDeadStores(CFG& cfg, ASTContext &Ctx, ParentMap& Parents,
+ Diagnostic &Diags);
void CheckUninitializedValues(CFG& cfg, ASTContext& Ctx, Diagnostic& Diags,
bool FullUninitTaint=false);
diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h
index c9cabf2073..cf9de720a2 100644
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -34,6 +34,7 @@ class GRExprEngine;
class ValueState;
class Stmt;
class BugReport;
+class ParentMap;
class BugType {
public:
@@ -156,6 +157,8 @@ public:
CFG& getCFG() { return getGraph().getCFG(); }
+ ParentMap& getParentMap();
+
void EmitWarning(BugReport& R);
void GeneratePathDiagnostic(PathDiagnostic& PD, BugReport& R);
diff --git a/include/clang/Analysis/PathSensitive/GRCoreEngine.h b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
index 5cb4fd7d52..19028ee875 100644
--- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
@@ -47,19 +47,10 @@ protected:
friend class GRIndirectGotoNodeBuilderImpl;
friend class GRSwitchNodeBuilderImpl;
friend class GREndPathNodeBuilderImpl;
-
- typedef llvm::DenseMap<Stmt*,Stmt*> ParentMapTy;
/// G - The simulation graph. Each node is a (location,state) pair.
llvm::OwningPtr<ExplodedGraphImpl> G;
-
- /// ParentMap - A lazily populated map from a Stmt* to its parent Stmt*.
- void* ParentMap;
-
- /// CurrentBlkExpr - The current Block-level expression being processed.
- /// This is used when lazily populating ParentMap.
- Stmt* CurrentBlkExpr;
-
+
/// WList - A set of queued nodes that need to be processed by the
/// worklist algorithm. It is up to the implementation of WList to decide
/// the order that nodes are processed.
diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index 4ea03adc4f..c9d1676c2d 100644
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -28,6 +28,7 @@ namespace clang {
class BugType;
class PathDiagnosticClient;
class Diagnostic;
+ class ParentMap;
class GRExprEngine {
@@ -51,6 +52,9 @@ protected:
/// G - the simulation graph.
GraphTy& G;
+ /// Parents - a lazily created map from Stmt* to parents.
+ ParentMap* Parents;
+
/// Liveness - live-variables information the ValueDecl* and block-level
/// Expr* in the CFG. Used to prune out dead state.
LiveVariables Liveness;
@@ -213,6 +217,10 @@ public:
GraphTy& getGraph() { return G; }
const GraphTy& getGraph() const { return G; }
+
+ /// getParentMap - Return a map from Stmt* to parents for the function/method
+ /// body being analyzed. This map is lazily constructed as needed.
+ ParentMap& getParentMap();
typedef BugTypeSet::iterator bug_type_iterator;
typedef BugTypeSet::const_iterator const_bug_type_iterator;
diff --git a/lib/Analysis/BugReporter.cpp b/lib/Analysis/BugReporter.cpp
index 240ecd19fe..26dba4e7e8 100644
--- a/lib/Analysis/BugReporter.cpp
+++ b/lib/Analysis/BugReporter.cpp
@@ -39,6 +39,10 @@ ValueStateManager& BugReporter::getStateManager() {
return Eng.getStateManager();
}
+ParentMap& BugReporter::getParentMap() {
+ return Eng.getParentMap();
+}
+
static inline Stmt* GetStmt(const ProgramPoint& P) {
if (const PostStmt* PS = dyn_cast<PostStmt>(&P)) {
return PS->getStmt();
diff --git a/lib/Analysis/DeadStores.cpp b/lib/Analysis/DeadStores.cpp
index 0f08b233b5..7d912b046e 100644
--- a/lib/Analysis/DeadStores.cpp
+++ b/lib/Analysis/DeadStores.cpp
@@ -19,28 +19,49 @@
#include "clang/Analysis/PathSensitive/GRExprEngine.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMap.h"
#include "llvm/Support/Compiler.h"
using namespace clang;
namespace {
-
+
class VISIBILITY_HIDDEN DeadStoreObs : public LiveVariables::ObserverTy {
ASTContext &Ctx;
Diagnostic &Diags;
DiagnosticClient &Client;
+ ParentMap& Parents;
+
public:
- DeadStoreObs(ASTContext &ctx, Diagnostic &diags, DiagnosticClient &client)
- : Ctx(ctx), Diags(diags), Client(client) {}
+ DeadStoreObs(ASTContext &ctx, Diagnostic &diags, DiagnosticClient &client,
+ ParentMap& parents)
+ : Ctx(ctx), Diags(diags), Client(client), Parents(parents) {}
virtual ~DeadStoreObs() {}
- unsigned GetDiag(VarDecl* VD) {
- std::string msg = "value stored to '" + std::string(VD->getName()) +
- "' is never used";
+ unsigned GetDiag(VarDecl* VD, bool inEnclosing = false) {
+ std::string name(VD->getName());
+
+ std::string msg = inEnclosing
+ ? "Although the value stored to '" + name +
+ "' is used in the enclosing expression, the value is never actually read"
+ " from '" + name + "'"
+ : "Value stored to '" + name + "' is never read";
- return Diags.getCustomDiagID(Diagnostic::Warning, msg.c_str());
-
+ return Diags.getCustomDiagID(Diagnostic::Warning, msg.c_str());
+ }
+
+ void CheckVarDecl(VarDecl* VD, Expr* Ex, Expr* Val,
+ bool hasEnclosing,
+ const LiveVariables::AnalysisDataTy& AD,
+ const LiveVariables::ValTy& Live) {
+
+ if (VD->hasLocalStorage() && !Live(VD, AD)) {
+ SourceRange R = Val->getSourceRange();
+ Diags.Report(&Client,
+ Ctx.getFullLoc(Ex->getSourceRange().getBegin()),
+ GetDiag(VD, hasEnclosing), 0, 0, &R, 1);
+ }
}
void CheckDeclRef(DeclRefExpr* DR, Expr* Val,
@@ -48,12 +69,7 @@ public:
const LiveVariables::ValTy& Live) {
if (VarDecl* VD = dyn_cast<VarDecl>(DR->getDecl()))
- if (VD->hasLocalStorage() && !Live(VD, AD)) {
- SourceRange R = Val->getSourceRange();
- Diags.Report(&Client,
- Ctx.getFullLoc(DR->getSourceRange().getBegin()),
- GetDiag(VD), 0, 0, &R, 1);
- }
+ CheckVarDecl(VD, DR, Val, false, AD, Live);
}
virtual void ObserveStmt(Stmt* S,
@@ -68,7 +84,22 @@ public:
if (!B->isAssignmentOp()) return; // Skip non-assignments.
if (DeclRefExpr* DR = dyn_cast<DeclRefExpr>(B->getLHS()))
- CheckDeclRef(DR, B->getRHS(), AD, Live);
+ if (VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+
+ // Special case: check for assigning null to a pointer. This
+ // is a common form of defensive programming.
+ // FIXME: Make this optional?
+
+ Expr* Val = B->getRHS();
+ llvm::APSInt Result(Ctx.getTypeSize(Val->getType()));
+
+ if (VD->getType()->isPointerType() &&
+ Val->IgnoreParenCasts()->isIntegerConstantExpr(Result, Ctx, 0))
+ if (Result == 0)
+ return;
+
+ CheckVarDecl(VD, DR, Val, Parents.isSubExpr(B), AD, Live);
+ }
}
else if (UnaryOperator* U = dyn_cast<UnaryOperator>(S)) {
if (!U->isIncrementOp())
@@ -116,10 +147,11 @@ public:
// Driver function to invoke the Dead-Stores checker on a CFG.
//===----------------------------------------------------------------------===//
-void clang::CheckDeadStores(CFG& cfg, ASTContext &Ctx, Diagnostic &Diags) {
+void clang::CheckDeadStores(CFG& cfg, ASTContext &Ctx,
+ ParentMap& Parents, Diagnostic &Diags) {
LiveVariables L(cfg);
L.runOnCFG(cfg);
- DeadStoreObs A(Ctx, Diags, Diags.getClient());
+ DeadStoreObs A(Ctx, Diags, Diags.getClient(), Parents);
L.runOnAllBlocks(cfg, &A);
}
@@ -197,9 +229,10 @@ public:
// Run the dead store checker and collect the diagnostics.
DiagCollector C(*this);
- DeadStoreObs A(BR.getContext(), BR.getDiagnostic(), C);
- GRExprEngine& Eng = BR.getEngine();
- Eng.getLiveness().runOnAllBlocks(Eng.getCFG(), &A);
+ DeadStoreObs A(BR.getContext(), BR.getDiagnostic(), C, BR.getParentMap());
+
+ GRExprEngine& Eng = BR.getEngine();
+ Eng.getLiveness().runOnAllBlocks(BR.getCFG(), &A);
// Emit the bug reports.
diff --git a/lib/Analysis/GRCoreEngine.cpp b/lib/Analysis/GRCoreEngine.cpp
index 7c2ca0c13f..a4bb7fa631 100644
--- a/lib/Analysis/GRCoreEngine.cpp
+++ b/lib/Analysis/GRCoreEngine.cpp
@@ -253,19 +253,6 @@ void GRCoreEngineImpl::HandlePostStmt(const PostStmt& L, CFGBlock* B,
}
}
-typedef llvm::DenseMap<Stmt*,Stmt*> ParentMapTy;
-/// PopulateParentMap - Recurse the AST starting at 'Parent' and add the
-/// mappings between child and parent to ParentMap.
-static void PopulateParentMap(Stmt* Parent, ParentMapTy& M) {
- for (Stmt::child_iterator I=Parent->child_begin(),
- E=Parent->child_end(); I!=E; ++I) {
-
- assert (M.find(*I) == M.end());
- M[*I] = Parent;
- PopulateParentMap(*I, M);
- }
-}
-
/// GenerateNode - Utility method to generate nodes, hook up successors,
/// and add nodes to the worklist.
void GRCoreEngineImpl::GenerateNode(const ProgramPoint& Loc, void* State,
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index e8c1ec57a9..9abfe6f79f 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -13,6 +13,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/AST/ParentMap.h"
#include "clang/Analysis/PathSensitive/GRExprEngine.h"
#include "clang/Analysis/PathSensitive/BugReporter.h"
#include "clang/Basic/SourceManager.h"
@@ -41,6 +42,7 @@ static inline Selector GetNullarySelector(const char* name, ASTContext& Ctx) {
GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx)
: CoreEngine(cfg, CD, Ctx, *this),
G(CoreEngine.getGraph()),
+ Parents(0),
Liveness(G.getCFG()),
Builder(NULL),
StateMgr(G.getContext(), G.getAllocator()),
@@ -56,7 +58,7 @@ GRExprEngine::GRExprEngine(CFG& cfg, Decl& CD, ASTContext& Ctx)
Liveness.runOnAllBlocks(G.getCFG(), NULL, true);
}
-GRExprEngine::~GRExprEngine() {
+GRExprEngine::~GRExprEngine() {
for (BugTypeSet::iterator I = BugTypes.begin(), E = BugTypes.end(); I!=E; ++I)
delete *I;
@@ -69,6 +71,17 @@ GRExprEngine::~GRExprEngine() {
delete *I;
delete [] NSExceptionInstanceRaiseSelectors;
+
+ delete Parents;
+}
+
+ParentMap& GRExprEngine::getParentMap() {
+ if (!Parents) {
+ Stmt* Body = getGraph().getCodeDecl().getCodeBody();
+ Parents = new ParentMap(Body);
+ }
+
+ return *Parents;
}
//===----------------------------------------------------------------------===//
diff --git a/test/Analysis/dead-stores.c b/test/Analysis/dead-stores.c
index 21427c6910..972410ac98 100644
--- a/test/Analysis/dead-stores.c
+++ b/test/Analysis/dead-stores.c
@@ -3,12 +3,12 @@
void f1() {
int k, y;
int abc=1;
- long idx=abc+3*5; // expected-warning {{never used}}
+ long idx=abc+3*5; // expected-warning {{never read}}
}
void f2(void *b) {
char *c = (char*)b; // no-warning
- char *d = b+1; // expected-warning {{never used}}
+ char *d = b+1; // expected-warning {{never read}}
printf("%s", c);
}
@@ -27,19 +27,32 @@ void f4(int k) {
if (k)
f1();
- k = 2; // expected-warning {{never used}}
+ k = 2; // expected-warning {{never read}}
}
void f5() {
int x = 4; // no-warning
- int *p = &x; // expected-warning{{never used}}
+ int *p = &x; // expected-warning{{never read}}
}
int f6() {
int x = 4;
- ++x; // expected-warning{{never used}}
+ ++x; // expected-warning{{never read}}
return 1;
}
+
+int f7(int *p) {
+ // This is allowed for defensive programming.
+ p = 0; // no-warning
+ return 1;
+}
+
+int f8(int *p) {
+ if (p = baz()) // expected-warning{{Although the value}}
+ return 1;
+ return 0;
+}
+
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-08-03 18:22:48 +0000