Index: lib/StaticAnalyzer/Checkers/MoveChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -66,17 +66,29 @@ bool STL : 1; // Is this an object of a standard type? }; + // 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); + // 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); + class MovedBugVisitor : public BugReporterVisitor { public: - MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {} + MovedBugVisitor(const MemRegion *R, const CXXRecordDecl *RD) + : Region(R), RD(RD), Found(false) {} void Profile(llvm::FoldingSetNodeID &ID) const override { static int X = 0; ID.AddPointer(&X); ID.AddPointer(Region); + // Don't add RD because it's, in theory, uniquely determined by + // the region. In practice though, it's not always possible to obtain + // the declaration directly from the region, that's why we store it + // in the first place. } std::shared_ptr VisitNode(const ExplodedNode *N, @@ -86,6 +98,8 @@ private: // The tracked region. const MemRegion *Region; + // The class of the tracked object. + const CXXRecordDecl *RD; bool Found; }; @@ -163,23 +177,21 @@ return nullptr; Found = true; - std::string ObjectName; - if (const auto DecReg = - unwrapRValueReferenceIndirection(Region)->getAs()) { - const auto *RegionDecl = dyn_cast(DecReg->getDecl()); - ObjectName = RegionDecl->getNameAsString(); - } - std::string InfoText; - if (ObjectName != "") - InfoText = "'" + ObjectName + "' became 'moved-from' here"; + SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + + OS << "Object"; + ObjectKind OK = explainObject(OS, Region, RD); + if (OK.STL) + OS << " is left in a valid but unspecified state after move"; else - InfoText = "Became 'moved-from' here"; + OS << " is moved"; // Generate the extra diagnostic. PathDiagnosticLocation Pos(S, BRC.getSourceManager(), N->getLocationContext()); - return std::make_shared(Pos, InfoText, true); -} + return std::make_shared(Pos, OS.str(), true); + } const ExplodedNode *MoveChecker::getMoveLocation(const ExplodedNode *N, const MemRegion *Region, @@ -204,7 +216,7 @@ MisuseKind MK) const { if (ExplodedNode *N = C.generateNonFatalErrorNode()) { if (!BT) - BT.reset(new BugType(this, "Usage of a 'moved-from' object", + BT.reset(new BugType(this, "Use-after-move", "C++ move semantics")); // Uniqueing report to the same object. @@ -216,27 +228,29 @@ MoveStmt, C.getSourceManager(), MoveNode->getLocationContext()); // Creating the error message. - std::string ErrorMessage; + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); switch(MK) { case MK_FunCall: - ErrorMessage = "Method call on a 'moved-from' object"; + OS << "Method called on moved-from object"; + explainObject(OS, Region, RD); break; case MK_Copy: - ErrorMessage = "Copying a 'moved-from' object"; + OS << "Moved-from object"; + explainObject(OS, Region, RD); + OS << " is copied"; break; case MK_Move: - ErrorMessage = "Moving a 'moved-from' object"; + OS << "Moved-from object"; + explainObject(OS, Region, RD); + OS << " is moved"; break; } - if (const auto DecReg = Region->getAs()) { - const auto *RegionDecl = dyn_cast(DecReg->getDecl()); - ErrorMessage += " '" + RegionDecl->getNameAsString() + "'"; - } auto R = - llvm::make_unique(*BT, ErrorMessage, N, LocUsedForUniqueing, + llvm::make_unique(*BT, OS.str(), N, LocUsedForUniqueing, MoveNode->getLocationContext()->getDecl()); - R->addVisitor(llvm::make_unique(Region)); + R->addVisitor(llvm::make_unique(Region, RD)); C.emitReport(std::move(R)); return N; } @@ -363,6 +377,23 @@ }; } +MoveChecker::ObjectKind MoveChecker::explainObject(llvm::raw_ostream &OS, + const MemRegion *MR, + const CXXRecordDecl *RD) { + // 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 = + dyn_cast_or_null(unwrapRValueReferenceIndirection(MR))) { + const auto *RegionDecl = cast(DR->getDecl()); + OS << " '" << RegionDecl->getNameAsString() << "'"; + } + ObjectKind OK = classifyObject(MR, RD); + if (OK.STL) { + OS << " of type '" << RD->getQualifiedNameAsString() << "'"; + } + return OK; +} + void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); const LocationContext *LC = C.getLocationContext(); Index: test/Analysis/use-after-move.cpp =================================================================== --- test/Analysis/use-after-move.cpp +++ test/Analysis/use-after-move.cpp @@ -88,7 +88,7 @@ A(const A &other) : i(other.i), d(other.d), b(other.b) {} A(A &&other) : i(other.i), d(other.d), b(std::move(other.b)) { #ifdef AGGRESSIVE - // expected-note@-2{{'b' became 'moved-from' here}} + // expected-note@-2{{Object 'b' is moved}} #endif } A(A &&other, char *k) { @@ -137,18 +137,18 @@ void simpleMoveCtorTest() { { A a; - A b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + A b = std::move(a); // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } { A a; - A b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - b = a; // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}} + A b = std::move(a); // expected-note {{Object 'a' is moved}} + b = a; // expected-warning {{Moved-from object 'a' is copied}} expected-note {{Moved-from object 'a' is copied}} } { A a; - A b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - b = std::move(a); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}} + A b = std::move(a); // expected-note {{Object 'a' is moved}} + b = std::move(a); // expected-warning {{Moved-from object 'a' is moved}} expected-note {{Moved-from object 'a' is moved}} } } @@ -156,20 +156,20 @@ { A a; A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + b = std::move(a); // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } { A a; A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - A c(a); // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}} + b = std::move(a); // expected-note {{Object 'a' is moved}} + A c(a); // expected-warning {{Moved-from object 'a' is copied}} expected-note {{Moved-from object 'a' is copied}} } { A a; A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - A c(std::move(a)); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}} + b = std::move(a); // expected-note {{Object 'a' is moved}} + A c(std::move(a)); // expected-warning {{Moved-from object 'a' is moved}} expected-note {{Moved-from object 'a' is moved}} } } @@ -178,8 +178,8 @@ A a; }; A a; - S s{std::move(a)}; // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + S s{std::move(a)}; // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } // Don't report a bug if the variable was assigned to in the meantime. @@ -227,19 +227,19 @@ A b; b = std::move(a); a = A(); - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + b = std::move(a); // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } // If a path exist where we not reinitialize the variable we report a bug. { A a; A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} + b = std::move(a); // expected-note {{Object 'a' is moved}} if (i < 10) { // expected-note {{Assuming 'i' is >= 10}} expected-note {{Taking false branch}} a = A(); } if (i > 5) { // expected-note {{Taking true branch}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } } } @@ -325,8 +325,8 @@ { A a; for (int i = 0; i < bignum(); i++) { // expected-note {{Loop condition is true. Entering loop body}} expected-note {{Loop condition is true. Entering loop body}} - constCopyOrMoveCall(std::move(a)); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}} - // expected-note@-1 {{'a' became 'moved-from' here}} + constCopyOrMoveCall(std::move(a)); // expected-warning {{Moved-from object 'a' is moved}} expected-note {{Moved-from object 'a' is moved}} + // expected-note@-1 {{Object 'a' is moved}} } } @@ -348,10 +348,10 @@ void uniqueTest(bool cond) { A a(42, 42.0); A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} + b = std::move(a); // expected-note {{Object 'a' is moved}} if (cond) { // expected-note {{Assuming 'cond' is not equal to 0}} expected-note {{Taking true branch}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } if (cond) { a.bar(); // no-warning @@ -362,8 +362,8 @@ void uniqueTest2() { A a; - A a1 = std::move(a); // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + A a1 = std::move(a); // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} A a2 = std::move(a); // no-warning a.foo(); // no-warning @@ -373,12 +373,12 @@ //even on moved-from objects. void moveSafeFunctionsTest() { A a; - A b = std::move(a); // expected-note {{'a' became 'moved-from' here}} + A b = std::move(a); // expected-note {{Object 'a' is moved}} a.empty(); // no-warning a.isEmpty(); // no-warning (void)a; // no-warning (bool)a; // expected-warning {{expression result unused}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } void moveStateResetFunctionsTest() { @@ -445,17 +445,17 @@ b = std::move(a); a.foo(); #ifdef AGGRESSIVE - // expected-note@-3{{'a' became 'moved-from' here}} - // expected-warning@-3 {{Method call on a 'moved-from' object 'a'}} - // expected-note@-4{{Method call on a 'moved-from' object 'a'}} + // expected-note@-3{{Object 'a' is moved}} + // expected-warning@-3 {{Method called on moved-from object 'a'}} + // expected-note@-4{{Method called on moved-from object 'a'}} #endif b = std::move(static_a); static_a.foo(); #ifdef AGGRESSIVE - // expected-note@-3{{'static_a' became 'moved-from' here}} - // expected-warning@-3{{Method call on a 'moved-from' object 'static_a'}} - // expected-note@-4{{Method call on a 'moved-from' object 'static_a'}} + // expected-note@-3{{Object 'static_a' is moved}} + // expected-warning@-3{{Method called on moved-from object 'static_a'}} + // expected-note@-4{{Method called on moved-from object 'static_a'}} #endif } }; @@ -466,26 +466,26 @@ Arr[2] = std::move(*Ptr); (*Ptr).foo(); #ifdef AGGRESSIVE - // expected-note@-3{{Became 'moved-from' here}} - // expected-warning@-3{{Method call on a 'moved-from' object}} - // expected-note@-4{{Method call on a 'moved-from' object}} + // expected-note@-3{{Object is moved}} + // expected-warning@-3{{Method called on moved-from object}} + // expected-note@-4{{Method called on moved-from object}} #endif Ptr = &Arr[1]; Arr[3] = std::move(Arr[1]); Ptr->foo(); #ifdef AGGRESSIVE - // expected-note@-3{{Became 'moved-from' here}} - // expected-warning@-3{{Method call on a 'moved-from' object}} - // expected-note@-4{{Method call on a 'moved-from' object}} + // expected-note@-3{{Object is moved}} + // expected-warning@-3{{Method called on moved-from object}} + // expected-note@-4{{Method called on moved-from object}} #endif Arr[3] = std::move(Arr[2]); Arr[2].foo(); #ifdef AGGRESSIVE - // expected-note@-3{{Became 'moved-from' here}} - // expected-warning@-3{{Method call on a 'moved-from' object}} - // expected-note@-4{{Method call on a 'moved-from' object}} + // expected-note@-3{{Object is moved}} + // expected-warning@-3{{Method called on moved-from object}} + // expected-note@-4{{Method called on moved-from object}} #endif Arr[2] = std::move(Arr[3]); // reinitialization @@ -545,9 +545,9 @@ A a, b; switch (i) { // expected-note {{Control jumps to 'case 1:'}} case 1: - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} + b = std::move(a); // expected-note {{Object 'a' is moved}} case 2: - a.foo(); // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object 'a'}} + a.foo(); // expected-warning {{Method called on moved-from object}} expected-note {{Method called on moved-from object 'a'}} break; } } @@ -562,13 +562,13 @@ } void interFunTest1(A &a) { - a.bar(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + a.bar(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} } void interFunTest2() { A a; A b; - b = std::move(a); // expected-note {{'a' became 'moved-from' here}} + b = std::move(a); // expected-note {{Object 'a' is moved}} interFunTest1(a); // expected-note {{Calling 'interFunTest1'}} } @@ -577,8 +577,8 @@ void paramEvaluateOrderTest() { A a; - foobar(std::move(a), a.getI()); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} - // expected-note@-1 {{'a' became 'moved-from' here}} + foobar(std::move(a), a.getI()); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} + // expected-note@-1 {{Object 'a' is moved}} //FALSE NEGATIVE since parameters evaluate order is undefined foobar(a.getI(), std::move(a)); //no-warning @@ -613,8 +613,8 @@ } { A a; - A a1 = std::move(a), a2 = a; // expected-warning {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}} - // expected-note@-1 {{'a' became 'moved-from' here}} + A a1 = std::move(a), a2 = a; // expected-warning {{Moved-from object 'a' is copied}} expected-note {{Moved-from object 'a' is copied}} + // expected-note@-1 {{Object 'a' is moved}} } } @@ -640,8 +640,8 @@ } { A a; - if (A(std::move(a)).foo() > 0 && a.foo() > 0) { // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} - // expected-note@-1 {{'a' became 'moved-from' here}} expected-note@-1 {{Assuming the condition is true}} expected-note@-1 {{Assuming the condition is false}} + if (A(std::move(a)).foo() > 0 && a.foo() > 0) { // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} + // expected-note@-1 {{Object 'a' is moved}} expected-note@-1 {{Assuming the condition is true}} expected-note@-1 {{Assuming the condition is false}} // expected-note@-2 {{Left side of '&&' is false}} expected-note@-2 {{Left side of '&&' is true}} // expected-note@-3 {{Taking false branch}} A().bar(); @@ -657,8 +657,8 @@ } { A a; - if (A(std::move(a)).foo() > 0 || a.foo() > 0) { // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} - // expected-note@-1 {{'a' became 'moved-from' here}} expected-note@-1 {{Assuming the condition is false}} expected-note@-1 {{Left side of '||' is false}} + if (A(std::move(a)).foo() > 0 || a.foo() > 0) { // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} + // expected-note@-1 {{Object 'a' is moved}} expected-note@-1 {{Assuming the condition is false}} expected-note@-1 {{Left side of '||' is false}} A().bar(); } } @@ -695,9 +695,9 @@ B b = std::move(a.b); a.b.foo(); #ifdef AGGRESSIVE - // expected-note@-3{{'b' became 'moved-from' here}} - // expected-warning@-3{{Method call on a 'moved-from' object 'b'}} - // expected-note@-4 {{Method call on a 'moved-from' object 'b'}} + // expected-note@-3{{Object 'b' is moved}} + // expected-warning@-3{{Method called on moved-from object 'b'}} + // expected-note@-4 {{Method called on moved-from object 'b'}} #endif } { @@ -707,21 +707,21 @@ #ifdef AGGRESSIVE // expected-note@-3{{Calling move constructor for 'A'}} // expected-note@-4{{Returning from move constructor for 'A'}} - // expected-warning@-4{{Method call on a 'moved-from' object 'b'}} - // expected-note@-5{{Method call on a 'moved-from' object 'b'}} + // expected-warning@-4{{Method called on moved-from object 'b'}} + // expected-note@-5{{Method called on moved-from object 'b'}} #endif } // Don't report a misuse if any SuperRegion is already reported. { A a; - A a1 = std::move(a); // expected-note {{'a' became 'moved-from' here}} - a.foo(); // expected-warning {{Method call on a 'moved-from' object 'a'}} expected-note {{Method call on a 'moved-from' object 'a'}} + A a1 = std::move(a); // expected-note {{Object 'a' is moved}} + a.foo(); // expected-warning {{Method called on moved-from object 'a'}} expected-note {{Method called on moved-from object 'a'}} a.b.foo(); // no-warning } { C c; - C c1 = std::move(c); // expected-note {{'c' became 'moved-from' here}} - c.foo(); // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}} + C c1 = std::move(c); // expected-note {{Object 'c' is moved}} + c.foo(); // expected-warning {{Method called on moved-from object 'c'}} expected-note {{Method called on moved-from object 'c'}} c.b.foo(); // no-warning } } @@ -742,8 +742,8 @@ void reportSuperClass() { C c; - C c1 = std::move(c); // expected-note {{'c' became 'moved-from' here}} - c.foo(); // expected-warning {{Method call on a 'moved-from' object 'c'}} expected-note {{Method call on a 'moved-from' object 'c'}} + C c1 = std::move(c); // expected-note {{Object 'c' is moved}} + c.foo(); // expected-warning {{Method called on moved-from object 'c'}} expected-note {{Method called on moved-from object 'c'}} C c2 = c; // no-warning } @@ -786,14 +786,14 @@ void foo() { // 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 W = std::move(V); // expected-note{{'V' became 'moved-from' here}} - V.push_back(123); // expected-warning{{Method call on a 'moved-from' object 'V'}} - // expected-note@-1{{Method call on a 'moved-from' object 'V'}} + std::vector 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@-1{{Method called on moved-from object 'V'}} } }; void localRValueMove(A &&a) { - A b = std::move(a); // expected-note{{'a' became 'moved-from' here}} - a.foo(); // expected-warning{{Method call on a 'moved-from' object}} - // expected-note@-1{{Method call on a 'moved-from' object}} + A b = std::move(a); // expected-note{{Object 'a' is moved}} + a.foo(); // expected-warning{{Method called on moved-from object 'a'}} + // expected-note@-1{{Method called on moved-from object 'a'}} }