Added path-sensitive checking for null pointer values passed to function arguments...

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 0f80f238829d0edeb2dadf67e1b5e4fb6ee3695d..817d66de1a91c86dfbece9586f500722da0b1d13 100644 (file)
--- a/include/clang/AST/Attr.h
+++ b/include/clang/AST/Attr.h
@@ -197,7 +197,7 @@ public:
     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 4374c1e40a64773619d08dd1a03a4527ad455abd..803a70397d391718d47325e8e9778457e0194742 100644 (file)
--- a/include/clang/Analysis/PathSensitive/BugReporter.h
+++ b/include/clang/Analysis/PathSensitive/BugReporter.h
@@ -300,12 +300,14 @@ public:
 };
   
 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 29fb3bc18624a8807fba439b6018bf0a145e3af3..eca591d4af0ecd054324cf51efab4942fa62f237 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRAuditor.h
+++ b/include/clang/Analysis/PathSensitive/GRAuditor.h
@@ -26,10 +26,11 @@ namespace clang {
 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 9c3098f93e413cc26be9b8b3aa22ad85f90254ff..16f79856ba8fea7303380bcfea91c88cb81bd213 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRCoreEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRCoreEngine.h
@@ -166,17 +166,19 @@ public:
 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 @@ public:
       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 @@ class GRCoreEngine : public GRCoreEngineImpl {
 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 @@ protected:
   }
   
   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 782ddcfffeacda47f8bc24fb38dbb01691727323..fcc9a0c3be47876ce2e36ae6d8fb7bb23d2d639c 100644 (file)
--- 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 3a56abe81b3ced43b6cbef7fe221e87ace23593a..554e6f5f8697d61192e66af24ede71757ff18c08 100644 (file)
--- a/include/clang/Analysis/PathSensitive/ValueState.h
+++ b/include/clang/Analysis/PathSensitive/ValueState.h
@@ -58,6 +58,8 @@ public:
   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 @@ public:
   // 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 @@ public:
   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 4475ed2bc5b8667518b39da0808a05931bcc490e..37cfc280b8f1ab6a5cedff5013c721714f5dafbb 100644 (file)
--- a/lib/Analysis/BasicObjCFoundationChecks.cpp
+++ b/lib/Analysis/BasicObjCFoundationChecks.cpp
@@ -126,7 +126,7 @@ public:
       delete *I;    
   }
   
-  virtual bool Audit(ExplodedNode<ValueState>* N);
+  virtual bool Audit(ExplodedNode<ValueState>* N, ValueStateManager&);
   
   virtual void EmitWarnings(BugReporter& BR);
   
@@ -153,7 +153,8 @@ clang::CreateBasicObjCFoundationChecks(ASTContext& Ctx,
 
 
 
-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 @@ public:
   
   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 @@ static const char* GetCFNumberTypeStr(uint64_t i) {
 }
 #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 8dee018931eca6297bb6dbabc3b145f2ba807093..f0c0d8dd9e87bf764ecd4d8d68a69a727f813b5e 100644 (file)
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -85,7 +85,7 @@ public:
       }
   }
   
-  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 @@ public:
     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 36e7233f25fc2e0e312e75082db613fc8047feeb..38327cbad5b2351dcaad61e5c8a623358a9dd9d9 100644 (file)
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -335,6 +335,63 @@ void UndefBranch::EmitWarnings(BugReporter& BR) {
   }
 }
 
+//===----------------------------------------------------------------------===//
+// __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 @@ void GRSimpleVals::RegisterChecks(GRExprEngine& Eng) {
   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 2c3e6dabc66f7a790457f2431908de98d700c493..1cf695438bb8b2617d26c15ec08dfb878ce0b6b1 100644 (file)
--- a/lib/Analysis/ValueState.cpp
+++ b/lib/Analysis/ValueState.cpp
@@ -26,6 +26,15 @@ bool ValueState::isNotEqual(SymbolID sym, const llvm::APSInt& V) const {
   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 @@ void ValueState::print(std::ostream& Out, CheckerStatePrinter* P,
     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 86691bd9a39e3b6eb1dbfafd04f9d9693a78106a..964b3e455068103aa7a274b0c27d43dc32f8669c 100644 (file)
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -56,3 +56,10 @@ int f5() {
   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}}
+} 
+
+