]> granicus.if.org Git - clang/commitdiff
[analyzer] Suppress false positives in std::shared_ptr
authorDevin Coughlin <dcoughlin@apple.com>
Wed, 6 Jul 2016 21:52:55 +0000 (21:52 +0000)
committerDevin Coughlin <dcoughlin@apple.com>
Wed, 6 Jul 2016 21:52:55 +0000 (21:52 +0000)
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

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

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h [new file with mode: 0644]
test/Analysis/Inputs/system-header-simulator-cxx.h
test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp [new file with mode: 0644]
test/Analysis/inlining/stl.cpp

index 85a0b4789696b8515bdf87fae5a1bf0d655ea53d..0e505463bb5e424828ccb1fb524ad07eefee9a59 100644 (file)
@@ -1596,12 +1596,6 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC,
         }
       }
 
-      // 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::getEndPath(BugReporterContext &BRC,
           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;
+        }
       }
     }
   }
diff --git a/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h b/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h
new file mode 100644 (file)
index 0000000..dc53af2
--- /dev/null
@@ -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)
+}
+
index f6b970088ea6c9860b7447de89ace247a9cc9511..b32d200364b18cd428b5fcbf7d6357dba8c40768 100644 (file)
@@ -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();
diff --git a/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp b/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp
new file mode 100644 (file)
index 0000000..143cbbe
--- /dev/null
@@ -0,0 +1,37 @@
+// 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 -verify %s
+
+// expected-no-diagnostics
+
+#include "../Inputs/system-header-simulator-cxx-std-suppression.h"
+
+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; // no-warning
+}
+
+class MyEngine;
+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
+}
index 2a8520f7671857f0d58dd8760788520317a2b3f8..95ac3f8172dbdf6f046ab82c4ded7bcce5c2fae8 100644 (file)
@@ -27,28 +27,3 @@ void testException(std::exception e) {
   // expected-warning@-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
-}