Do not inline methods of C++ containers (coming from headers).

This silences false positives (leaks, use of uninitialized value) in simple
code that uses containers such as std::vector and std::list. The analyzer
cannot reason about the internal invariances of those data structures which
leads to false positives. Until we come up with a better solution to that
problem, let's just not inline the methods of the containers and allow objects
to escape whenever such methods are called.

This just extends an already existing flag "c++-container-inlining" and applies
the heuristic not only to constructors and destructors of the containers, but
to all of their methods.

We have a bunch of distinct user reports all related to this issue
(radar://16058651, radar://16580751, radar://16384286, radar://16795491
[PR19637]).

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

diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 520fed08e334fbab9f25b71a9b320832b4b56a10..978c3e20ab20a7b84da566288be4ec7742eaf6b4 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -202,8 +202,8 @@ private:
   /// \sa mayInlineCXXAllocator
   Optional<bool> InlineCXXAllocator;
 
-  /// \sa mayInlineCXXContainerCtorsAndDtors
-  Optional<bool> InlineCXXContainerCtorsAndDtors;
+  /// \sa mayInlineCXXContainerMethods
+  Optional<bool> InlineCXXContainerMethods;
 
   /// \sa mayInlineCXXSharedPtrDtor
   Optional<bool> InlineCXXSharedPtrDtor;
@@ -303,12 +303,12 @@ public:
   /// accepts the values "true" and "false".
   bool mayInlineCXXAllocator();
 
-  /// Returns whether or not constructors and destructors of C++ container
-  /// objects may be considered for inlining.
+  /// Returns whether or not methods 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();
+  bool mayInlineCXXContainerMethods();
 
   /// Returns whether or not the destructor of C++ 'shared_ptr' may be
   /// considered for inlining.

diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 85d66ba600f026db9289dd60ce2297d927ec78b7..7944c7eb00037681ff238203d52d6f9f6d6b4705 100644 (file)
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -140,8 +140,8 @@ bool AnalyzerOptions::mayInlineCXXAllocator() {
                           /*Default=*/false);
 }
 
-bool AnalyzerOptions::mayInlineCXXContainerCtorsAndDtors() {
-  return getBooleanOption(InlineCXXContainerCtorsAndDtors,
+bool AnalyzerOptions::mayInlineCXXContainerMethods() {
+  return getBooleanOption(InlineCXXContainerMethods,
                           "c++-container-inlining",
                           /*Default=*/false);
 }

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 4619f62bd26aad0ea422dffd4e1a7b55f3602b38..3f608ba79ebc319c8f9f71165a2d7989b72fe4b5 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -708,18 +708,16 @@ static bool isContainerClass(const ASTContext &Ctx, const CXXRecordDecl *RD) {
          hasMember(Ctx, RD, "iterator_category");
 }
 
-/// Returns true if the given function refers to a constructor or destructor of
-/// a C++ container or iterator.
+/// Returns true if the given function refers to a method of a C++ container
+/// or iterator.
 ///
-/// We generally do a poor job modeling most containers right now, and would
-/// prefer not to inline their setup and teardown.
-static bool isContainerCtorOrDtor(const ASTContext &Ctx,
-                                  const FunctionDecl *FD) {
-  if (!(isa<CXXConstructorDecl>(FD) || isa<CXXDestructorDecl>(FD)))
-    return false;
-
-  const CXXRecordDecl *RD = cast<CXXMethodDecl>(FD)->getParent();
-  return isContainerClass(Ctx, RD);
+/// We generally do a poor job modeling most containers right now, and might
+/// prefer not to inline their methods.
+static bool isContainerMethod(const ASTContext &Ctx,
+                              const FunctionDecl *FD) {
+  if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD))
+    return isContainerClass(Ctx, MD->getParent());
+  return false;
 }
 
 /// Returns true if the given function is the destructor of a class named
@@ -765,9 +763,9 @@ static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
 
       // Conditionally control the inlining of methods on objects that look
       // like C++ containers.
-      if (!Opts.mayInlineCXXContainerCtorsAndDtors())
+      if (!Opts.mayInlineCXXContainerMethods())
         if (!Ctx.getSourceManager().isInMainFile(FD->getLocation()))
-          if (isContainerCtorOrDtor(Ctx, FD))
+          if (isContainerMethod(Ctx, FD))
             return false;
             
       // Conditionally control the inlining of the destructor of C++ shared_ptr.

diff --git a/test/Analysis/inlining/containers.cpp b/test/Analysis/inlining/containers.cpp
index 73b2957ad6dedecff8b474aaa63041864e711a91..c757da66bedbf732e9752fcc276a6425e4a63bc3 100644 (file)
--- a/test/Analysis/inlining/containers.cpp
+++ b/test/Analysis/inlining/containers.cpp
@@ -103,7 +103,10 @@ public:
   ~MySet() { delete[] storage; }
 
   bool isEmpty() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return size == 0;
   }
 
@@ -114,23 +117,35 @@ public:
   };
 
   iterator begin() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return iterator(storage);
   }
 
   iterator end() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return iterator(storage+size);
   }
 
   typedef int *raw_iterator;
 
   raw_iterator raw_begin() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return storage;
   }
   raw_iterator raw_end() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return storage + size;
   }
 };
@@ -145,7 +160,10 @@ public:
   }
 
   void useIterator(iterator i) {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
   }
 };
 
@@ -174,7 +192,10 @@ public:
   typedef IterImpl wrapped_iterator;
 
   wrapped_iterator begin() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return IterImpl(impl.begin());
   }
 };
@@ -193,7 +214,10 @@ public:
   typedef MySet::iterator iterator;
 
   iterator start() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+#if INLINE
+    // expected-warning@-2 {{TRUE}}
+#endif
     return impl.begin();
   }
 };
@@ -212,7 +236,10 @@ public:
   using iterator = MySet::iterator;
 
   iterator start() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return impl.begin();
   }
 };
@@ -233,7 +260,10 @@ public:
   };
 
   iterator start() {
-    clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+    clang_analyzer_checkInlined(true);
+    #if INLINE
+        // expected-warning@-2 {{TRUE}}
+    #endif
     return iterator{impl.begin().impl};
   }
 };