[analyzer] Do not invalidate arguments when the parameter's

type is a pointer to const. (radar://10595327)

The regions corresponding to the pointer and reference arguments to
a function get invalidated by the calls since a function call can
possibly modify the pointed to data. With this change, we are not going
to invalidate the data if the argument is a pointer to const. This
change makes the analyzer more optimistic in reporting errors.
(Support for C, C++ and Obj C)

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

8 files changed, 152 insertions, 11 deletions

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
index add3479804..ed589f9c44 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
@@ -246,6 +246,9 @@ public:
   SVal getCXXCallee() const;
   SVal getInstanceMessageReceiver(const LocationContext *LC) const;
 
+  /// Get the declaration of the function or method.
+  const Decl *getDecl() const;
+
   unsigned getNumArgs() const {
     if (!CallE)
       return Msg.getNumArgs();
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index dc49c85a97..46ebd84b65 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -63,6 +63,46 @@ void ExprEngine::processCallExit(CallExitNodeBuilder &B) {
   B.generateNode(state);
 }
 
+static bool isPointerToConst(const ParmVarDecl *ParamDecl) {
+  QualType PointeeTy = ParamDecl->getOriginalType()->getPointeeType();
+  if (PointeeTy != QualType() && PointeeTy.isConstQualified() &&
+      !PointeeTy->isAnyPointerType() && !PointeeTy->isReferenceType()) {
+    return true;
+  }
+  return false;
+}
+
+// Try to retrieve the function declaration and find the function parameter
+// types which are pointers/references to a non-pointer const.
+// We do not invalidate the corresponding argument regions.
+static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
+                       const CallOrObjCMessage &Call) {
+  const Decl *CallDecl = Call.getDecl();
+  if (!CallDecl)
+    return;
+
+  if (const FunctionDecl *FDecl = dyn_cast<FunctionDecl>(CallDecl)) {
+    for (unsigned Idx = 0, E = Call.getNumArgs(); Idx != E; ++Idx) {
+      if (FDecl && Idx < FDecl->getNumParams()) {
+        if (isPointerToConst(FDecl->getParamDecl(Idx)))
+          PreserveArgs.insert(Idx);
+      }
+    }
+    return;
+  }
+
+  if (const ObjCMethodDecl *MDecl = dyn_cast<ObjCMethodDecl>(CallDecl)) {
+    assert(MDecl->param_size() <= Call.getNumArgs());
+    unsigned Idx = 0;
+    for (clang::ObjCMethodDecl::param_const_iterator
+         I = MDecl->param_begin(), E = MDecl->param_end(); I != E; ++I, ++Idx) {
+      if (isPointerToConst(*I))
+        PreserveArgs.insert(Idx);
+    }
+    return;
+  }
+}
+
 const ProgramState *
 ExprEngine::invalidateArguments(const ProgramState *State,
                                 const CallOrObjCMessage &Call,
@@ -84,13 +124,21 @@ ExprEngine::invalidateArguments(const ProgramState *State,
 
   } else if (Call.isFunctionCall()) {
     // Block calls invalidate all captured-by-reference values.
-    if (const MemRegion *Callee = Call.getFunctionCallee().getAsRegion()) {
+    SVal CalleeVal = Call.getFunctionCallee();
+    if (const MemRegion *Callee = CalleeVal.getAsRegion()) {
       if (isa<BlockDataRegion>(Callee))
         RegionsToInvalidate.push_back(Callee);
     }
   }
 
+  // Indexes of arguments whose values will be preserved by the call.
+  llvm::SmallSet<unsigned, 1> PreserveArgs;
+  findPtrToConstParams(PreserveArgs, Call);
+
   for (unsigned idx = 0, e = Call.getNumArgs(); idx != e; ++idx) {
+    if (PreserveArgs.count(idx))
+      continue;
+
     SVal V = Call.getArgSVal(idx);
 
     // If we are passing a location wrapped as an integer, unwrap it and
@@ -104,7 +152,7 @@ ExprEngine::invalidateArguments(const ProgramState *State,
       // Invalidate the value of the variable passed by reference.
 
       // Are we dealing with an ElementRegion?  If the element type is
-      // a basic integer type (e.g., char, int) and the underying region
+      // a basic integer type (e.g., char, int) and the underlying region
       // is a variable region then strip off the ElementRegion.
       // FIXME: We really need to think about this for the general case
       //   as sometimes we are reasoning about arrays and other times
@@ -115,7 +163,7 @@ ExprEngine::invalidateArguments(const ProgramState *State,
         // we'll leave it in for now until we have a systematic way of
         // handling all of these cases.  Eventually we need to come up
         // with an interface to StoreManager so that this logic can be
-        // approriately delegated to the respective StoreManagers while
+        // appropriately delegated to the respective StoreManagers while
         // still allowing us to do checker-specific logic (e.g.,
         // invalidating reference counts), probably via callbacks.
         if (ER->getElementType()->isIntegralOrEnumerationType()) {
diff --git a/lib/StaticAnalyzer/Core/ObjCMessage.cpp b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
index 0974fe877a..1edc3769e7 100644
--- a/lib/StaticAnalyzer/Core/ObjCMessage.cpp
+++ b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
+#include "clang/AST/DeclCXX.h"
 
 using namespace clang;
 using namespace ento;
@@ -162,3 +163,21 @@ CallOrObjCMessage::getInstanceMessageReceiver(const LocationContext *LC) const {
   assert(isObjCMessage());
   return Msg.getInstanceReceiverSVal(State, LC);
 }
+
+const Decl *CallOrObjCMessage::getDecl() const {
+  if (isCXXCall()) {
+    const CXXMemberCallExpr *CE =
+        cast<CXXMemberCallExpr>(CallE.dyn_cast<const CallExpr *>());
+    assert(CE);
+    return CE->getMethodDecl();
+  } else if (isObjCMessage()) {
+    return Msg.getMethodDecl();
+  } else if (isFunctionCall()) {
+    // In case of a C style call, use the path sensitive information to find
+    // the function declaration.
+    SVal CalleeVal = getFunctionCallee();
+    return CalleeVal.getAsFunctionDecl();
+  }
+  return 0;
+}
+
diff --git a/test/Analysis/method-call-intra-p.cpp b/test/Analysis/method-call-intra-p.cpp
new file mode 100644
index 0000000000..701479faa8
--- /dev/null
+++ b/test/Analysis/method-call-intra-p.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region -verify %s
+
+// Intra-procedural C++ tests.
+
+// Test relaxing function call arguments invalidation to be aware of const
+// arguments. radar://10595327
+struct InvalidateArgs {
+  void ttt(const int &nptr);
+  virtual void vttt(const int *nptr);
+};
+struct ChildOfInvalidateArgs: public InvalidateArgs {
+  virtual void vttt(const int *nptr);
+};
+void declarationFun(int x) {
+  InvalidateArgs t;
+  x = 3;
+  int y = x + 1;
+  int *p = 0;
+  t.ttt(y);
+  if (x == y)
+      y = *p; // no-warning
+}
+void virtualFun(int x) {
+  ChildOfInvalidateArgs t;
+  InvalidateArgs *pt = &t;
+  x = 3;
+  int y = x + 1;
+  int *p = 0;
+  pt->vttt(&y);
+  if (x == y)
+      y = *p; // no-warning
+}
diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m
index 520b944cd5..a1319fb5ea 100644
--- a/test/Analysis/misc-ps.m
+++ b/test/Analysis/misc-ps.m
@@ -274,6 +274,17 @@ void rdar_6777003(int x) {
   *p = 1; // expected-warning{{Dereference of null pointer}}  
 }
 
+// Check that the pointer-to-conts arguments do not get invalidated by Obj C 
+// interfaces. radar://10595327
+int rdar_10595327(char *str) {
+  char fl = str[0]; 
+  int *p = 0;
+  NSString *s = [NSString stringWithUTF8String:str];
+  if (str[0] != fl)
+      return *p; // no-warning
+  return 0;
+}
+
 // For pointer arithmetic, --/++ should be treated as preserving non-nullness,
 // regardless of how well the underlying StoreManager reasons about pointer
 // arithmetic.
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index 5da9699176..84350802a3 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -289,4 +289,25 @@ void pr4759() {
   pr4759_aux(p); // expected-warning{{Function call argument is an uninitialized value}}
 }
 
-
+// Relax function call arguments invalidation to be aware of const
+// arguments. Test with function pointers. radar://10595327
+void ttt(const int *nptr);
+void ttt2(const int *nptr);
+typedef void (*NoConstType)(int*);
+int foo10595327(int b) {
+  void (*fp)(int *);
+  // We use path sensitivity to get the function declaration. Even when the
+  // function pointer is cast to non pointer-to-const parameter type, we can
+  // find the right function declaration.
+  if (b > 5)
+    fp = (NoConstType)ttt2;
+  else
+    fp = (NoConstType)ttt;
+  int x = 3;
+  int y = x + 1;
+  int *p = 0;
+  fp(&y);
+  if (x == y)
+      return *p; // no-warning
+  return 0;
+}
diff --git a/test/Analysis/string.c b/test/Analysis/string.c
index 89283befad..fcbe298a8f 100644
--- a/test/Analysis/string.c
+++ b/test/Analysis/string.c
@@ -133,6 +133,18 @@ void strlen_indirect(char *x) {
     (void)*(char*)0; // expected-warning{{null}}
 }
 
+void strlen_indirect2(char *x) {
+  size_t a = strlen(x);
+  char *p = x;
+  char **p2 = &p;
+  extern void use_string_ptr2(char**);
+  use_string_ptr2(p2);
+
+  size_t c = strlen(x);
+  if (a == 0 && c != 0)
+    (void)*(char*)0; // expected-warning{{null}}
+}
+
 void strlen_liveness(const char *x) {
   if (strlen(x) < 5)
     return;
diff --git a/test/Analysis/taint-tester.c b/test/Analysis/taint-tester.c
index 02afe6d4f2..4aa170ce21 100644
--- a/test/Analysis/taint-tester.c
+++ b/test/Analysis/taint-tester.c
@@ -164,15 +164,10 @@ void atoiTest() {
   int d = atoi(s); // expected-warning + {{tainted}}
   int td = d; // expected-warning + {{tainted}}
 
-  // TODO: We shouldn't have to redefine the taint source here.
-  char sl[80];
-  scanf("%s", sl);
-  long l = atol(sl); // expected-warning + {{tainted}}
+  long l = atol(s); // expected-warning + {{tainted}}
   int tl = l; // expected-warning + {{tainted}}
 
-  char sll[80];
-  scanf("%s", sll);
-  long long ll = atoll(sll); // expected-warning + {{tainted}}
+  long long ll = atoll(s); // expected-warning + {{tainted}}
   int tll = ll; // expected-warning + {{tainted}}
 
 }