r274691 - [analyzer] Suppress false positives in std::shared_ptr

Devin Coughlin via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 6 14:52:56 PDT 2016


Author: dcoughlin
Date: Wed Jul  6 16:52:55 2016
New Revision: 274691

URL: http://llvm.org/viewvc/llvm-project?rev=274691&view=rev
Log:
[analyzer] Suppress false positives in std::shared_ptr

The analyzer does not model C++ temporary destructors completely and so
reports false alarms about leaks of memory allocated by the internals of
shared_ptr:

  std::shared_ptr<int> p(new int(1));
  p = nullptr; // 'Potential leak of memory pointed to by field __cntrl_'

This patch suppresses all diagnostics where the end of the path is inside
a method in std::shared_ptr.

It also reorganizes the tests for suppressions in the C++ standard library
to use a separate simulated header for library functions with bugs
that were deliberately inserted to test suppression. This will prevent
other tests from using these as models.

rdar://problem/23652766

Added:
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h
    cfe/trunk/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp
      - copied, changed from r274682, cfe/trunk/test/Analysis/inlining/stl.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
    cfe/trunk/test/Analysis/inlining/stl.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=274691&r1=274690&r2=274691&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Jul  6 16:52:55 2016
@@ -1596,12 +1596,6 @@ LikelyFalsePositiveSuppressionBRVisitor:
         }
       }
 
-      // The analyzer issues a false positive on
-      //   std::basic_string<uint8_t> v; v.push_back(1);
-      // and
-      //   std::u16string s; s += u'a';
-      // because we cannot reason about the internal invariants of the
-      // datastructure.
       for (const LocationContext *LCtx = N->getLocationContext(); LCtx;
            LCtx = LCtx->getParent()) {
         const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(LCtx->getDecl());
@@ -1609,10 +1603,24 @@ LikelyFalsePositiveSuppressionBRVisitor:
           continue;
 
         const CXXRecordDecl *CD = MD->getParent();
+        // The analyzer issues a false positive on
+        //   std::basic_string<uint8_t> v; v.push_back(1);
+        // and
+        //   std::u16string s; s += u'a';
+        // because we cannot reason about the internal invariants of the
+        // datastructure.
         if (CD->getName() == "basic_string") {
           BR.markInvalid(getTag(), nullptr);
           return nullptr;
         }
+
+        // The analyzer issues a false positive on
+        //    std::shared_ptr<int> p(new int(1)); p = nullptr;
+        // because it does not reason properly about temporary destructors.
+        if (CD->getName() == "shared_ptr") {
+          BR.markInvalid(getTag(), nullptr);
+          return nullptr;
+        }
       }
     }
   }

Added: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h?rev=274691&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h (added)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h Wed Jul  6 16:52:55 2016
@@ -0,0 +1,146 @@
+// This is a fake system header with divide-by-zero bugs introduced in
+// c++ std library functions. We use these bugs to test hard-coded
+// suppression of diagnostics within standard library functions that are known
+// to produce false positives.
+
+#pragma clang system_header
+
+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 _Tp>
+  class allocator {
+  public:
+    void deallocate(void *p) {
+      ::delete p;
+    }
+  };
+
+  template <class _Alloc>
+  class allocator_traits {
+  public:
+    static void deallocate(void *p) {
+      _Alloc().deallocate(p);
+    }
+  };
+
+  template <class _Tp, class _Alloc>
+  class __list_imp
+  {};
+
+  template <class _Tp, class _Alloc = allocator<_Tp> >
+  class list
+  : private __list_imp<_Tp, _Alloc>
+  {
+  public:
+    void pop_front() {
+      // Fake use-after-free.
+      // No warning is expected as we are suppressing warning coming
+      // out of std::list.
+      int z = 0;
+      z = 5/z;
+    }
+    bool empty() const;
+  };
+
+  // basic_string
+  template<class _CharT, class _Alloc = allocator<_CharT> >
+  class __attribute__ ((__type_visibility__("default"))) basic_string {
+    bool isLong;
+    union {
+      _CharT localStorage[4];
+      _CharT *externalStorage;
+
+      void assignExternal(_CharT *newExternal) {
+        externalStorage = newExternal;
+      }
+    } storage;
+
+    typedef allocator_traits<_Alloc> __alloc_traits;
+
+  public:
+    basic_string();
+
+    void push_back(int c) {
+      // Fake error trigger.
+      // No warning is expected as we are suppressing warning coming
+      // out of std::basic_string.
+      int z = 0;
+      z = 5/z;
+    }
+
+    _CharT *getBuffer() {
+      return isLong ? storage.externalStorage : storage.localStorage;
+    }
+
+    basic_string &operator +=(int c) {
+      // Fake deallocate stack-based storage.
+      // No warning is expected as we are suppressing warnings within
+      // std::basic_string.
+      __alloc_traits::deallocate(getBuffer());
+    }
+
+    basic_string &operator =(const basic_string &other) {
+      // Fake deallocate stack-based storage, then use the variable in the
+      // same union.
+      // No warning is expected as we are suppressing warnings within
+      // std::basic_string.
+      __alloc_traits::deallocate(getBuffer());
+      storage.assignExternal(new _CharT[4]);
+    }
+  };
+
+template<class _Engine, class _UIntType>
+class __independent_bits_engine {
+public:
+  // constructors and seeding functions
+  __independent_bits_engine(_Engine& __e, size_t __w);
+};
+
+template<class _Engine, class _UIntType>
+__independent_bits_engine<_Engine, _UIntType>
+    ::__independent_bits_engine(_Engine& __e, size_t __w)
+{
+  // Fake error trigger.
+  // No warning is expected as we are suppressing warning coming
+  // out of std::__independent_bits_engine.
+  int z = 0;
+  z = 5/z;
+}
+
+#if __has_feature(cxx_decltype)
+typedef decltype(nullptr) nullptr_t;
+
+template<class _Tp>
+class shared_ptr
+{
+public:
+  constexpr shared_ptr(nullptr_t);
+  explicit shared_ptr(_Tp* __p);
+
+  shared_ptr(shared_ptr&& __r) { }
+
+  ~shared_ptr();
+
+  shared_ptr& operator=(shared_ptr&& __r) {
+    // Fake error trigger.
+    // No warning is expected as we are suppressing warning coming
+    // out of std::shared_ptr.
+    int z = 0;
+    z = 5/z;
+  }
+};
+
+template<class _Tp>
+inline
+constexpr
+shared_ptr<_Tp>::shared_ptr(nullptr_t) {
+}
+
+#endif // __has_feature(cxx_decltype)
+}
+

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h?rev=274691&r1=274690&r2=274691&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h Wed Jul  6 16:52:55 2016
@@ -229,106 +229,6 @@ namespace std {
   struct bidirectional_iterator_tag : public forward_iterator_tag { };
   struct random_access_iterator_tag : public bidirectional_iterator_tag { };
 
-  template <class _Tp>
-  class allocator {
-  public:
-    void deallocate(void *p) {
-      ::delete p;
-    }
-  };
-
-  template <class _Alloc>
-  class allocator_traits {
-  public:
-    static void deallocate(void *p) {
-      _Alloc().deallocate(p);
-    }
-  };
-
-  template <class _Tp, class _Alloc>
-  class __list_imp
-  {};
-
-  template <class _Tp, class _Alloc = allocator<_Tp> >
-  class list
-  : private __list_imp<_Tp, _Alloc>
-  {
-  public:
-    void pop_front() {
-      // Fake use-after-free.
-      // No warning is expected as we are suppressing warning coming
-      // out of std::list.
-      int z = 0;
-      z = 5/z;
-    }
-    bool empty() const;
-  };
-
-  // basic_string
-  template<class _CharT, class _Alloc = allocator<_CharT> >
-  class __attribute__ ((__type_visibility__("default"))) basic_string {
-    bool isLong;
-    union {
-      _CharT localStorage[4];
-      _CharT *externalStorage;
-
-      void assignExternal(_CharT *newExternal) {
-        externalStorage = newExternal;
-      }
-    } storage;
-
-    typedef allocator_traits<_Alloc> __alloc_traits;
-
-  public:
-    basic_string();
-
-    void push_back(int c) {
-      // Fake error trigger.
-      // No warning is expected as we are suppressing warning coming
-      // out of std::basic_string.
-      int z = 0;
-      z = 5/z;
-    }
-
-    _CharT *getBuffer() {
-      return isLong ? storage.externalStorage : storage.localStorage;
-    }
-
-    basic_string &operator +=(int c) {
-      // Fake deallocate stack-based storage.
-      // No warning is expected as we are suppressing warnings within
-      // std::basic_string.
-      __alloc_traits::deallocate(getBuffer());
-    }
-
-    basic_string &operator =(const basic_string &other) {
-      // Fake deallocate stack-based storage, then use the variable in the
-      // same union.
-      // No warning is expected as we are suppressing warnings within
-      // std::basic_string.
-      __alloc_traits::deallocate(getBuffer());
-      storage.assignExternal(new _CharT[4]);
-    }
-  };
-
-template<class _Engine, class _UIntType>
-class __independent_bits_engine {
-public:
-  // constructors and seeding functions
-  __independent_bits_engine(_Engine& __e, size_t __w);
-};
-
-template<class _Engine, class _UIntType>
-__independent_bits_engine<_Engine, _UIntType>
-    ::__independent_bits_engine(_Engine& __e, size_t __w)
-{
-  // Fake error trigger.
-  // No warning is expected as we are suppressing warning coming
-  // out of std::basic_string.
-  int z = 0;
-  z = 5/z;
-}
-
 }
 
 void* operator new(std::size_t, const std::nothrow_t&) throw();

Copied: cfe/trunk/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp (from r274682, cfe/trunk/test/Analysis/inlining/stl.cpp)
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp?p2=cfe/trunk/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp&p1=cfe/trunk/test/Analysis/inlining/stl.cpp&r1=274682&r2=274691&rev=274691&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/stl.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp Wed Jul  6 16:52:55 2016
@@ -1,32 +1,9 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete,debug.ExprInspection -analyzer-config c++-container-inlining=true -analyzer-config c++-stdlib-inlining=false -std=c++11 -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete,debug.ExprInspection -analyzer-config c++-container-inlining=true -analyzer-config c++-stdlib-inlining=true -std=c++11 -DINLINE=1 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete,debug.ExprInspection -analyzer-config c++-container-inlining=true -analyzer-config c++-stdlib-inlining=true -std=c++11 -verify %s
 
-#include "../Inputs/system-header-simulator-cxx.h"
+// expected-no-diagnostics
 
-void clang_analyzer_eval(bool);
-
-void testVector(std::vector<int> &nums) {
-  if (nums.begin()) return;
-  if (nums.end()) return;
-  
-  clang_analyzer_eval(nums.size() == 0);
-#if INLINE
-  // expected-warning at -2 {{TRUE}}
-#else
-  // expected-warning at -4 {{UNKNOWN}}
-#endif
-}
-
-void testException(std::exception e) {
-  // Notice that the argument is NOT passed by reference, so we can devirtualize.
-  const char *x = e.what();
-  clang_analyzer_eval(x == 0);
-#if INLINE
-  // expected-warning at -2 {{TRUE}}
-#else
-  // expected-warning at -4 {{UNKNOWN}}
-#endif
-}
+#include "../Inputs/system-header-simulator-cxx-std-suppression.h"
 
 void testList_pop_front(std::list<int> list) {
   while(!list.empty())
@@ -45,10 +22,16 @@ void testBasicStringSuppression_append()
 
 void testBasicStringSuppression_assign(std::basic_string<char32_t> &v,
                                        const std::basic_string<char32_t> &v2) {
-  v = v2;
+  v = v2; // no-warning
 }
 
 class MyEngine;
-void testSupprerssion_independent_bits_engine(MyEngine& e) {
+void testSuppression_independent_bits_engine(MyEngine& e) {
   std::__independent_bits_engine<MyEngine, unsigned int> x(e, 64); // no-warning
 }
+
+void testSuppression_std_shared_pointer() {
+  std::shared_ptr<int> p(new int(1));
+
+  p = nullptr; // no-warning
+}

Modified: cfe/trunk/test/Analysis/inlining/stl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/stl.cpp?rev=274691&r1=274690&r2=274691&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/stl.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/stl.cpp Wed Jul  6 16:52:55 2016
@@ -27,28 +27,3 @@ void testException(std::exception e) {
   // expected-warning at -4 {{UNKNOWN}}
 #endif
 }
-
-void testList_pop_front(std::list<int> list) {
-  while(!list.empty())
-    list.pop_front();  // no-warning
-}
-
-void testBasicStringSuppression() {
-  std::basic_string<uint8_t> v;
-  v.push_back(1); // no-warning
-}
-
-void testBasicStringSuppression_append() {
-  std::basic_string<char32_t> v;
-  v += 'c'; // no-warning
-}
-
-void testBasicStringSuppression_assign(std::basic_string<char32_t> &v,
-                                       const std::basic_string<char32_t> &v2) {
-  v = v2;
-}
-
-class MyEngine;
-void testSupprerssion_independent_bits_engine(MyEngine& e) {
-  std::__independent_bits_engine<MyEngine, unsigned int> x(e, 64); // no-warning
-}




More information about the cfe-commits mailing list