r349191 - [analyzer] MoveChecker Pt.6: Suppress the warning for the move-safe STL classes.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 14 12:52:57 PST 2018
Author: dergachev
Date: Fri Dec 14 12:52:57 2018
New Revision: 349191
URL: http://llvm.org/viewvc/llvm-project?rev=349191&view=rev
Log:
[analyzer] MoveChecker Pt.6: Suppress the warning for the move-safe STL classes.
Some C++ standard library classes provide additional guarantees about their
state after move. Suppress warnings on such classes until a more precise
behavior is implemented. Warnings for locals are not suppressed anyway
because it's still most likely a bug.
Differential Revision: https://reviews.llvm.org/D55307
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
cfe/trunk/test/Analysis/use-after-move.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp?rev=349191&r1=349190&r2=349191&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MoveChecker.cpp Fri Dec 14 12:52:57 2018
@@ -20,6 +20,7 @@
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/StringSet.h"
using namespace clang;
using namespace ento;
@@ -67,20 +68,43 @@ private:
bool STL : 1; // Is this an object of a standard type?
};
+ // Not all of these are entirely move-safe, but they do provide *some*
+ // guarantees, and it means that somebody is using them after move
+ // in a valid manner.
+ // TODO: We can still try to identify *unsafe* use after move, such as
+ // dereference of a moved-from smart pointer (which is guaranteed to be null).
+ const llvm::StringSet<> StandardMoveSafeClasses = {
+ "basic_filebuf",
+ "basic_ios",
+ "future",
+ "optional",
+ "packaged_task"
+ "promise",
+ "shared_future",
+ "shared_lock",
+ "shared_ptr",
+ "thread",
+ "unique_ptr",
+ "unique_lock",
+ "weak_ptr",
+ };
+
// Obtains ObjectKind of an object. Because class declaration cannot always
// be easily obtained from the memory region, it is supplied separately.
- static ObjectKind classifyObject(const MemRegion *MR,
- const CXXRecordDecl *RD);
+ ObjectKind classifyObject(const MemRegion *MR, const CXXRecordDecl *RD) const;
// Classifies the object and dumps a user-friendly description string to
// the stream. Return value is equivalent to classifyObject.
- static ObjectKind explainObject(llvm::raw_ostream &OS,
- const MemRegion *MR, const CXXRecordDecl *RD);
+ ObjectKind explainObject(llvm::raw_ostream &OS,
+ const MemRegion *MR, const CXXRecordDecl *RD) const;
+
+ bool isStandardMoveSafeClass(const CXXRecordDecl *RD) const;
class MovedBugVisitor : public BugReporterVisitor {
public:
- MovedBugVisitor(const MemRegion *R, const CXXRecordDecl *RD)
- : Region(R), RD(RD), Found(false) {}
+ MovedBugVisitor(const MoveChecker &Chk,
+ const MemRegion *R, const CXXRecordDecl *RD)
+ : Chk(Chk), Region(R), RD(RD), Found(false) {}
void Profile(llvm::FoldingSetNodeID &ID) const override {
static int X = 0;
@@ -97,6 +121,7 @@ private:
BugReport &BR) override;
private:
+ const MoveChecker &Chk;
// The tracked region.
const MemRegion *Region;
// The class of the tracked object.
@@ -157,7 +182,7 @@ static const MemRegion *unwrapRValueRefe
std::shared_ptr<PathDiagnosticPiece>
MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
- BugReporterContext &BRC, BugReport &) {
+ BugReporterContext &BRC, BugReport &BR) {
// We need only the last move of the reported object's region.
// The visitor walks the ExplodedGraph backwards.
if (Found)
@@ -182,7 +207,7 @@ MoveChecker::MovedBugVisitor::VisitNode(
llvm::raw_svector_ostream OS(Str);
OS << "Object";
- ObjectKind OK = explainObject(OS, Region, RD);
+ ObjectKind OK = Chk.explainObject(OS, Region, RD);
if (OK.STL)
OS << " is left in a valid but unspecified state after move";
else
@@ -251,7 +276,7 @@ ExplodedNode *MoveChecker::reportBug(con
auto R =
llvm::make_unique<BugReport>(*BT, OS.str(), N, LocUsedForUniqueing,
MoveNode->getLocationContext()->getDecl());
- R->addVisitor(llvm::make_unique<MovedBugVisitor>(Region, RD));
+ R->addVisitor(llvm::make_unique<MovedBugVisitor>(*this, Region, RD));
C.emitReport(std::move(R));
return N;
}
@@ -370,20 +395,30 @@ bool MoveChecker::isInMoveSafeContext(co
return false;
}
-MoveChecker::ObjectKind MoveChecker::classifyObject(const MemRegion *MR,
- const CXXRecordDecl *RD) {
+bool MoveChecker::isStandardMoveSafeClass(const CXXRecordDecl *RD) const {
+ const IdentifierInfo *II = RD->getIdentifier();
+ return II && StandardMoveSafeClasses.count(II->getName());
+}
+
+MoveChecker::ObjectKind
+MoveChecker::classifyObject(const MemRegion *MR,
+ const CXXRecordDecl *RD) const {
+ // Local variables and local rvalue references are classified as "Local".
+ // For the purposes of this checker, we classify move-safe STL types
+ // as not-"STL" types, because that's how the checker treats them.
MR = unwrapRValueReferenceIndirection(MR);
return {
/*Local=*/
MR && isa<VarRegion>(MR) && isa<StackSpaceRegion>(MR->getMemorySpace()),
/*STL=*/
- RD && RD->getDeclContext()->isStdNamespace()
+ RD && RD->getDeclContext()->isStdNamespace() &&
+ !isStandardMoveSafeClass(RD)
};
}
-MoveChecker::ObjectKind MoveChecker::explainObject(llvm::raw_ostream &OS,
- const MemRegion *MR,
- const CXXRecordDecl *RD) {
+MoveChecker::ObjectKind
+MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
+ const CXXRecordDecl *RD) const {
// We may need a leading space every time we actually explain anything,
// and we never know if we are to explain anything until we try.
if (const auto DR =
Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h?rev=349191&r1=349190&r2=349191&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h Fri Dec 14 12:52:57 2018
@@ -237,6 +237,13 @@ namespace std {
return static_cast<RvalRef>(a);
}
+ template <class T>
+ void swap(T &a, T &b) {
+ T c(std::move(a));
+ a = std::move(b);
+ b = std::move(c);
+ }
+
template<typename T>
class vector {
typedef T value_type;
@@ -770,6 +777,22 @@ namespace std {
}
+#if __cplusplus >= 201103L
+namespace std {
+ template <typename T> // TODO: Implement the stub for deleter.
+ class unique_ptr {
+ public:
+ unique_ptr(const unique_ptr &) = delete;
+ unique_ptr(unique_ptr &&);
+
+ T *get() const;
+
+ typename std::add_lvalue_reference<T>::type operator*() const;
+ T *operator->() const;
+ };
+}
+#endif
+
#ifdef TEST_INLINABLE_ALLOCATORS
namespace std {
void *malloc(size_t);
Modified: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp?rev=349191&r1=349190&r2=349191&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp Fri Dec 14 12:52:57 2018
@@ -19,6 +19,6 @@ class C {
void testCopyNull(C *I, C *E) {
std::copy(I, E, (C *)0);
#ifndef SUPPRESSED
- // expected-warning at ../Inputs/system-header-simulator-cxx.h:670 {{Called C++ object pointer is null}}
+ // expected-warning at ../Inputs/system-header-simulator-cxx.h:677 {{Called C++ object pointer is null}}
#endif
}
Modified: cfe/trunk/test/Analysis/use-after-move.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/use-after-move.cpp?rev=349191&r1=349190&r2=349191&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/use-after-move.cpp (original)
+++ cfe/trunk/test/Analysis/use-after-move.cpp Fri Dec 14 12:52:57 2018
@@ -13,47 +13,7 @@
// RUN: -analyzer-config exploration_strategy=dfs -DDFS=1\
// RUN: -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
-namespace std {
-
-typedef __typeof(sizeof(int)) size_t;
-
-template <typename>
-struct remove_reference;
-
-template <typename _Tp>
-struct remove_reference { typedef _Tp type; };
-
-template <typename _Tp>
-struct remove_reference<_Tp &> { typedef _Tp type; };
-
-template <typename _Tp>
-struct remove_reference<_Tp &&> { typedef _Tp type; };
-
-template <typename _Tp>
-typename remove_reference<_Tp>::type &&move(_Tp &&__t) {
- return static_cast<typename remove_reference<_Tp>::type &&>(__t);
-}
-
-template <typename _Tp>
-_Tp &&forward(typename remove_reference<_Tp>::type &__t) noexcept {
- return static_cast<_Tp &&>(__t);
-}
-
-template <class T>
-void swap(T &a, T &b) {
- T c(std::move(a));
- a = std::move(b);
- b = std::move(c);
-}
-
-template <typename T>
-class vector {
-public:
- vector();
- void push_back(const T &t);
-};
-
-} // namespace std
+#include "Inputs/system-header-simulator-cxx.h"
class B {
public:
@@ -832,13 +792,26 @@ MoveOnlyWithDestructor foo() {
class HasSTLField {
std::vector<int> V;
- void foo() {
+ void testVector() {
// Warn even in non-aggressive mode when it comes to STL, because
// in STL the object is left in "valid but unspecified state" after move.
std::vector<int> W = std::move(V); // expected-note{{Object 'V' of type 'std::vector' is left in a valid but unspecified state after move}}
V.push_back(123); // expected-warning{{Method called on moved-from object 'V'}}
// expected-note at -1{{Method called on moved-from object 'V'}}
}
+
+ std::unique_ptr<int> P;
+ void testUniquePtr() {
+ // unique_ptr remains in a well-defined state after move.
+ std::unique_ptr<int> Q = std::move(P);
+ P.get();
+#ifdef AGGRESSIVE
+ // expected-warning at -2{{Method called on moved-from object 'P'}}
+ // expected-note at -4{{Object 'P' is moved}}
+ // expected-note at -4{{Method called on moved-from object 'P'}}
+#endif
+ *P += 1; // FIXME: Should warn that the pointer is null.
+ }
};
void localRValueMove(A &&a) {
@@ -846,3 +819,11 @@ void localRValueMove(A &&a) {
a.foo(); // expected-warning{{Method called on moved-from object 'a'}}
// expected-note at -1{{Method called on moved-from object 'a'}}
}
+
+void localUniquePtr(std::unique_ptr<int> P) {
+ // Even though unique_ptr is safe to use after move,
+ // reusing a local variable this way usually indicates a bug.
+ std::unique_ptr<int> Q = std::move(P); // expected-note{{Object 'P' is moved}}
+ P.get(); // expected-warning{{Method called on moved-from object 'P'}}
+ // expected-note at -1{{Method called on moved-from object 'P'}}
+}
More information about the cfe-commits
mailing list