r182071 - [analyzer] Don't inline ~shared_ptr.
Jordan Rose
jordan_rose at apple.com
Thu May 16 19:16:49 PDT 2013
Author: jrose
Date: Thu May 16 21:16:49 2013
New Revision: 182071
URL: http://llvm.org/viewvc/llvm-project?rev=182071&view=rev
Log:
[analyzer] Don't inline ~shared_ptr.
The analyzer can't see the reference count for shared_ptr, so it doesn't
know whether a given destruction is going to delete the referenced object.
This leads to spurious leak and use-after-free warnings.
For now, just ban destructors named '~shared_ptr', which catches
std::shared_ptr, std::tr1::shared_ptr, and boost::shared_ptr.
PR15987
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/NewDelete-checker-test.cpp
cfe/trunk/test/Analysis/analyzer-config.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=182071&r1=182070&r2=182071&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu May 16 21:16:49 2013
@@ -201,6 +201,9 @@ private:
/// \sa mayInlineCXXContainerCtorsAndDtors
Optional<bool> InlineCXXContainerCtorsAndDtors;
+ /// \sa mayInlineCXXSharedPtrDtor
+ Optional<bool> InlineCXXSharedPtrDtor;
+
/// \sa mayInlineObjCMethod
Optional<bool> ObjCInliningMode;
@@ -294,6 +297,16 @@ public:
/// accepts the values "true" and "false".
bool mayInlineCXXContainerCtorsAndDtors();
+ /// Returns whether or not the destructor of C++ 'shared_ptr' may be
+ /// considered for inlining.
+ ///
+ /// This covers std::shared_ptr, std::tr1::shared_ptr, and boost::shared_ptr,
+ /// and indeed any destructor named "~shared_ptr".
+ ///
+ /// This is controlled by the 'c++-shared_ptr-inlining' config option, which
+ /// accepts the values "true" and "false".
+ bool mayInlineCXXSharedPtrDtor();
+
/// Returns whether or not paths that go through null returns should be
/// suppressed.
///
Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=182071&r1=182070&r2=182071&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Thu May 16 21:16:49 2013
@@ -140,6 +140,12 @@ bool AnalyzerOptions::mayInlineCXXContai
/*Default=*/false);
}
+bool AnalyzerOptions::mayInlineCXXSharedPtrDtor() {
+ return getBooleanOption(InlineCXXSharedPtrDtor,
+ "c++-shared_ptr-inlining",
+ /*Default=*/false);
+}
+
bool AnalyzerOptions::mayInlineObjCMethod() {
return getBooleanOption(ObjCInliningMode,
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=182071&r1=182070&r2=182071&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Thu May 16 21:16:49 2013
@@ -717,6 +717,21 @@ static bool isContainerCtorOrDtor(const
return isContainerClass(Ctx, RD);
}
+/// Returns true if the given function is the destructor of a class named
+/// "shared_ptr".
+static bool isCXXSharedPtrDtor(const FunctionDecl *FD) {
+ const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(FD);
+ if (!Dtor)
+ return false;
+
+ const CXXRecordDecl *RD = Dtor->getParent();
+ if (const IdentifierInfo *II = RD->getDeclName().getAsIdentifierInfo())
+ if (II->isStr("shared_ptr"))
+ return true;
+
+ return false;
+}
+
/// 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
@@ -749,6 +764,15 @@ static bool mayInlineDecl(const CallEven
if (!Ctx.getSourceManager().isFromMainFile(FD->getLocation()))
if (isContainerCtorOrDtor(Ctx, FD))
return false;
+
+ // Conditionally control the inlining of the destructor of C++ shared_ptr.
+ // We don't currently do a good job modeling shared_ptr because we can't
+ // see the reference count, so treating as opaque is probably the best
+ // idea.
+ if (!Opts.mayInlineCXXSharedPtrDtor())
+ if (isCXXSharedPtrDtor(FD))
+ return false;
+
}
}
Modified: cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-checker-test.cpp?rev=182071&r1=182070&r2=182071&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/NewDelete-checker-test.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-checker-test.cpp Thu May 16 21:16:49 2013
@@ -206,3 +206,98 @@ void testConstEscapePlacementNew() {
void *y = new (x) int;
escapeVoidPtr(y);
} // no-warning
+
+
+namespace reference_count {
+ class control_block {
+ unsigned count;
+ public:
+ control_block() : count(0) {}
+ void retain() { ++count; }
+ int release() { return --count; }
+ };
+
+ template <typename T>
+ class shared_ptr {
+ T *p;
+ control_block *control;
+
+ public:
+ shared_ptr() : p(0), control(0) {}
+ explicit shared_ptr(T *p) : p(p), control(new control_block) {
+ control->retain();
+ }
+ shared_ptr(shared_ptr &other) : p(other.p), control(other.control) {
+ if (control)
+ control->retain();
+ }
+ ~shared_ptr() {
+ if (control && control->release() == 0) {
+ delete p;
+ delete control;
+ }
+ };
+
+ T &operator *() {
+ return *p;
+ };
+
+ void swap(shared_ptr &other) {
+ T *tmp = p;
+ p = other.p;
+ other.p = tmp;
+
+ control_block *ctrlTmp = control;
+ control = other.control;
+ other.control = ctrlTmp;
+ }
+ };
+
+ void testSingle() {
+ shared_ptr<int> a(new int);
+ *a = 1;
+ }
+
+ void testDouble() {
+ shared_ptr<int> a(new int);
+ shared_ptr<int> b = a;
+ *a = 1;
+ }
+
+ void testInvalidated() {
+ shared_ptr<int> a(new int);
+ shared_ptr<int> b = a;
+ *a = 1;
+
+ extern void use(shared_ptr<int> &);
+ use(b);
+ }
+
+ void testNestedScope() {
+ shared_ptr<int> a(new int);
+ {
+ shared_ptr<int> b = a;
+ }
+ *a = 1;
+ }
+
+ void testSwap() {
+ shared_ptr<int> a(new int);
+ shared_ptr<int> b;
+ shared_ptr<int> c = a;
+ shared_ptr<int>(c).swap(b);
+ }
+
+ void testUseAfterFree() {
+ int *p = new int;
+ {
+ shared_ptr<int> a(p);
+ shared_ptr<int> b = a;
+ }
+
+ // FIXME: We should get a warning here, but we don't because we've
+ // conservatively modeled ~shared_ptr.
+ *p = 1;
+ }
+}
+
Modified: cfe/trunk/test/Analysis/analyzer-config.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=182071&r1=182070&r2=182071&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/analyzer-config.cpp (original)
+++ cfe/trunk/test/Analysis/analyzer-config.cpp Thu May 16 21:16:49 2013
@@ -13,6 +13,7 @@ public:
// CHECK: [config]
// CHECK-NEXT: c++-container-inlining = false
// CHECK-NEXT: c++-inlining = destructors
+// CHECK-NEXT: c++-shared_ptr-inlining = false
// CHECK-NEXT: c++-stdlib-inlining = true
// CHECK-NEXT: c++-template-inlining = true
// CHECK-NEXT: cfg-conditional-static-initializers = true
@@ -28,4 +29,4 @@ public:
// CHECK-NEXT: mode = deep
// CHECK-NEXT: region-store-small-struct-limit = 2
// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 16
+// CHECK-NEXT: num-entries = 17
More information about the cfe-commits
mailing list