r211832 - Do not inline methods of C++ containers (coming from headers).
Anna Zaks
ganna at apple.com
Thu Jun 26 18:03:06 PDT 2014
Author: zaks
Date: Thu Jun 26 20:03:05 2014
New Revision: 211832
URL: http://llvm.org/viewvc/llvm-project?rev=211832&view=rev
Log:
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]).
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/inlining/containers.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=211832&r1=211831&r2=211832&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu Jun 26 20:03:05 2014
@@ -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.
Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=211832&r1=211831&r2=211832&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Thu Jun 26 20:03:05 2014
@@ -140,8 +140,8 @@ bool AnalyzerOptions::mayInlineCXXAlloca
/*Default=*/false);
}
-bool AnalyzerOptions::mayInlineCXXContainerCtorsAndDtors() {
- return getBooleanOption(InlineCXXContainerCtorsAndDtors,
+bool AnalyzerOptions::mayInlineCXXContainerMethods() {
+ return getBooleanOption(InlineCXXContainerMethods,
"c++-container-inlining",
/*Default=*/false);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=211832&r1=211831&r2=211832&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Thu Jun 26 20:03:05 2014
@@ -708,18 +708,16 @@ static bool isContainerClass(const ASTCo
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(AnalysisDeclCo
// 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.
Modified: cfe/trunk/test/Analysis/inlining/containers.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/containers.cpp?rev=211832&r1=211831&r2=211832&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/containers.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/containers.cpp Thu Jun 26 20:03:05 2014
@@ -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 at -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 at -2 {{TRUE}}
+ #endif
return iterator(storage);
}
iterator end() {
- clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+ clang_analyzer_checkInlined(true);
+ #if INLINE
+ // expected-warning at -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 at -2 {{TRUE}}
+ #endif
return storage;
}
raw_iterator raw_end() {
- clang_analyzer_checkInlined(true); // expected-warning {{TRUE}}
+ clang_analyzer_checkInlined(true);
+ #if INLINE
+ // expected-warning at -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 at -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 at -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 at -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 at -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 at -2 {{TRUE}}
+ #endif
return iterator{impl.begin().impl};
}
};
More information about the cfe-commits
mailing list