[cfe-commits] r147002 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/ObjCMessage.cpp test/Analysis/method-call-intra-p.cpp test/Analysis/misc-ps.m test/Analysis/null-deref-ps.c test/Analysis/string.c test/Analysis/taint-tester.c

Anna Zaks ganna at apple.com
Tue Dec 20 14:35:30 PST 2011


Author: zaks
Date: Tue Dec 20 16:35:30 2011
New Revision: 147002

URL: http://llvm.org/viewvc/llvm-project?rev=147002&view=rev
Log:
[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)

Added:
    cfe/trunk/test/Analysis/method-call-intra-p.cpp
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp
    cfe/trunk/test/Analysis/misc-ps.m
    cfe/trunk/test/Analysis/null-deref-ps.c
    cfe/trunk/test/Analysis/string.c
    cfe/trunk/test/Analysis/taint-tester.c

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h Tue Dec 20 16:35:30 2011
@@ -246,6 +246,9 @@
   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();

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Tue Dec 20 16:35:30 2011
@@ -63,6 +63,46 @@
   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 @@
 
   } 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 @@
       // 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 @@
         // 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()) {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp Tue Dec 20 16:35:30 2011
@@ -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 @@
   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;
+}
+

Added: cfe/trunk/test/Analysis/method-call-intra-p.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/method-call-intra-p.cpp?rev=147002&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/method-call-intra-p.cpp (added)
+++ cfe/trunk/test/Analysis/method-call-intra-p.cpp Tue Dec 20 16:35:30 2011
@@ -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
+}

Modified: cfe/trunk/test/Analysis/misc-ps.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.m (original)
+++ cfe/trunk/test/Analysis/misc-ps.m Tue Dec 20 16:35:30 2011
@@ -274,6 +274,17 @@
   *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.

Modified: cfe/trunk/test/Analysis/null-deref-ps.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-ps.c?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-ps.c (original)
+++ cfe/trunk/test/Analysis/null-deref-ps.c Tue Dec 20 16:35:30 2011
@@ -289,4 +289,25 @@
   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;
+}

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Tue Dec 20 16:35:30 2011
@@ -133,6 +133,18 @@
     (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;

Modified: cfe/trunk/test/Analysis/taint-tester.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-tester.c?rev=147002&r1=147001&r2=147002&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/taint-tester.c (original)
+++ cfe/trunk/test/Analysis/taint-tester.c Tue Dec 20 16:35:30 2011
@@ -164,15 +164,10 @@
   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}}
 
 }





More information about the cfe-commits mailing list