r178805 - [analyzer] Enable destructor inlining by default (c++-inlining=destructors).

Jordan Rose jordan_rose at apple.com
Thu Apr 4 16:10:29 PDT 2013


Author: jrose
Date: Thu Apr  4 18:10:29 2013
New Revision: 178805

URL: http://llvm.org/viewvc/llvm-project?rev=178805&view=rev
Log:
[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 <memory>

  void test() {
    std::auto_ptr<int> releaser(new int[4]);
  } // memory allocated with 'new[]' should not be deleted with 'delete'

<rdar://problem/12295363>

Modified:
    cfe/trunk/docs/analyzer/IPA.txt
    cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
    cfe/trunk/test/Analysis/alloc-match-dealloc.mm
    cfe/trunk/test/Analysis/analyzer-config.cpp

Modified: cfe/trunk/docs/analyzer/IPA.txt
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/analyzer/IPA.txt?rev=178805&r1=178804&r2=178805&view=diff
==============================================================================
--- cfe/trunk/docs/analyzer/IPA.txt (original)
+++ cfe/trunk/docs/analyzer/IPA.txt Thu Apr  4 18:10:29 2013
@@ -46,11 +46,14 @@ Each of these modes implies that all the
 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
 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
 -----------------------

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=178805&r1=178804&r2=178805&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Thu Apr  4 18:10:29 2013
@@ -74,7 +74,7 @@ AnalyzerOptions::mayInlineCXXMemberFunct
     static const char *ModeKey = "c++-inlining";
     
     StringRef ModeStr(Config.GetOrCreateValue(ModeKey,
-                                              "constructors").getValue());
+                                              "destructors").getValue());
 
     CXXInlineableMemberKind &MutableMode =
       const_cast<CXXInlineableMemberKind &>(CXXMemberInliningMode);

Modified: cfe/trunk/test/Analysis/alloc-match-dealloc.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/alloc-match-dealloc.mm?rev=178805&r1=178804&r2=178805&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/alloc-match-dealloc.mm (original)
+++ cfe/trunk/test/Analysis/alloc-match-dealloc.mm Thu Apr  4 18:10:29 2013
@@ -185,3 +185,37 @@ void testStandardPlacementNewAfterDelete
   delete p;
   p = new(p) int; // no-warning
 }
+
+
+// Smart pointer example
+template <typename T>
+struct SimpleSmartPointer {
+  T *ptr;
+
+  explicit SimpleSmartPointer(T *p = 0) : ptr(p) {}
+  ~SimpleSmartPointer() {
+    delete ptr;
+    // expected-warning at -1 {{Memory allocated by 'new[]' should be deallocated by 'delete[]', not 'delete'}}
+    // expected-warning at -2 {{Memory allocated by malloc() should be deallocated by free(), not 'delete'}}
+  }
+};
+
+void testSimpleSmartPointerArrayNew() {
+  {
+    SimpleSmartPointer<int> a(new int);
+  } // no-warning
+
+  {
+    SimpleSmartPointer<int> a(new int[4]);
+  }
+}
+
+void testSimpleSmartPointerMalloc() {
+  {
+    SimpleSmartPointer<int> a(new int);
+  } // no-warning
+
+  {
+    SimpleSmartPointer<int> a((int *)malloc(4));
+  }
+}

Modified: cfe/trunk/test/Analysis/analyzer-config.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=178805&r1=178804&r2=178805&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/analyzer-config.cpp (original)
+++ cfe/trunk/test/Analysis/analyzer-config.cpp Thu Apr  4 18:10:29 2013
@@ -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





More information about the cfe-commits mailing list