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@-2 {{TRUE}} -#else - // expected-warning@-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@-2 {{TRUE}} -#else - // expected-warning@-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@-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 -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits