[analyzer] For now, don't inline [cd]tors of C++ containers.

This is a heuristic to make up for the fact that the analyzer doesn't
model C++ containers very well. One example is modeling that
'std::distance(I, E) == 0' implies 'I == E'. In the future, it would be
nice to model this explicitly, but for now it just results in a lot of
false positives.

The actual heuristic checks if the base type has a member named 'begin' or
'iterator'. If so, we treat the constructors and destructors of that type
as opaque, rather than inlining them.

This is intended to drastically reduce the number of false positives
reported with experimental destructor support turned on. We can tweak the
heuristic in the future, but we'd rather err on the side of false negatives
for now.

<rdar://problem/13497258>

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

diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index b9f6f6cce14286592f3fc24eacb330a6c2db5244..6dbdbbf89b96b4ae97a1ed1feda9ce8d441002ee 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -198,6 +198,9 @@ private:
   /// \sa mayInlineTemplateFunctions
   Optional<bool> InlineTemplateFunctions;
 
+  /// \sa mayInlineCXXContainerCtorsAndDtors
+  Optional<bool> InlineCXXContainerCtorsAndDtors;
+
   /// \sa mayInlineObjCMethod
   Optional<bool> ObjCInliningMode;
 
@@ -281,6 +284,13 @@ public:
   /// accepts the values "true" and "false".
   bool mayInlineTemplateFunctions();
 
+  /// Returns whether or not constructors and destructors of C++ container
+  /// objects may be considered for inlining.
+  ///
+  /// This is controlled by the 'c++-container-inlining' config option, which
+  /// accepts the values "true" and "false".
+  bool mayInlineCXXContainerCtorsAndDtors();
+
   /// Returns whether or not paths that go through null returns should be
   /// suppressed.
   ///

diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 465f408e7f28b47bb34e07f6da71398169d794c2..64e1bae8b95f35b29c7a745172d9bdc30aad95fd 100644 (file)
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -134,6 +134,13 @@ bool AnalyzerOptions::mayInlineTemplateFunctions() {
                           /*Default=*/true);
 }
 
+bool AnalyzerOptions::mayInlineCXXContainerCtorsAndDtors() {
+  return getBooleanOption(InlineCXXContainerCtorsAndDtors,
+                          "c++-container-inlining",
+                          /*Default=*/false);
+}
+
+
 bool AnalyzerOptions::mayInlineObjCMethod() {
   return getBooleanOption(ObjCInliningMode,
                           "objc-inlining",

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 67b249c5e9794a01e4be3497f7a061f1f749b354..ee603d728e4e6dab2d64ee0c0a926b107541dbea 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -674,6 +674,53 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call,
   return CIP_Allowed;
 }
 
+/// Returns true if the given C++ class is a container.
+///
+/// Our heuristic for this is whether it contains a method named 'begin()' or a
+/// nested type named 'iterator'.
+static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) {
+  // Don't record any path information.
+  CXXBasePaths Paths(false, false, false);
+
+  const IdentifierInfo &BeginII = Ctx.Idents.get("begin");
+  DeclarationName BeginName = Ctx.DeclarationNames.getIdentifier(&BeginII);
+  DeclContext::lookup_const_result BeginDecls = RD->lookup(BeginName);
+  if (!BeginDecls.empty())
+    return true;
+  if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember,
+                        BeginName.getAsOpaquePtr(),
+                        Paths))
+    return true;
+  
+  const IdentifierInfo &IterII = Ctx.Idents.get("iterator");
+  DeclarationName IteratorName = Ctx.DeclarationNames.getIdentifier(&IterII);
+  DeclContext::lookup_const_result IterDecls = RD->lookup(IteratorName);
+  if (!IterDecls.empty())
+    return true;
+  if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember,
+                        IteratorName.getAsOpaquePtr(),
+                        Paths))
+    return true;
+
+  return false;
+}
+
+/// Returns true if the given function refers to a constructor or destructor of
+/// a C++ container.
+///
+/// We generally do a poor job modeling most containers right now, and would
+/// prefer not to inline their methods.
+static bool isContainerCtorOrDtor(const ASTContext &Ctx,
+                                  const FunctionDecl *FD) {
+  // Heuristic: a type is a container if it contains a "begin()" method
+  // or a type named "iterator".
+  if (!(isa<CXXConstructorDecl>(FD) || isa<CXXDestructorDecl>(FD)))
+    return false;
+
+  const CXXRecordDecl *RD = cast<CXXMethodDecl>(FD)->getParent();
+  return isContainerClass(Ctx, RD);
+}
+
 /// Returns true if the function in \p CalleeADC may be inlined in general.
 ///
 /// This checks static properties of the function, such as its signature and
@@ -681,16 +728,14 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call,
 /// in any context.
 static bool mayInlineDecl(const CallEvent &Call, AnalysisDeclContext *CalleeADC,
                           AnalyzerOptions &Opts) {
-  const Decl *D = CalleeADC->getDecl();
-
   // FIXME: Do not inline variadic calls.
   if (Call.isVariadic())
     return false;
 
   // Check certain C++-related inlining policies.
-  ASTContext &Ctx = D->getASTContext();
+  ASTContext &Ctx = CalleeADC->getASTContext();
   if (Ctx.getLangOpts().CPlusPlus) {
-    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CalleeADC->getDecl())) {
       // Conditionally control the inlining of template functions.
       if (!Opts.mayInlineTemplateFunctions())
         if (FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate)
@@ -701,6 +746,13 @@ static bool mayInlineDecl(const CallEvent &Call, AnalysisDeclContext *CalleeADC,
         if (Ctx.getSourceManager().isInSystemHeader(FD->getLocation()))
           if (IsInStdNamespace(FD))
             return false;
+
+      // Conditionally control the inlining of methods on objects that look
+      // like C++ containers.
+      if (!Opts.mayInlineCXXContainerCtorsAndDtors())
+        if (!Ctx.getSourceManager().isFromMainFile(FD->getLocation()))
+          if (isContainerCtorOrDtor(Ctx, FD))
+            return false;
     }
   }
 

diff --git a/test/Analysis/analyzer-config.cpp b/test/Analysis/analyzer-config.cpp
index c3a04c946e7addece3d61d0547ac970fe8e9f056..df1d486797fa3c5134eea135f615a81dcde417ba 100644 (file)
--- a/test/Analysis/analyzer-config.cpp
+++ b/test/Analysis/analyzer-config.cpp
@@ -11,6 +11,7 @@ public:
 };
 
 // CHECK: [config]
+// CHECK-NEXT: c++-container-inlining = false
 // CHECK-NEXT: c++-inlining = constructors
 // CHECK-NEXT: c++-stdlib-inlining = true
 // CHECK-NEXT: c++-template-inlining = true
@@ -25,4 +26,4 @@ public:
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 13
+// CHECK-NEXT: num-entries = 14

diff --git a/test/Analysis/diagnostics/explicit-suppression.cpp b/test/Analysis/diagnostics/explicit-suppression.cpp
index 6bc950f5d500dd309e2a77eec1bd6c459708b81a..79afeed6c56d1dcef1797eae385bf950eaa93e3a 100644 (file)
--- a/test/Analysis/diagnostics/explicit-suppression.cpp
+++ b/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -13,7 +13,7 @@ void testCopyNull(int *I, int *E) {
   std::copy(I, E, (int *)0);
 #ifndef SUPPRESSED
   // This line number comes from system-header-simulator-cxx.h.
-  // expected-warning@65 {{Dereference of null pointer}}
+  // expected-warning@79 {{Dereference of null pointer}}
 #endif
 }
 
@@ -56,6 +56,15 @@ void testCopyNull(int *I, int *E) {
 
 
 
+
+
+
+
+
+
+
+
+
 
 
 

diff --git a/test/Analysis/inlining/containers.cpp b/test/Analysis/inlining/containers.cpp
new file mode 100644 (file)
index 0000000..4500dff
--- /dev/null
+++ b/test/Analysis/inlining/containers.cpp
@@ -0,0 +1,234 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors -analyzer-config c++-container-inlining=false -verify %s
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors -analyzer-config c++-container-inlining=true -DINLINE=1 -verify %s
+
+#ifndef HEADER
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
+
+#define HEADER
+#include "containers.cpp"
+#undef HEADER
+
+void test() {
+  MySet set(0);
+
+  clang_analyzer_eval(set.isEmpty());
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+
+  clang_analyzer_eval(set.raw_begin() == set.raw_end());
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+
+  clang_analyzer_eval(set.begin().impl == set.end().impl);
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+}
+
+void testSubclass(MySetSubclass &sub) {
+  sub.useIterator(sub.begin());
+
+  MySetSubclass local;
+}
+
+void testWrappers(BeginOnlySet &w1, IteratorStructOnlySet &w2,
+                  IteratorTypedefOnlySet &w3, IteratorUsingOnlySet &w4) {
+  BeginOnlySet local1;
+  IteratorStructOnlySet local2;
+  IteratorTypedefOnlySet local3;
+  IteratorUsingOnlySet local4;
+
+  clang_analyzer_eval(w1.begin().impl.impl == w1.begin().impl.impl);
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+
+  clang_analyzer_eval(w2.start().impl == w2.start().impl);
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+
+  clang_analyzer_eval(w3.start().impl == w3.start().impl);
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+
+  clang_analyzer_eval(w4.start().impl == w4.start().impl);
+#if INLINE
+  // expected-warning@-2 {{TRUE}}
+#else
+  // expected-warning@-4 {{UNKNOWN}}
+#endif
+}
+
+
+#else
+
+class MySet {
+  int *storage;
+  unsigned size;
+public:
+  MySet() : storage(0), size(0) {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  MySet(unsigned n) : storage(new int[n]), size(n) {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  ~MySet() { delete[] storage; }
+
+  bool isEmpty() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return size == 0;
+  }
+
+  struct iterator {
+    int *impl;
+
+    iterator(int *p) : impl(p) {}
+  };
+
+  iterator begin() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return iterator(storage);
+  }
+
+  iterator end() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return iterator(storage+size);
+  }
+
+  typedef int *raw_iterator;
+
+  raw_iterator raw_begin() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return storage;
+  }
+  raw_iterator raw_end() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return storage + size;
+  }
+};
+
+class MySetSubclass : public MySet {
+public:
+  MySetSubclass() {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  void useIterator(iterator i) {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+  }
+};
+
+class BeginOnlySet {
+  MySet impl;
+public:
+  struct IterImpl {
+    MySet::iterator impl;
+    IterImpl(MySet::iterator i) : impl(i) {
+      clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    }
+  };
+
+  BeginOnlySet() {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  typedef IterImpl wrapped_iterator;
+
+  wrapped_iterator begin() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return IterImpl(impl.begin());
+  }
+};
+
+class IteratorTypedefOnlySet {
+  MySet impl;
+public:
+
+  IteratorTypedefOnlySet() {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  typedef MySet::iterator iterator;
+
+  iterator start() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return impl.begin();
+  }
+};
+
+class IteratorUsingOnlySet {
+  MySet impl;
+public:
+
+  IteratorUsingOnlySet() {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  using iterator = MySet::iterator;
+
+  iterator start() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return impl.begin();
+  }
+};
+
+class IteratorStructOnlySet {
+  MySet impl;
+public:
+
+  IteratorStructOnlySet() {
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
+  }
+
+  struct iterator {
+    int *impl;
+  };
+
+  iterator start() {
+    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    return iterator{impl.begin().impl};
+  }
+};
+
+#endif

diff --git a/test/Analysis/inlining/stl.cpp b/test/Analysis/inlining/stl.cpp
index b09a5126bc639364c218bd5f3cd86c2fcb5e7ca1..6053daaf3a21aa6ab8705595b35d775252ec2803 100644 (file)
--- a/test/Analysis/inlining/stl.cpp
+++ b/test/Analysis/inlining/stl.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-stdlib-inlining=false -verify %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-stdlib-inlining=true -DINLINE=1 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-container-inlining=true -analyzer-config c++-stdlib-inlining=false -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-container-inlining=true -analyzer-config c++-stdlib-inlining=true -DINLINE=1 -verify %s
 
 #include "../Inputs/system-header-simulator-cxx.h"