r316852 - [analyzer] MisusedMovedObjectChecker: More precise warning message

Peter Szecsi via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 28 16:24:00 PDT 2017


Author: szepet
Date: Sat Oct 28 16:24:00 2017
New Revision: 316852

URL: http://llvm.org/viewvc/llvm-project?rev=316852&view=rev
Log:
[analyzer] MisusedMovedObjectChecker: More precise warning message

Added new enum in order to differentiate the warning messages on "misusing" into
3 categories: function calls, moving an object, copying an object. (At the
moment the checker gives the same message in case of copying and moving.)

Additional test cases added as well.

Differential Revision: https://reviews.llvm.org/D38674


Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
    cfe/trunk/test/Analysis/MisusedMovedObject.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp?rev=316852&r1=316851&r2=316852&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MisusedMovedObjectChecker.cpp Sat Oct 28 16:24:00 2017
@@ -60,6 +60,7 @@ public:
                   const char *NL, const char *Sep) const override;
 
 private:
+  enum MisuseKind {MK_FunCall, MK_Copy, MK_Move};
   class MovedBugVisitor : public BugReporterVisitorImpl<MovedBugVisitor> {
   public:
     MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {}
@@ -83,7 +84,7 @@ private:
 
   mutable std::unique_ptr<BugType> BT;
   ExplodedNode *reportBug(const MemRegion *Region, const CallEvent &Call,
-                          CheckerContext &C, bool isCopy) const;
+                          CheckerContext &C, MisuseKind MK) const;
   bool isInMoveSafeContext(const LocationContext *LC) const;
   bool isStateResetMethod(const CXXMethodDecl *MethodDec) const;
   bool isMoveSafeMethod(const CXXMethodDecl *MethodDec) const;
@@ -179,7 +180,7 @@ const ExplodedNode *MisusedMovedObjectCh
 ExplodedNode *MisusedMovedObjectChecker::reportBug(const MemRegion *Region,
                                                    const CallEvent &Call,
                                                    CheckerContext &C,
-                                                   bool isCopy = false) const {
+                                                   MisuseKind MK) const {
   if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     if (!BT)
       BT.reset(new BugType(this, "Usage of a 'moved-from' object",
@@ -195,10 +196,17 @@ ExplodedNode *MisusedMovedObjectChecker:
 
     // Creating the error message.
     std::string ErrorMessage;
-    if (isCopy)
-      ErrorMessage = "Copying a 'moved-from' object";
-    else
-      ErrorMessage = "Method call on a 'moved-from' object";
+    switch(MK) {
+      case MK_FunCall:
+        ErrorMessage = "Method call on a 'moved-from' object";
+        break;
+      case MK_Copy:
+        ErrorMessage = "Copying a 'moved-from' object";
+        break;
+      case MK_Move:
+        ErrorMessage = "Moving a 'moved-from' object";
+        break;
+    }
     if (const auto DecReg = Region->getAs<DeclRegion>()) {
       const auto *RegionDecl = dyn_cast<NamedDecl>(DecReg->getDecl());
       ErrorMessage += " '" + RegionDecl->getNameAsString() + "'";
@@ -365,7 +373,10 @@ void MisusedMovedObjectChecker::checkPre
       const RegionState *ArgState = State->get<TrackedRegionMap>(ArgRegion);
       if (ArgState && ArgState->isMoved()) {
         if (!isInMoveSafeContext(LC)) {
-          N = reportBug(ArgRegion, Call, C, /*isCopy=*/true);
+          if(CtorDec->isMoveConstructor())
+            N = reportBug(ArgRegion, Call, C, MK_Move);
+          else
+            N = reportBug(ArgRegion, Call, C, MK_Copy);
           State = State->set<TrackedRegionMap>(ArgRegion,
                                                RegionState::getReported());
         }
@@ -405,7 +416,10 @@ void MisusedMovedObjectChecker::checkPre
           State->get<TrackedRegionMap>(IC->getArgSVal(0).getAsRegion());
       if (ArgState && ArgState->isMoved() && !isInMoveSafeContext(LC)) {
         const MemRegion *ArgRegion = IC->getArgSVal(0).getAsRegion();
-        N = reportBug(ArgRegion, Call, C, /*isCopy=*/true);
+        if(MethodDecl->isMoveAssignmentOperator())
+          N = reportBug(ArgRegion, Call, C, MK_Move);
+        else
+          N = reportBug(ArgRegion, Call, C, MK_Copy);
         State =
             State->set<TrackedRegionMap>(ArgRegion, RegionState::getReported());
       }
@@ -443,7 +457,7 @@ void MisusedMovedObjectChecker::checkPre
   if (isInMoveSafeContext(LC))
     return;
 
-  N = reportBug(ThisRegion, Call, C);
+  N = reportBug(ThisRegion, Call, C, MK_FunCall);
   State = State->set<TrackedRegionMap>(ThisRegion, RegionState::getReported());
   C.addTransition(State, N);
 }

Modified: cfe/trunk/test/Analysis/MisusedMovedObject.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/MisusedMovedObject.cpp?rev=316852&r1=316851&r2=316852&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/MisusedMovedObject.cpp (original)
+++ cfe/trunk/test/Analysis/MisusedMovedObject.cpp Sat Oct 28 16:24:00 2017
@@ -38,6 +38,7 @@ public:
   B() = default;
   B(const B &) = default;
   B(B &&) = default;
+  B& operator=(const B &q) = default;
   void operator=(B &&b) {
     return;
   }
@@ -70,6 +71,12 @@ public:
   A(A &&other, char *k) {
     moveconstruct(std::move(other));
   }
+  void operator=(const A &other) {
+    i = other.i;
+    d = other.d;
+    b = other.b;
+    return;
+  }
   void operator=(A &&other) {
     moveconstruct(std::move(other));
     return;
@@ -105,17 +112,42 @@ void copyOrMoveCall(A a) {
 }
 
 void simpleMoveCtorTest() {
-  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'}}
+  {
+    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 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 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'}}
+  }
 }
 
 void simpleMoveAssignementTest() {
-  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'}}
+  {
+    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'}}
+  }
+  {
+    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'}}
+  }
+  {
+    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'}}
+  }
 }
 
 void moveInInitListTest() {
@@ -270,7 +302,7 @@ void loopTest() {
   {
     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 {{Copying a 'moved-from' object 'a'}} expected-note {{Copying a 'moved-from' object 'a'}}
+      constCopyOrMoveCall(std::move(a)); // expected-warning {{Moving a 'moved-from' object 'a'}} expected-note {{Moving a 'moved-from' object 'a'}}
       // expected-note at -1 {{'a' became 'moved-from' here}}
     }
   }
@@ -447,7 +479,7 @@ void differentBranchesTest(int i) {
   // Same thing, but with a switch statement.
   {
     A a, b;
-    switch (i) { // expected-note {{Control jumps to 'case 1:'  at line 451}}
+    switch (i) { // expected-note {{Control jumps to 'case 1:'  at line 483}}
     case 1:
       b = std::move(a); // no-warning
       break;            // expected-note {{Execution jumps to the end of the function}}
@@ -459,7 +491,7 @@ void differentBranchesTest(int i) {
   // However, if there's a fallthrough, we do warn.
   {
     A a, b;
-    switch (i) { // expected-note {{Control jumps to 'case 1:'  at line 463}}
+    switch (i) { // expected-note {{Control jumps to 'case 1:'  at line 495}}
     case 1:
       b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
     case 2:




More information about the cfe-commits mailing list