r328066 - [analyzer] Suppress more MallocChecker positives in smart pointer destructors.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 20 17:49:47 PDT 2018
Author: dergachev
Date: Tue Mar 20 17:49:47 2018
New Revision: 328066
URL: http://llvm.org/viewvc/llvm-project?rev=328066&view=rev
Log:
[analyzer] Suppress more MallocChecker positives in smart pointer destructors.
r326249 wasn't quite enough because we often run out of inlining stack depth
limit and for that reason fail to see the atomics we're looking for.
Add a more straightforward false positive suppression that is based on the name
of the class. I.e. if we're releasing a pointer in a destructor of a "something
shared/intrusive/reference/counting something ptr/pointer something", then any
use-after-free or double-free that occurs later would likely be a false
positive.
rdar://problem/38013606
Differential Revision: https://reviews.llvm.org/D44281
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/NewDelete-atomics.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=328066&r1=328065&r2=328066&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Tue Mar 20 17:49:47 2018
@@ -2829,6 +2829,19 @@ static SymbolRef findFailedReallocSymbol
return nullptr;
}
+static bool isReferenceCountingPointerDestructor(const CXXDestructorDecl *DD) {
+ if (const IdentifierInfo *II = DD->getParent()->getIdentifier()) {
+ StringRef N = II->getName();
+ if (N.contains_lower("ptr") || N.contains_lower("pointer")) {
+ if (N.contains_lower("ref") || N.contains_lower("cnt") ||
+ N.contains_lower("intrusive") || N.contains_lower("shared")) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode(
const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
BugReport &BR) {
@@ -2882,21 +2895,33 @@ std::shared_ptr<PathDiagnosticPiece> Mal
StackHint = new StackHintGeneratorForSymbol(Sym,
"Returning; memory was released");
- // See if we're releasing memory while inlining a destructor (or one of
- // its callees). If so, enable the atomic-related suppression within that
- // destructor (and all of its callees), which would kick in while visiting
- // other nodes (the visit order is from the bug to the graph root).
+ // See if we're releasing memory while inlining a destructor
+ // (or one of its callees). This turns on various common
+ // false positive suppressions.
+ bool FoundAnyDestructor = false;
for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) {
- if (isa<CXXDestructorDecl>(LC->getDecl())) {
- assert(!ReleaseDestructorLC &&
- "There can be only one release point!");
- ReleaseDestructorLC = LC->getCurrentStackFrame();
- // It is unlikely that releasing memory is delegated to a destructor
- // inside a destructor of a shared pointer, because it's fairly hard
- // to pass the information that the pointer indeed needs to be
- // released into it. So we're only interested in the innermost
- // destructor.
- break;
+ if (const auto *DD = dyn_cast<CXXDestructorDecl>(LC->getDecl())) {
+ if (isReferenceCountingPointerDestructor(DD)) {
+ // This immediately looks like a reference-counting destructor.
+ // We're bad at guessing the original reference count of the object,
+ // so suppress the report for now.
+ BR.markInvalid(getTag(), DD);
+ } else if (!FoundAnyDestructor) {
+ assert(!ReleaseDestructorLC &&
+ "There can be only one release point!");
+ // Suspect that it's a reference counting pointer destructor.
+ // On one of the next nodes might find out that it has atomic
+ // reference counting operations within it (see the code above),
+ // and if so, we'd conclude that it likely is a reference counting
+ // pointer destructor.
+ ReleaseDestructorLC = LC->getCurrentStackFrame();
+ // It is unlikely that releasing memory is delegated to a destructor
+ // inside a destructor of a shared pointer, because it's fairly hard
+ // to pass the information that the pointer indeed needs to be
+ // released into it. So we're only interested in the innermost
+ // destructor.
+ FoundAnyDestructor = true;
+ }
}
}
} else if (isRelinquished(RS, RSPrev, S)) {
Modified: cfe/trunk/test/Analysis/NewDelete-atomics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-atomics.cpp?rev=328066&r1=328065&r2=328066&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/NewDelete-atomics.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-atomics.cpp Tue Mar 20 17:49:47 2018
@@ -2,6 +2,10 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -DTEST_INLINABLE_ALLOCATORS -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-inline-max-stack-depth 2 -analyzer-config ipa-always-inline-size=2 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-inline-max-stack-depth 2 -analyzer-config ipa-always-inline-size=2 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-inline-max-stack-depth 2 -analyzer-config ipa-always-inline-size=2 -analyzer-checker=core,cplusplus.NewDelete -std=c++11 -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-inline-max-stack-depth 2 -analyzer-config ipa-always-inline-size=2 -analyzer-checker=core,cplusplus.NewDeleteLeaks -DLEAKS -std=c++11 -DTEST_INLINABLE_ALLOCATORS -verify %s
// expected-no-diagnostics
@@ -51,7 +55,7 @@ public:
delete Ptr;
}
- Obj *getPtr() const { return Ptr; }
+ Obj *getPtr() const { return Ptr; } // no-warning
};
void testDestroyLocalRefPtr() {
More information about the cfe-commits
mailing list