]> granicus.if.org Git - clang/commitdiff
[analyzer] Invalidate destination of std::copy() and std::copy_backward().
authorDevin Coughlin <dcoughlin@apple.com>
Sun, 7 Feb 2016 16:55:44 +0000 (16:55 +0000)
committerDevin Coughlin <dcoughlin@apple.com>
Sun, 7 Feb 2016 16:55:44 +0000 (16:55 +0000)
Now that the libcpp implementations of these methods has a branch that doesn't call
memmove(), the analyzer needs to invalidate the destination for these methods explicitly.

rdar://problem/23575656

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

include/clang/Analysis/AnalysisContext.h
lib/Analysis/AnalysisDeclContext.cpp
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/Inputs/system-header-simulator-cxx.h
test/Analysis/bstring.cpp [new file with mode: 0644]
test/Analysis/diagnostics/explicit-suppression.cpp

index 931190e43a66861c7f20db9039444ad450d0e9ee..4324a0352e3a1b6f79aae687e2e89bddfd2884d3 100644 (file)
@@ -201,6 +201,10 @@ public:
     }
     return static_cast<T*>(data);
   }
+
+  /// Returns true if the root namespace of the given declaration is the 'std'
+  /// C++ namespace.
+  static bool isInStdNamespace(const Decl *D);
 private:
   ManagedAnalysis *&getAnalysisImpl(const void* tag);
 
index 52c7f2613654b30d2fefce8dd33e43d7554a9901..94f753ed912809815300969448f7caf3859694f0 100644 (file)
@@ -317,6 +317,21 @@ AnalysisDeclContext::getBlockInvocationContext(const LocationContext *parent,
                                                                BD, ContextData);
 }
 
+bool AnalysisDeclContext::isInStdNamespace(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext()->getEnclosingNamespaceContext();
+  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
+  if (!ND)
+    return false;
+
+  while (const DeclContext *Parent = ND->getParent()) {
+    if (!isa<NamespaceDecl>(Parent))
+      break;
+    ND = cast<NamespaceDecl>(Parent);
+  }
+
+  return ND->isStdNamespace();
+}
+
 LocationContextManager & AnalysisDeclContext::getLocationContextManager() {
   assert(Manager &&
          "Cannot create LocationContexts without an AnalysisDeclContextManager!");
index 17537445d66c5c87dc68b6e0fde6a2f1ec7942f7..5130dd610aee0b3c49918011de8411251668475b 100644 (file)
@@ -118,6 +118,10 @@ public:
 
   void evalStrsep(CheckerContext &C, const CallExpr *CE) const;
 
+  void evalStdCopy(CheckerContext &C, const CallExpr *CE) const;
+  void evalStdCopyBackward(CheckerContext &C, const CallExpr *CE) const;
+  void evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const;
+
   // Utility methods
   std::pair<ProgramStateRef , ProgramStateRef >
   static assumeZero(CheckerContext &C,
@@ -1950,7 +1954,57 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const {
   C.addTransition(State);
 }
 
+// These should probably be moved into a C++ standard library checker.
+void CStringChecker::evalStdCopy(CheckerContext &C, const CallExpr *CE) const {
+  evalStdCopyCommon(C, CE);
+}
+
+void CStringChecker::evalStdCopyBackward(CheckerContext &C,
+                                         const CallExpr *CE) const {
+  evalStdCopyCommon(C, CE);
+}
+
+void CStringChecker::evalStdCopyCommon(CheckerContext &C,
+                                       const CallExpr *CE) const {
+  if (CE->getNumArgs() < 3)
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  const LocationContext *LCtx = C.getLocationContext();
+
+  // template <class _InputIterator, class _OutputIterator>
+  // _OutputIterator
+  // copy(_InputIterator __first, _InputIterator __last,
+  //        _OutputIterator __result)
+
+  // Invalidate the destination buffer
+  const Expr *Dst = CE->getArg(2);
+  SVal DstVal = State->getSVal(Dst, LCtx);
+  State = InvalidateBuffer(C, State, Dst, DstVal, /*IsSource=*/false,
+                           /*Size=*/nullptr);
+
+  SValBuilder &SVB = C.getSValBuilder();
+
+  SVal ResultVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+  State = State->BindExpr(CE, LCtx, ResultVal);
+
+  C.addTransition(State);
+}
+
+static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) {
+  IdentifierInfo *II = FD->getIdentifier();
+  if (!II)
+    return false;
+
+  if (!AnalysisDeclContext::isInStdNamespace(FD))
+    return false;
 
+  if (II->getName().equals(Name))
+    return true;
+
+  return false;
+}
 //===----------------------------------------------------------------------===//
 // The driver method, and other Checker callbacks.
 //===----------------------------------------------------------------------===//
@@ -1999,6 +2053,10 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
     evalFunction =  &CStringChecker::evalBcopy;
   else if (C.isCLibraryFunction(FDecl, "bcmp"))
     evalFunction =  &CStringChecker::evalMemcmp;
+  else if (isCPPStdLibraryFunction(FDecl, "copy"))
+    evalFunction =  &CStringChecker::evalStdCopy;
+  else if (isCPPStdLibraryFunction(FDecl, "copy_backward"))
+    evalFunction =  &CStringChecker::evalStdCopyBackward;
 
   // If the callee isn't a string function, let another checker handle it.
   if (!evalFunction)
index ae5cd546f8d89cca2c8e62582f4f9a1729adc5d0..859a2220e09e861543e24d03f9883b56db0c3721 100644 (file)
@@ -1540,20 +1540,6 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond,
   return event;
 }
 
-
-// FIXME: Copied from ExprEngineCallAndReturn.cpp.
-static bool isInStdNamespace(const Decl *D) {
-  const DeclContext *DC = D->getDeclContext()->getEnclosingNamespaceContext();
-  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
-  if (!ND)
-    return false;
-
-  while (const NamespaceDecl *Parent = dyn_cast<NamespaceDecl>(ND->getParent()))
-    ND = Parent;
-
-  return ND->isStdNamespace();
-}
-
 std::unique_ptr<PathDiagnosticPiece>
 LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
                                                     const ExplodedNode *N,
@@ -1564,7 +1550,7 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
   AnalyzerOptions &Options = Eng.getAnalysisManager().options;
   const Decl *D = N->getLocationContext()->getDecl();
 
-  if (isInStdNamespace(D)) {
+  if (AnalysisDeclContext::isInStdNamespace(D)) {
     // Skip reports within the 'std' namespace. Although these can sometimes be
     // the user's fault, we currently don't report them very well, and
     // Note that this will not help for any other data structure libraries, like
index 74cc8d2ccbc5b3c253fcc37462fd482fa7cca169..2c24dc1353a1ff2b66f40652b20507ff6ee61dff 100644 (file)
@@ -382,21 +382,6 @@ void ExprEngine::examineStackFrames(const Decl *D, const LocationContext *LCtx,
 
 }
 
-static bool IsInStdNamespace(const FunctionDecl *FD) {
-  const DeclContext *DC = FD->getEnclosingNamespaceContext();
-  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
-  if (!ND)
-    return false;
-
-  while (const DeclContext *Parent = ND->getParent()) {
-    if (!isa<NamespaceDecl>(Parent))
-      break;
-    ND = cast<NamespaceDecl>(Parent);
-  }
-
-  return ND->isStdNamespace();
-}
-
 // The GDM component containing the dynamic dispatch bifurcation info. When
 // the exact type of the receiver is not known, we want to explore both paths -
 // one on which we do inline it and the other one on which we don't. This is
@@ -761,7 +746,7 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
       // Conditionally control the inlining of C++ standard library functions.
       if (!Opts.mayInlineCXXStandardLibrary())
         if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation()))
-          if (IsInStdNamespace(FD))
+          if (AnalysisDeclContext::isInStdNamespace(FD))
             return false;
 
       // Conditionally control the inlining of methods on objects that look
index f9049c3ae9e72f89c9077d8e33cc0c9506eabebf..f6b970088ea6c9860b7447de89ace247a9cc9511 100644 (file)
@@ -7,6 +7,9 @@
 
 typedef unsigned char uint8_t;
 
+typedef __typeof__(sizeof(int)) size_t;
+void *memmove(void *s1, const void *s2, size_t n);
+
 namespace std {
   template <class T1, class T2>
   struct pair {
@@ -104,11 +107,120 @@ namespace std {
     const _E* end()   const {return __begin_ + __size_;}
   };
 
+  template <bool, class _Tp = void> struct enable_if {};
+  template <class _Tp> struct enable_if<true, _Tp> {typedef _Tp type;};
+
+  template <class _Tp, _Tp __v>
+  struct integral_constant
+  {
+      static const _Tp      value = __v;
+      typedef _Tp               value_type;
+      typedef integral_constant type;
+
+     operator value_type() const {return value;}
+
+     value_type operator ()() const {return value;}
+  };
+
+  template <class _Tp, _Tp __v>
+  const _Tp integral_constant<_Tp, __v>::value;
+
+    template <class _Tp, class _Arg>
+    struct is_trivially_assignable
+      : integral_constant<bool, __is_trivially_assignable(_Tp, _Arg)>
+    {
+    };
+
+  typedef integral_constant<bool,true>  true_type;
+  typedef integral_constant<bool,false> false_type;
+
+  template <class _Tp> struct is_const            : public false_type {};
+  template <class _Tp> struct is_const<_Tp const> : public true_type {};
+
+  template <class _Tp> struct  is_reference        : public false_type {};
+  template <class _Tp> struct  is_reference<_Tp&>  : public true_type {};
+
+  template <class _Tp, class _Up> struct  is_same           : public false_type {};
+  template <class _Tp>            struct  is_same<_Tp, _Tp> : public true_type {};
+
+  template <class _Tp, bool = is_const<_Tp>::value || is_reference<_Tp>::value    >
+  struct __add_const             {typedef _Tp type;};
+
+  template <class _Tp>
+  struct __add_const<_Tp, false> {typedef const _Tp type;};
+
+  template <class _Tp> struct add_const {typedef typename __add_const<_Tp>::type type;};
+
+  template <class _Tp> struct  remove_const            {typedef _Tp type;};
+  template <class _Tp> struct  remove_const<const _Tp> {typedef _Tp type;};
+
+  template <class _Tp> struct  add_lvalue_reference    {typedef _Tp& type;};
+
+  template <class _Tp> struct is_trivially_copy_assignable
+      : public is_trivially_assignable<typename add_lvalue_reference<_Tp>::type,
+            typename add_lvalue_reference<typename add_const<_Tp>::type>::type> {};
+
+    template<class InputIter, class OutputIter>
+    OutputIter __copy(InputIter II, InputIter IE, OutputIter OI) {
+      while (II != IE)
+        *OI++ = *II++;
+
+      return OI;
+    }
+
+  template <class _Tp, class _Up>
+  inline
+  typename enable_if
+  <
+      is_same<typename remove_const<_Tp>::type, _Up>::value &&
+      is_trivially_copy_assignable<_Up>::value,
+      _Up*
+  >::type __copy(_Tp* __first, _Tp* __last, _Up* __result) {
+      size_t __n = __last - __first;
+
+      if (__n > 0)
+        memmove(__result, __first, __n * sizeof(_Up));
+
+      return __result + __n;
+    }
+
   template<class InputIter, class OutputIter>
   OutputIter copy(InputIter II, InputIter IE, OutputIter OI) {
-    while (II != IE)
-      *OI++ = *II++;
-    return OI;
+    return __copy(II, IE, OI);
+  }
+
+  template <class _BidirectionalIterator, class _OutputIterator>
+  inline
+  _OutputIterator
+  __copy_backward(_BidirectionalIterator __first, _BidirectionalIterator __last,
+                  _OutputIterator __result)
+  {
+      while (__first != __last)
+          *--__result = *--__last;
+      return __result;
+  }
+
+  template <class _Tp, class _Up>
+  inline
+  typename enable_if
+  <
+      is_same<typename remove_const<_Tp>::type, _Up>::value &&
+      is_trivially_copy_assignable<_Up>::value,
+      _Up*
+  >::type __copy_backward(_Tp* __first, _Tp* __last, _Up* __result) {
+      size_t __n = __last - __first;
+
+    if (__n > 0)
+    {
+        __result -= __n;
+        memmove(__result, __first, __n * sizeof(_Up));
+    }
+    return __result;
+  }
+
+  template<class InputIter, class OutputIter>
+  OutputIter copy_backward(InputIter II, InputIter IE, OutputIter OI) {
+    return __copy_backward(II, IE, OI);
   }
 
   struct input_iterator_tag { };
diff --git a/test/Analysis/bstring.cpp b/test/Analysis/bstring.cpp
new file mode 100644 (file)
index 0000000..0b4e7e9
--- /dev/null
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+#include "Inputs/system-header-simulator-for-malloc.h"
+
+void clang_analyzer_eval(int);
+
+int *testStdCopyInvalidatesBuffer(std::vector<int> v) {
+  int n = v.size();
+  int *buf = (int *)malloc(n * sizeof(int));
+
+  buf[0] = 66;
+
+  // Call to copy should invalidate buf.
+  std::copy(v.begin(), v.end(), buf);
+
+  int i = buf[0];
+
+  clang_analyzer_eval(i == 66); // expected-warning {{UNKNOWN}}
+
+  return buf;
+}
+
+int *testStdCopyBackwardInvalidatesBuffer(std::vector<int> v) {
+  int n = v.size();
+  int *buf = (int *)malloc(n * sizeof(int));
+  
+  buf[0] = 66;
+
+  // Call to copy_backward should invalidate buf.
+  std::copy_backward(v.begin(), v.end(), buf + n);
+
+  int i = buf[0];
+
+  clang_analyzer_eval(i == 66); // expected-warning {{UNKNOWN}}
+
+  return buf;
+}
index 78067573e3220161828a180f76a227e8d9fcafa4..67a47d02b6e9541d01591158c19577590009ede6 100644 (file)
@@ -9,9 +9,15 @@
 
 void clang_analyzer_eval(bool);
 
-void testCopyNull(int *I, int *E) {
-  std::copy(I, E, (int *)0);
+class C {
+  // The virtual function is to make C not trivially copy assignable so that we call the
+  // variant of std::copy() that does not defer to memmove().
+  virtual int f();
+};
+
+void testCopyNull(C *I, C *E) {
+  std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:110 {{Dereference of null pointer}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:166 {{Called C++ object pointer is null}}
 #endif
 }