Implement second part of PR 2600: NSError** parameter may be null, and should be...

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@56318 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Analysis/PathSensitive/BasicValueFactory.h b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
index c3729ed53e2a1756b72cd644de807a91e84747e7..29957a7996f9405e95ce64a9d492ac9c94706ea8 100644 (file)
--- a/include/clang/Analysis/PathSensitive/BasicValueFactory.h
+++ b/include/clang/Analysis/PathSensitive/BasicValueFactory.h
@@ -77,6 +77,8 @@ public:
   
   const std::pair<RVal, RVal>&
   getPersistentRValPair(const RVal& V1, const RVal& V2);  
+  
+  const RVal* getPersistentRVal(RVal X);
 };
 
 } // end clang namespace

diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h
index d08e442e35c79840d6c0b42a360df76c385123dc..1a288c24afcb01c172a842b2ce661f62934dca45 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRExprEngine.h
+++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h
@@ -293,6 +293,13 @@ public:
   null_deref_iterator null_derefs_begin() { return ExplicitNullDeref.begin(); }
   null_deref_iterator null_derefs_end() { return ExplicitNullDeref.end(); }
   
+  null_deref_iterator implicit_null_derefs_begin() {
+    return ImplicitNullDeref.begin();
+  }
+  null_deref_iterator implicit_null_derefs_end() {
+    return ImplicitNullDeref.end();
+  }
+  
   typedef BadDerefTy::iterator undef_deref_iterator;
   undef_deref_iterator undef_derefs_begin() { return UndefDeref.begin(); }
   undef_deref_iterator undef_derefs_end() { return UndefDeref.end(); }

diff --git a/include/clang/Analysis/PathSensitive/GRState.h b/include/clang/Analysis/PathSensitive/GRState.h
index 993977984ff60cecaf88b13381dea2369be1d365..fd81a86946faa6c4e27919c1ff665122154aad7e 100644 (file)
--- a/include/clang/Analysis/PathSensitive/GRState.h
+++ b/include/clang/Analysis/PathSensitive/GRState.h
@@ -43,12 +43,25 @@ namespace clang {
 
 class GRStateManager;
 class GRTransferFuncs;
+
+//===----------------------------------------------------------------------===//
+// GRStateTrait - Traits used by the Generic Data Map of a GRState.
+//===----------------------------------------------------------------------===//
   
+template <typename T> struct GRStatePartialTrait;
+
+template <typename T> struct GRStateTrait {
+  typedef typename T::data_type data_type;
+  static inline void* GDMIndex() { return &T::TagInt; }   
+  static inline void* MakeVoidPtr(data_type D) { return (void*) D; }
+  static inline data_type MakeData(void* const* P) {
+    return P ? (data_type) *P : (data_type) 0;
+  }
+};
+
 //===----------------------------------------------------------------------===//
 // GRState- An ImmutableMap type Stmt*/Decl*/Symbols to RVals.
 //===----------------------------------------------------------------------===//
-
-template<typename T> struct GRStateTrait;
   
 /// GRState - This class encapsulates the actual data values for
 ///  for a "state" in our symbolic value tracking.  It is intended to be
@@ -169,7 +182,13 @@ public:
   void print(std::ostream& Out, StoreManager& StoreMgr,
              ConstraintManager& ConstraintMgr,
              Printer **Beg = 0, Printer **End = 0,
-             const char* nl = "\n", const char *sep = "") const;  
+             const char* nl = "\n", const char *sep = "") const; 
+  
+  // Tags used for the Generic Data Map.
+  struct NullDerefTag {
+    static int TagInt;
+    typedef const RVal* data_type;
+  };
 };
   
 template<> struct GRTrait<GRState*> {
@@ -566,8 +585,7 @@ public:
   
   void printDOT(std::ostream& Out) const;
 };
-  
-  
+
 } // end clang namespace
 
 #endif

diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index f836e5b9951c83e8c29909caa4293e7c52e3e041..e35525df9c534da2ded173602cb235311c6382ec 100644 (file)
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -17,7 +17,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
 #include "llvm/ADT/StringExtras.h"
-#include <sstream>
+
 using namespace clang;
 
 bool QualType::isConstant(ASTContext& Ctx) const {

diff --git a/lib/Analysis/BasicValueFactory.cpp b/lib/Analysis/BasicValueFactory.cpp
index 8d737a9472dea38a794bff48de22c477bf8b76bd..a9bb2cec4bda7d83dd7b5aa61d4af7fbb21a7a38 100644 (file)
--- a/lib/Analysis/BasicValueFactory.cpp
+++ b/lib/Analysis/BasicValueFactory.cpp
@@ -247,3 +247,8 @@ BasicValueFactory::getPersistentRValPair(const RVal& V1, const RVal& V2) {
   return P->getValue();
 }
 
+const RVal* BasicValueFactory::getPersistentRVal(RVal X) {
+  return &getPersistentRValWithData(X, 0).first;
+}  
+
+

diff --git a/lib/Analysis/CheckNSError.cpp b/lib/Analysis/CheckNSError.cpp
index 4c7f2cf25d1a7d3f65645938bb0b2a116bd0ae67..d39fa0a86a2ac2dda7803c0215245876f9cc3e56 100644 (file)
--- a/lib/Analysis/CheckNSError.cpp
+++ b/lib/Analysis/CheckNSError.cpp
@@ -37,9 +37,16 @@ class VISIBILITY_HIDDEN NSErrorCheck : public BugTypeCacheLocation {
 
   bool CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII);
   
+  void CheckParamDeref(VarDecl* V, GRStateRef state, GRExprEngine& Eng,
+                       GRBugReporter& BR); 
+  
+  const char* desc;
 public:
+  NSErrorCheck() : desc(0) {}
+  
   void EmitWarnings(BugReporter& BR) { EmitGRWarnings(cast<GRBugReporter>(BR));}
   const char* getName() const { return "NSError** null dereference"; }
+  const char* getDescription() const { return desc; }
 };  
   
 } // end anonymous namespace
@@ -77,6 +84,14 @@ void NSErrorCheck::EmitGRWarnings(GRBugReporter& BR) {
               CodeDecl.getLocation());
     
   }
+  
+  // Scan the NSError** parameters for an implicit null dereference.
+  for (llvm::SmallVectorImpl<VarDecl*>::iterator I=Params.begin(),
+        E=Params.end(); I!=E; ++I)    
+    for (GRExprEngine::GraphTy::roots_iterator RI=G.roots_begin(),
+         RE=G.roots_end(); RI!=RE; ++RI)
+      CheckParamDeref(*I, GRStateRef((*RI)->getState(), Eng.getStateManager()),
+                      Eng, BR);
 }
 
 void NSErrorCheck::CheckSignature(ObjCMethodDecl& M, QualType& ResultTy,
@@ -104,3 +119,36 @@ bool NSErrorCheck::CheckArgument(QualType ArgTy, IdentifierInfo* NSErrorII) {
   if (!IT) return false;
   return IT->getDecl()->getIdentifier() == NSErrorII;
 }
+
+void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState,
+                                   GRExprEngine& Eng, GRBugReporter& BR) {
+  
+  RVal ParamRVal = rootState.GetRVal(lval::DeclVal(Param));
+
+  // FIXME: For now assume that ParamRVal is symbolic.  We need to generalize
+  // this later.
+  lval::SymbolVal* SV = dyn_cast<lval::SymbolVal>(&ParamRVal);
+  if (!SV) return;
+  
+  // Iterate over the implicit-null dereferences.
+  for (GRExprEngine::null_deref_iterator I=Eng.implicit_null_derefs_begin(),
+       E=Eng.implicit_null_derefs_end(); I!=E; ++I) {
+    
+    GRStateRef state = GRStateRef((*I)->getState(), Eng.getStateManager());
+    const RVal* X = state.get<GRState::NullDerefTag>();    
+    const lval::SymbolVal* SVX = dyn_cast_or_null<lval::SymbolVal>(X);
+    if (!SVX || SVX->getSymbol() != SV->getSymbol()) continue;
+
+    // Emit an error.
+    BugReport R(*this, *I);
+
+    std::string msg;
+    llvm::raw_string_ostream os(msg);
+    os << "Potential null dereference.  According to coding standards in "
+          "'Creating and Returning NSError Objects' the parameter '"
+        << Param->getName() << "' may be null.";    
+    desc = os.str().c_str();
+
+    BR.EmitWarning(R);    
+  }
+}

diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index aa5ab6af7fa09a39e2f051ae1ea0de4f1e7c1a1f..8d6fea7f7c764ce4686cdc5e42afbcfa77b36a2e 100644 (file)
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -1010,10 +1010,15 @@ const GRState* GRExprEngine::EvalLocation(Expr* Ex, NodeTy* Pred,
   // "Assume" that the pointer is NULL.
   
   bool isFeasibleNull = false;
-  const GRState* StNull = Assume(St, LV, false, isFeasibleNull);
+  GRStateRef StNull = GRStateRef(Assume(St, LV, false, isFeasibleNull),
+                                 getStateManager());
   
   if (isFeasibleNull) {
     
+    // Use the Generic Data Map to mark in the state what lval was null.
+    const RVal* PersistentLV = getBasicVals().getPersistentRVal(LV);
+    StNull = StNull.set<GRState::NullDerefTag>(PersistentLV);
+    
     // We don't use "MakeNode" here because the node will be a sink
     // and we have no intention of processing it later.
     

diff --git a/lib/Analysis/GRState.cpp b/lib/Analysis/GRState.cpp
index 4bef72c6c5c00731c1834256d673832180dbbd54..67bff39fc8c83569332f6c9525181031abbe2466 100644 (file)
--- a/lib/Analysis/GRState.cpp
+++ b/lib/Analysis/GRState.cpp
@@ -261,3 +261,9 @@ bool GRStateManager::isEqual(const GRState* state, Expr* Ex,
 bool GRStateManager::isEqual(const GRState* state, Expr* Ex, uint64_t x) {
   return isEqual(state, Ex, BasicVals.getValue(x, Ex->getType()));
 }
+
+//===----------------------------------------------------------------------===//
+// Persistent values for indexing into the Generic Data Map.
+
+int GRState::NullDerefTag::TagInt = 0;
+