[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
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182071 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 2d7e0b4..2ef802d 100644
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -201,6 +201,9 @@
/// \sa mayInlineCXXContainerCtorsAndDtors
Optional<bool> InlineCXXContainerCtorsAndDtors;
+ /// \sa mayInlineCXXSharedPtrDtor
+ Optional<bool> InlineCXXSharedPtrDtor;
+
/// \sa mayInlineObjCMethod
Optional<bool> ObjCInliningMode;
@@ -294,6 +297,16 @@
/// 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.
///
diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 93d4fcd..9dcf58b 100644
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -140,6 +140,12 @@
/*Default=*/false);
}
+bool AnalyzerOptions::mayInlineCXXSharedPtrDtor() {
+ return getBooleanOption(InlineCXXSharedPtrDtor,
+ "c++-shared_ptr-inlining",
+ /*Default=*/false);
+}
+
bool AnalyzerOptions::mayInlineObjCMethod() {
return getBooleanOption(ObjCInliningMode,
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 06570a4..b76649e 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -717,6 +717,21 @@
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 @@
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;
+
}
}
diff --git a/test/Analysis/NewDelete-checker-test.cpp b/test/Analysis/NewDelete-checker-test.cpp
index 5d134bc..8379635 100644
--- a/test/Analysis/NewDelete-checker-test.cpp
+++ b/test/Analysis/NewDelete-checker-test.cpp
@@ -206,3 +206,98 @@
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;
+ }
+}
+
diff --git a/test/Analysis/analyzer-config.cpp b/test/Analysis/analyzer-config.cpp
index bf18a5e..521344a 100644
--- a/test/Analysis/analyzer-config.cpp
+++ b/test/Analysis/analyzer-config.cpp
@@ -13,6 +13,7 @@
// 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 @@
// 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
