From b11a9086ebaf8e081daa8a6cd94ea99c97c027d2 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Thu, 4 Apr 2013 23:10:29 +0000 Subject: [PATCH] [analyzer] Enable destructor inlining by default (c++-inlining=destructors). This turns on not only destructor inlining, but inlining of constructors for types with non-trivial destructors. Per r178516, we will still not inline the constructor or destructor of anything that looks like a container unless the analyzer-config option 'c++-container-inlining' is set to 'true'. In addition to the more precise path-sensitive model, this allows us to catch simple smart pointer issues: #include void test() { std::auto_ptr releaser(new int[4]); } // memory allocated with 'new[]' should not be deleted with 'delete' git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178805 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/analyzer/IPA.txt | 41 ++++++++++++++++++--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp | 2 +- test/Analysis/alloc-match-dealloc.mm | 34 +++++++++++++++++ test/Analysis/analyzer-config.cpp | 2 +- 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/docs/analyzer/IPA.txt b/docs/analyzer/IPA.txt index 4cffcb7454..01e73cec7f 100644 --- a/docs/analyzer/IPA.txt +++ b/docs/analyzer/IPA.txt @@ -46,11 +46,14 @@ Each of these modes implies that all the previous member function kinds will be inlined as well; it doesn't make sense to inline destructors without inlining constructors, for example. -The default c++-inlining mode is 'constructors', meaning that member functions, -overloaded operators, and some constructors will be inlined. If a type has a -non-trivial destructor, however, its constructor will not be inlined. Note that -no C++ member functions will be inlined under -analyzer-config ipa=none or --analyzer-config ipa=basic-inlining. +The default c++-inlining mode is 'destructors', meaning that all member +functions with visible definitions will be considered for inlining. In some +cases the analyzer may still choose not to inline the function. + +Note that under 'constructors', constructors for types with non-trivial +destructors will not be inlined. Additionally, no C++ member functions will be +inlined under -analyzer-config ipa=none or -analyzer-config ipa=basic-inlining, +regardless of the setting of the c++-inlining mode. ### c++-template-inlining ### @@ -73,7 +76,8 @@ considered for inlining. -analyzer-config c++-template-inlining=[true | false] -Currently, C++ standard library functions are NOT considered for inlining by default. +Currently, C++ standard library functions are considered for inlining by +default. The standard library functions and the STL in particular are used ubiquitously enough that our tolerance for false positives is even lower here. A false @@ -81,6 +85,31 @@ positive due to poor modeling of the STL leads to a poor user experience, since most users would not be comfortable adding assertions to system headers in order to silence analyzer warnings. +### c++-container-inlining ### + +This option controls whether constructors and destructors of "container" types +should be considered for inlining. + + -analyzer-config c++-container-inlining=[true | false] + +Currently, these constructors and destructors are NOT considered for inlining +by default. + +The current implementation of this setting checks whether a type has a member +named 'iterator' or a member named 'begin'; these names are idiomatic in C++, +with the latter specified in the C++11 standard. The analyzer currently does a +fairly poor job of modeling certain data structure invariants of container-like +objects. For example, these three expressions should be equivalent: + + std::distance(c.begin(), c.end()) == 0 + c.begin() == c.end() + c.empty()) + +Many of these issues are avoided if containers always have unknown, symbolic +state, which is what happens when their constructors are treated as opaque. +In the future, we may decide specific containers are "safe" to model through +inlining, or choose to model them directly using checkers instead. + Basics of Implementation ----------------------- diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp index 64e1bae8b9..ae707395fc 100644 --- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp +++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp @@ -74,7 +74,7 @@ AnalyzerOptions::mayInlineCXXMemberFunction(CXXInlineableMemberKind K) { static const char *ModeKey = "c++-inlining"; StringRef ModeStr(Config.GetOrCreateValue(ModeKey, - "constructors").getValue()); + "destructors").getValue()); CXXInlineableMemberKind &MutableMode = const_cast(CXXMemberInliningMode); diff --git a/test/Analysis/alloc-match-dealloc.mm b/test/Analysis/alloc-match-dealloc.mm index 5a7ec1e8b0..56d46d99b0 100644 --- a/test/Analysis/alloc-match-dealloc.mm +++ b/test/Analysis/alloc-match-dealloc.mm @@ -185,3 +185,37 @@ void testStandardPlacementNewAfterDelete() { delete p; p = new(p) int; // no-warning } + + +// Smart pointer example +template +struct SimpleSmartPointer { + T *ptr; + + explicit SimpleSmartPointer(T *p = 0) : ptr(p) {} + ~SimpleSmartPointer() { + delete ptr; + // expected-warning@-1 {{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}} + // expected-warning@-2 {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}} + } +}; + +void testSimpleSmartPointerArrayNew() { + { + SimpleSmartPointer a(new int); + } // no-warning + + { + SimpleSmartPointer a(new int[4]); + } +} + +void testSimpleSmartPointerMalloc() { + { + SimpleSmartPointer a(new int); + } // no-warning + + { + SimpleSmartPointer a((int *)malloc(4)); + } +} diff --git a/test/Analysis/analyzer-config.cpp b/test/Analysis/analyzer-config.cpp index df1d486797..1224204f8c 100644 --- a/test/Analysis/analyzer-config.cpp +++ b/test/Analysis/analyzer-config.cpp @@ -12,7 +12,7 @@ public: // CHECK: [config] // CHECK-NEXT: c++-container-inlining = false -// CHECK-NEXT: c++-inlining = constructors +// CHECK-NEXT: c++-inlining = destructors // CHECK-NEXT: c++-stdlib-inlining = true // CHECK-NEXT: c++-template-inlining = true // CHECK-NEXT: cfg-conditional-static-initializers = true -- 2.40.0