From 20384159b6ed792f40d3b9cd906e63b911c6bafc Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Wed, 6 Jul 2016 21:52:55 +0000 Subject: [PATCH] [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 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 --- .../Core/BugReporterVisitors.cpp | 20 ++- ...tem-header-simulator-cxx-std-suppression.h | 146 ++++++++++++++++++ .../Inputs/system-header-simulator-cxx.h | 100 ------------ .../implicit-cxx-std-suppression.cpp | 37 +++++ test/Analysis/inlining/stl.cpp | 25 --- 5 files changed, 197 insertions(+), 131 deletions(-) create mode 100644 test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h create mode 100644 test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 85a0b47896..0e505463bb 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1596,12 +1596,6 @@ LikelyFalsePositiveSuppressionBRVisitor::getEndPath(BugReporterContext &BRC, } } - // The analyzer issues a false positive on - // std::basic_string 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(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 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 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 index 0000000000..dc53af269c --- /dev/null +++ b/test/Analysis/Inputs/system-header-simulator-cxx-std-suppression.h @@ -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 allocator { + public: + void deallocate(void *p) { + ::delete p; + } + }; + + template + class allocator_traits { + public: + static void deallocate(void *p) { + _Alloc().deallocate(p); + } + }; + + template + class __list_imp + {}; + + template > + 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 __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 __independent_bits_engine { +public: + // constructors and seeding functions + __independent_bits_engine(_Engine& __e, size_t __w); +}; + +template +__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 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 +inline +constexpr +shared_ptr<_Tp>::shared_ptr(nullptr_t) { +} + +#endif // __has_feature(cxx_decltype) +} + diff --git a/test/Analysis/Inputs/system-header-simulator-cxx.h b/test/Analysis/Inputs/system-header-simulator-cxx.h index f6b970088e..b32d200364 100644 --- a/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -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 allocator { - public: - void deallocate(void *p) { - ::delete p; - } - }; - - template - class allocator_traits { - public: - static void deallocate(void *p) { - _Alloc().deallocate(p); - } - }; - - template - class __list_imp - {}; - - template > - 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 __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 __independent_bits_engine { -public: - // constructors and seeding functions - __independent_bits_engine(_Engine& __e, size_t __w); -}; - -template -__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 index 0000000000..143cbbeeac --- /dev/null +++ b/test/Analysis/diagnostics/implicit-cxx-std-suppression.cpp @@ -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 list) { + while(!list.empty()) + list.pop_front(); // no-warning +} + +void testBasicStringSuppression() { + std::basic_string v; + v.push_back(1); // no-warning +} + +void testBasicStringSuppression_append() { + std::basic_string v; + v += 'c'; // no-warning +} + +void testBasicStringSuppression_assign(std::basic_string &v, + const std::basic_string &v2) { + v = v2; // no-warning +} + +class MyEngine; +void testSuppression_independent_bits_engine(MyEngine& e) { + std::__independent_bits_engine x(e, 64); // no-warning +} + +void testSuppression_std_shared_pointer() { + std::shared_ptr p(new int(1)); + + p = nullptr; // no-warning +} diff --git a/test/Analysis/inlining/stl.cpp b/test/Analysis/inlining/stl.cpp index 2a8520f767..95ac3f8172 100644 --- a/test/Analysis/inlining/stl.cpp +++ b/test/Analysis/inlining/stl.cpp @@ -27,28 +27,3 @@ void testException(std::exception e) { // expected-warning@-4 {{UNKNOWN}} #endif } - -void testList_pop_front(std::list list) { - while(!list.empty()) - list.pop_front(); // no-warning -} - -void testBasicStringSuppression() { - std::basic_string v; - v.push_back(1); // no-warning -} - -void testBasicStringSuppression_append() { - std::basic_string v; - v += 'c'; // no-warning -} - -void testBasicStringSuppression_assign(std::basic_string &v, - const std::basic_string &v2) { - v = v2; -} - -class MyEngine; -void testSupprerssion_independent_bits_engine(MyEngine& e) { - std::__independent_bits_engine x(e, 64); // no-warning -} -- 2.40.0