Added path-sensitive checking for null pointer values passed to function arguments marked nonnull.
This implements <rdar://problem/6069935>
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@53891 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/AST/Attr.h b/include/clang/AST/Attr.h
index 0f80f23..817d66d 100644
--- a/include/clang/AST/Attr.h
+++ b/include/clang/AST/Attr.h
@@ -197,7 +197,7 @@
delete [] ArgNums;
}
- bool isNonNull(unsigned arg) {
+ bool isNonNull(unsigned arg) const {
return ArgNums ? std::binary_search(ArgNums, ArgNums+Size, arg) : true;
}
};
diff --git a/include/clang/Analysis/PathSensitive/BugReporter.h b/include/clang/Analysis/PathSensitive/BugReporter.h
index 4374c1e..803a703 100644
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -300,12 +300,14 @@
};
class SimpleBugType : public BugTypeCacheLocation {
- const char* name;
+ const char* name;
+ const char* desc;
public:
- SimpleBugType(const char* n) : name(n) {}
+ SimpleBugType(const char* n) : name(n), desc(n) {}
+ SimpleBugType(const char* n, const char* d) : name(n), desc(d) {}
virtual const char* getName() const { return name; }
- virtual const char* getDescription() const { return name; }
+ virtual const char* getDescription() const { return desc; }
};
} // end clang namespace
diff --git a/include/clang/Analysis/PathSensitive/GRAuditor.h b/include/clang/Analysis/PathSensitive/GRAuditor.h
index 29fb3bc..eca591d 100644
--- a/include/clang/Analysis/PathSensitive/GRAuditor.h
+++ b/include/clang/Analysis/PathSensitive/GRAuditor.h
@@ -26,10 +26,11 @@
template <typename STATE>
class GRAuditor {
public:
- typedef ExplodedNode<STATE> NodeTy;
+ typedef ExplodedNode<STATE> NodeTy;
+ typedef typename STATE::ManagerTy ManagerTy;
virtual ~GRAuditor() {}
- virtual bool Audit(NodeTy* N) = 0;
+ virtual bool Audit(NodeTy* N, ManagerTy& M) = 0;
};
diff --git a/include/clang/Analysis/PathSensitive/GRCoreEngine.h b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
index 9c3098f..16f7985 100644
--- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
@@ -166,17 +166,19 @@
template<typename STATE>
class GRStmtNodeBuilder {
public:
- typedef STATE StateTy;
- typedef ExplodedNode<StateTy> NodeTy;
+ typedef STATE StateTy;
+ typedef typename StateTy::ManagerTy StateManagerTy;
+ typedef ExplodedNode<StateTy> NodeTy;
private:
GRStmtNodeBuilderImpl& NB;
+ StateManagerTy& Mgr;
const StateTy* CleanedState;
GRAuditor<StateTy>* Auditor;
public:
- GRStmtNodeBuilder(GRStmtNodeBuilderImpl& nb) : NB(nb),
- Auditor(0), PurgingDeadSymbols(false),
+ GRStmtNodeBuilder(GRStmtNodeBuilderImpl& nb, StateManagerTy& mgr) :
+ NB(nb), Mgr(mgr), Auditor(0), PurgingDeadSymbols(false),
BuildSinks(false), HasGeneratedNode(false) {
CleanedState = getLastNode()->getState();
@@ -244,7 +246,7 @@
if (BuildSinks)
N->markAsSink();
else {
- if (Auditor && Auditor->Audit(N))
+ if (Auditor && Auditor->Audit(N, Mgr))
N->markAsSink();
Dst.Add(N);
@@ -552,6 +554,7 @@
public:
typedef SUBENGINE SubEngineTy;
typedef typename SubEngineTy::StateTy StateTy;
+ typedef typename StateTy::ManagerTy StateManagerTy;
typedef ExplodedGraph<StateTy> GraphTy;
typedef typename GraphTy::NodeTy NodeTy;
@@ -568,7 +571,7 @@
}
virtual void ProcessStmt(Stmt* S, GRStmtNodeBuilderImpl& BuilderImpl) {
- GRStmtNodeBuilder<StateTy> Builder(BuilderImpl);
+ GRStmtNodeBuilder<StateTy> Builder(BuilderImpl,SubEngine.getStateManager());
SubEngine.ProcessStmt(S, Builder);
}
diff --git a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
index 782ddcf..fcc9a0c 100644
--- a/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
+++ b/include/clang/Analysis/PathSensitive/GRSimpleAPICheck.h
@@ -17,10 +17,10 @@
#define LLVM_CLANG_ANALYSIS_GRAPICHECKS
#include "clang/Analysis/PathSensitive/GRAuditor.h"
+#include "clang/Analysis/PathSensitive/ValueState.h"
namespace clang {
-class ValueState;
class Diagnostic;
class BugReporter;
class ASTContext;
diff --git a/include/clang/Analysis/PathSensitive/ValueState.h b/include/clang/Analysis/PathSensitive/ValueState.h
index 3a56abe..554e6f5 100644
--- a/include/clang/Analysis/PathSensitive/ValueState.h
+++ b/include/clang/Analysis/PathSensitive/ValueState.h
@@ -58,6 +58,8 @@
typedef llvm::ImmutableMap<SymbolID,IntSetTy> ConstNotEqTy;
typedef llvm::ImmutableMap<SymbolID,const llvm::APSInt*> ConstEqTy;
+ typedef ValueStateManager ManagerTy;
+
private:
void operator=(const ValueState& R) const;
@@ -120,6 +122,8 @@
// Queries.
bool isNotEqual(SymbolID sym, const llvm::APSInt& V) const;
+ bool isEqual(SymbolID sym, const llvm::APSInt& V) const;
+
const llvm::APSInt* getSymVal(SymbolID sym) const;
RVal LookupExpr(Expr* E) const {
@@ -361,6 +365,10 @@
const ValueState* AddNE(const ValueState* St, SymbolID sym,
const llvm::APSInt& V);
+
+ bool isEqual(const ValueState* state, Expr* Ex, const llvm::APSInt& V);
+ bool isEqual(const ValueState* state, Expr* Ex, uint64_t);
+
// Assumption logic.
const ValueState* Assume(const ValueState* St, RVal Cond, bool Assumption,
bool& isFeasible) {
diff --git a/lib/Analysis/BasicObjCFoundationChecks.cpp b/lib/Analysis/BasicObjCFoundationChecks.cpp
index 4475ed2..37cfc28 100644
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -126,7 +126,7 @@
delete *I;
}
- virtual bool Audit(ExplodedNode<ValueState>* N);
+ virtual bool Audit(ExplodedNode<ValueState>* N, ValueStateManager&);
virtual void EmitWarnings(BugReporter& BR);
@@ -153,7 +153,8 @@
-bool BasicObjCFoundationChecks::Audit(ExplodedNode<ValueState>* N) {
+bool BasicObjCFoundationChecks::Audit(ExplodedNode<ValueState>* N,
+ ValueStateManager&) {
ObjCMessageExpr* ME =
cast<ObjCMessageExpr>(cast<PostStmt>(N->getLocation()).getStmt());
@@ -348,7 +349,7 @@
virtual ~AuditCFNumberCreate() {}
- virtual bool Audit(ExplodedNode<ValueState>* N);
+ virtual bool Audit(ExplodedNode<ValueState>* N, ValueStateManager&);
virtual void EmitWarnings(BugReporter& BR) {
Desc.EmitWarnings(BR);
@@ -454,7 +455,7 @@
}
#endif
-bool AuditCFNumberCreate::Audit(ExplodedNode<ValueState>* N) {
+bool AuditCFNumberCreate::Audit(ExplodedNode<ValueState>* N,ValueStateManager&){
CallExpr* CE = cast<CallExpr>(cast<PostStmt>(N->getLocation()).getStmt());
Expr* Callee = CE->getCallee();
RVal CallV = GetRVal(N->getState(), Callee);
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index 8dee018..f0c0d8d 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -85,7 +85,7 @@
}
}
- virtual bool Audit(NodeTy* N) {
+ virtual bool Audit(NodeTy* N, ValueStateManager& VMgr) {
Stmt* S = cast<PostStmt>(N->getLocation()).getStmt();
void* key = reinterpret_cast<void*>((uintptr_t) S->getStmtClass());
MapTy::iterator MI = M.find(key);
@@ -96,7 +96,7 @@
bool isSink = false;
for (Checks::iterator I=MI->second.begin(), E=MI->second.end(); I!=E; ++I)
- isSink |= (*I)->Audit(N);
+ isSink |= (*I)->Audit(N, VMgr);
return isSink;
}
diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp
index 36e7233..38327cb 100644
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -335,6 +335,63 @@
}
}
+//===----------------------------------------------------------------------===//
+// __attribute__(nonnull) checking
+
+class VISIBILITY_HIDDEN CheckAttrNonNull : public GRSimpleAPICheck {
+ SimpleBugType BT;
+ std::list<RangedBugReport> Reports;
+
+public:
+ CheckAttrNonNull() :
+ BT("'nonnull' argument passed null",
+ "Null pointer passed as an argument to a 'nonnull' parameter") {}
+
+
+ virtual bool Audit(ExplodedNode<ValueState>* N, ValueStateManager& VMgr) {
+ CallExpr* CE = cast<CallExpr>(cast<PostStmt>(N->getLocation()).getStmt());
+ const ValueState* state = N->getState();
+
+ RVal X = VMgr.GetRVal(state, CE->getCallee());
+
+ if (!isa<lval::FuncVal>(X))
+ return false;
+
+ FunctionDecl* FD = dyn_cast<FunctionDecl>(cast<lval::FuncVal>(X).getDecl());
+ const NonNullAttr* Att = FD->getAttr<NonNullAttr>();
+
+ if (!Att)
+ return false;
+
+ // Iterate through the arguments of CE and check them for null.
+
+ unsigned idx = 0;
+ bool hasError = false;
+
+ for (CallExpr::arg_iterator I=CE->arg_begin(), E=CE->arg_end(); I!=E;
+ ++I, ++idx) {
+
+ if (!VMgr.isEqual(state, *I, 0) || !Att->isNonNull(idx))
+ continue;
+
+ RangedBugReport R(BT, N);
+ R.addRange((*I)->getSourceRange());
+ Reports.push_back(R);
+ hasError = true;
+ }
+
+ return hasError;
+ }
+
+ virtual void EmitWarnings(BugReporter& BR) {
+ for (std::list<RangedBugReport>::iterator I=Reports.begin(),
+ E=Reports.end(); I!=E; ++I)
+ BR.EmitWarning(*I);
+ }
+};
+
+//===----------------------------------------------------------------------===//
+// Check registration.
void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) {
@@ -360,6 +417,7 @@
Check = CreateAuditCFNumberCreate(Ctx, VMgr);
Eng.AddCheck(Check, Stmt::CallExprClass);
+ Eng.AddCheck(new CheckAttrNonNull(), Stmt::CallExprClass);
}
//===----------------------------------------------------------------------===//
diff --git a/lib/Analysis/ValueState.cpp b/lib/Analysis/ValueState.cpp
index 2c3e6da..1cf6954 100644
--- a/lib/Analysis/ValueState.cpp
+++ b/lib/Analysis/ValueState.cpp
@@ -26,6 +26,15 @@
return T ? T->contains(&V) : false;
}
+bool ValueState::isEqual(SymbolID sym, const llvm::APSInt& V) const {
+
+ // Retrieve the EQ-set associated with the given symbol.
+ const ConstEqTy::data_type* T = ConstEq.lookup(sym);
+
+ // See if V is present in the EQ-set.
+ return T ? **T == V : false;
+}
+
const llvm::APSInt* ValueState::getSymVal(SymbolID sym) const {
ConstEqTy::data_type* T = ConstEq.lookup(sym);
return T ? *T : NULL;
@@ -296,6 +305,35 @@
P->PrintCheckerState(Out, CheckerState, nl, sep);
}
+
+//===----------------------------------------------------------------------===//
+// Queries.
+//===----------------------------------------------------------------------===//
+
+bool ValueStateManager::isEqual(const ValueState* state, Expr* Ex,
+ const llvm::APSInt& Y) {
+ RVal V = GetRVal(state, Ex);
+
+ if (lval::ConcreteInt* X = dyn_cast<lval::ConcreteInt>(&V))
+ return X->getValue() == Y;
+
+ if (nonlval::ConcreteInt* X = dyn_cast<nonlval::ConcreteInt>(&V))
+ return X->getValue() == Y;
+
+ if (nonlval::SymbolVal* X = dyn_cast<nonlval::SymbolVal>(&V))
+ return state->isEqual(X->getSymbol(), Y);
+
+ if (lval::SymbolVal* X = dyn_cast<lval::SymbolVal>(&V))
+ return state->isEqual(X->getSymbol(), Y);
+
+ return false;
+}
+
+bool ValueStateManager::isEqual(const ValueState* state, Expr* Ex,
+ uint64_t x) {
+ return isEqual(state, Ex, BasicVals.getValue(x, Ex->getType()));
+}
+
//===----------------------------------------------------------------------===//
// "Assume" logic.
//===----------------------------------------------------------------------===//
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index 86691bd..964b3e4 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -56,3 +56,10 @@
return s[0]; // no-warning
}
+int bar(int* p) __attribute__((nonnull));
+
+int f6(int *p) {
+ return !p ? bar(p) : *p; // expected-warning {{Null pointer passed as an argument to a 'nonnull' parameter}}
+}
+
+