[analyzer][draft] Explain where references are coming from in the use after move check

[martong]

LGTM, with nits.

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

434

This could be const couldn't it? Plus we should express that this is a pointer, I think.

435

clang/test/Analysis/use-after-move.cpp

799–800

I suppose these changes stem from D131262 coupled with trackExpressionValue, right? ...

1019–1025

... And this is the only new relevant test case.

[NoQ]

clang/test/Analysis/use-after-move.cpp

1019–1025

Hmmm. This example actually looks alright without an explanation of ref or a. Once the object is moved, it doesn't matter what its backstory was. Did you mean std::move(ref)? That would warrant a note about ref (but still, not necessarily about the initializer of a).

clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp

 private:
   BugType BT{this, "Use-after-move", categories::CXXMoveSemantics};
 
   // Check if the given form of potential misuse of a given object
   // should be reported. If so, get it reported. The callback from which
   // this function was called should immediately return after the call
   // because this function adds one or two transitions.
   void modelUse(ProgramStateRef State, const MemRegion *Region,
-                const CXXRecordDecl *RD, MisuseKind MK,
+                const Expr *ArgExpr, const CXXRecordDecl *RD, MisuseKind MK,
                 CheckerContext &C) const;
 
   // Returns the exploded node against which the report was emitted.
   // The caller *must* add any further transitions against this node.
-  ExplodedNode *reportBug(const MemRegion *Region, const CXXRecordDecl *RD,
-                          CheckerContext &C, MisuseKind MK) const;
+  ExplodedNode *reportBug(const MemRegion *Region, const Expr *ArgExpr,
+                          const CXXRecordDecl *RD, CheckerContext &C,
+                          MisuseKind MK) const;
 
   bool isInMoveSafeContext(const LocationContext *LC) const;
   bool isStateResetMethod(const CXXMethodDecl *MethodDec) const;
   bool isMoveSafeMethod(const CXXMethodDecl *MethodDec) const;
   const ExplodedNode *getMoveLocation(const ExplodedNode *N,
                                       const MemRegion *Region,
                                       CheckerContext &C) const;
 };
if (!State->get<TrackedRegionMap>(Region))
       break;
     MoveNode = N;
     N = N->pred_empty() ? nullptr : *(N->pred_begin());
   }
   return MoveNode;
 }
 
 void MoveChecker::modelUse(ProgramStateRef State, const MemRegion *Region,
-                           const CXXRecordDecl *RD, MisuseKind MK,
+                           const Expr* ArgExpr, const CXXRecordDecl *RD, MisuseKind MK,
                            CheckerContext &C) const {
   assert(!C.isDifferent() && "No transitions should have been made by now");
   const RegionState *RS = State->get<TrackedRegionMap>(Region);
   ObjectKind OK = classifyObject(Region, RD);
 
   // Just in case: if it's not a smart pointer but it does have operator *,
   // we shouldn't call the bug a dereference.
   if (MK == MK_Dereference && OK.StdKind != SK_SmartPtr)
if (isAnyBaseRegionReported(State, Region)) {
     if (misuseCausesCrash(MK)) {
       C.generateSink(State, C.getPredecessor());
     } else {
       C.addTransition(State);
     }
     return;
   }
 
-  ExplodedNode *N = reportBug(Region, RD, C, MK);
+  ExplodedNode *N = reportBug(Region, ArgExpr, RD, C, MK);
 
   // If the program has already crashed on this path, don't bother.
   if (N->isSink())
     return;
 
   State = State->set<TrackedRegionMap>(Region, RegionState::getReported());
   C.addTransition(State, N);
 }
 
-ExplodedNode *MoveChecker::reportBug(const MemRegion *Region,
+ExplodedNode *MoveChecker::reportBug(const MemRegion *Region, const Expr *Arg,
                                      const CXXRecordDecl *RD, CheckerContext &C,
                                      MisuseKind MK) const {
   if (ExplodedNode *N = misuseCausesCrash(MK) ? C.generateErrorNode()
                                               : C.generateNonFatalErrorNode()) {
     // Uniqueing report to the same object.
     PathDiagnosticLocation LocUsedForUniqueing;
     const ExplodedNode *MoveNode = getMoveLocation(N, Region, C);
 
switch(MK) {
         explainObject(OS, Region, RD, MK);
         break;
     }
 
     auto R = std::make_unique<PathSensitiveBugReport>(
         BT, OS.str(), N, LocUsedForUniqueing,
         MoveNode->getLocationContext()->getDecl());
     R->addVisitor(std::make_unique<MovedBugVisitor>(*this, Region, RD, MK));
+    if (Arg) {
+      Arg = Arg->IgnoreParenImpCasts();
+      if (auto ArgDeclRefExpr = dyn_cast<DeclRefExpr>(Arg)) {
+        if (auto VD = dyn_cast<VarDecl>(ArgDeclRefExpr->getDecl())) {
+          auto State = N->getState();
+          auto RawArgSval = State->getLValue(VD, N->getLocationContext());
+          if (RawArgSval.getAsRegion() != Region)
+            bugreporter::trackExpressionValue(N, Arg, *R);
+        }
+      } else
+        bugreporter::trackExpressionValue(N, Arg, *R);
+    }
     C.emitReport(std::move(R));
     return N;
   }
   return nullptr;
 }
 
 void MoveChecker::checkPostCall(const CallEvent &Call,
                                 CheckerContext &C) const {
void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
   if (const auto *CC = dyn_cast<CXXConstructorCall>(&Call)) {
     State = removeFromState(State, CC->getCXXThisVal().getAsRegion());
     auto CtorDec = CC->getDecl();
     // Check for copying a moved-from object and report the bug.
     if (CtorDec && CtorDec->isCopyOrMoveConstructor()) {
       const MemRegion *ArgRegion = CC->getArgSVal(0).getAsRegion();
       const CXXRecordDecl *RD = CtorDec->getParent();
       MisuseKind MK = CtorDec->isMoveConstructor() ? MK_Move : MK_Copy;
-      modelUse(State, ArgRegion, RD, MK, C);
+      modelUse(State, ArgRegion, CC->getArgExpr(0), RD, MK, C);
       return;
     }
   }
 
   const auto IC = dyn_cast<CXXInstanceCall>(&Call);
   if (!IC)
     return;
 
if (OOK == OO_Equal) {
       // only for move or copy assignment's argument.
       State = removeFromState(State, ThisRegion);
 
       if (MethodDecl->isCopyAssignmentOperator() ||
           MethodDecl->isMoveAssignmentOperator()) {
         const MemRegion *ArgRegion = IC->getArgSVal(0).getAsRegion();
         MisuseKind MK =
             MethodDecl->isMoveAssignmentOperator() ? MK_Move : MK_Copy;
-        modelUse(State, ArgRegion, RD, MK, C);
+        modelUse(State, ArgRegion, IC->getArgExpr(0), RD, MK, C);
         return;
       }
       C.addTransition(State);
       return;
     }
 
     if (OOK == OO_Star || OOK == OO_Arrow) {
-      modelUse(State, ThisRegion, RD, MK_Dereference, C);
+      modelUse(State, ThisRegion, IC->getCXXThisExpr(), RD, MK_Dereference, C);
       return;
     }
   }
 
-  modelUse(State, ThisRegion, RD, MK_FunCall, C);
+  modelUse(State, ThisRegion, IC->getCXXThisExpr(), RD, MK_FunCall, C);
 }
 
 void MoveChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   TrackedRegionMapTy TrackedRegions = State->get<TrackedRegionMap>();
   for (auto E : TrackedRegions) {
     const MemRegion *Region = E.first;

clang/test/Analysis/use-after-move.cpp

 
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
 void clang_analyzer_printState();
 
 class B {
 public:
-  B() = default;
+  B() = default;  // aggressive-note 2 {{Returning without writing to '*this'}}
   B(const B &) = default;
   B(B &&) = default;
   B& operator=(const B &q) = default;
   void operator=(B &&b) {
     return;
   }
   void foo() { return; }
 };
 
 class A {
   int i;
   double d;
 
 public:
   B b;
-  A(int ii = 42, double dd = 1.0) : d(dd), i(ii), b(B()) {}
+  A(int ii = 42, double dd = 1.0) : d(dd), i(ii), b(B()) {} // aggressive-note 2{{Calling defaulted default constructor for 'B'}}
+                                                            // aggressive-note@-1 2{{Returning from default constructor for 'B'}}
   void moveconstruct(A &&other) {
     std::swap(b, other.b);
     std::swap(d, other.d);
     std::swap(i, other.i);
     return;
   }
   static A get() {
     A v(12, 13);
#endif
   }
   {
     A a;
     A b = std::move(a); // peaceful-note {{Object 'a' is moved}}
     b = a; // peaceful-warning {{Moved-from object 'a' is copied}}
            // peaceful-note@-1 {{Moved-from object 'a' is copied}}
   }
   {
-    A a;
+    A a;                // peaceful-note {{'a' initialized here}}
     A b = std::move(a); // peaceful-note {{Object 'a' is moved}}
     b = std::move(a); // peaceful-warning {{Moved-from object 'a' is moved}}
                       // peaceful-note@-1 {{Moved-from object 'a' is moved}}
   }
 }
 
 void simpleMoveAssignementTest() {
   {
     A a;
     A b;
     b = std::move(a); // peaceful-note {{Object 'a' is moved}}
     a.foo(); // peaceful-warning {{Method called on moved-from object 'a'}}
              // peaceful-note@-1 {{Method called on moved-from object 'a'}}
   }
   {
     A a;
     A b;
     b = std::move(a); // peaceful-note {{Object 'a' is moved}}
     A c(a); // peaceful-warning {{Moved-from object 'a' is copied}}
             // peaceful-note@-1 {{Moved-from object 'a' is copied}}
   }
   {
-    A a;
+    A a;              // peaceful-note {{'a' initialized here}}
     A b;
     b = std::move(a); // peaceful-note {{Object 'a' is moved}}
     A c(std::move(a)); // peaceful-warning {{Moved-from object 'a' is moved}}
                        // peaceful-note@-1 {{Moved-from object 'a' is moved}}
   }
 }
 
 void moveInInitListTest() {
void loopTest() {
     A a;
     for (int i = 0; i < 2; i++) { // peaceful-note    {{Loop condition is true.  Entering loop body}}
                                   // peaceful-note@-1 {{Loop condition is true.  Entering loop body}}
                                   // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}}
       copyOrMoveCall(a);          // no-warning
     }
   }
   {
-    A a;
+    A a;                                 // peaceful-note {{'a' initialized here}}
     for (int i = 0; i < bignum(); i++) { // peaceful-note    {{Loop condition is true.  Entering loop body}}
                                          // peaceful-note@-1 {{Loop condition is true.  Entering loop body}}
       constCopyOrMoveCall(std::move(a)); // peaceful-note {{Object 'a' is moved}}
                                          // peaceful-warning@-1 {{Moved-from object 'a' is moved}}
                                          // peaceful-note@-2    {{Moved-from object 'a' is moved}}
     }
   }
 
void f() {
     b = std::move(static_a); // aggressive-note {{Object 'static_a' is moved}}
     static_a.foo(); // aggressive-warning {{Method called on moved-from object 'static_a'}}
                     // aggressive-note@-1 {{Method called on moved-from object 'static_a'}}
   }
 };
 
 void PtrAndArrayTest() {
   A *Ptr = new A(1, 1.5);
-  A Arr[10];
+  A Arr[10];                // aggressive-note{{'Arr' initialized here}}
   Arr[2] = std::move(*Ptr); // aggressive-note{{Object is moved}}
   (*Ptr).foo(); // aggressive-warning{{Method called on moved-from object}}
                 // aggressive-note@-1{{Method called on moved-from object}}
 
-  Ptr = &Arr[1];
+  Ptr = &Arr[1];              // aggressive-note {{Value assigned to 'Ptr'}}
   Arr[3] = std::move(Arr[1]); // aggressive-note {{Object is moved}}
   Ptr->foo(); // aggressive-warning {{Method called on moved-from object}}
               // aggressive-note@-1 {{Method called on moved-from object}}
 
   Arr[3] = std::move(Arr[2]); // aggressive-note{{Object is moved}}
   Arr[2].foo(); // aggressive-warning{{Method called on moved-from object}}
                 // aggressive-note@-1{{Method called on moved-from object}}
 

 }
 
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
            // peaceful-note@-1 {{Method called on moved-from object 'a'}}
 }
 
 void interFunTest2() {
-  A a;
+  A a;              // peaceful-note {{'a' initialized here}}
   A b;
   b = std::move(a); // peaceful-note {{Object 'a' is moved}}
-  interFunTest1(a); // peaceful-note {{Calling 'interFunTest1'}}
+  interFunTest1(a); // peaceful-note {{Calling 'interFunTest1'}} peaceful-note {{Passing value via 1st parameter 'a'}}
 }
 
 void foobar(A a, int i);
 void foobar(int i, A a);
 
 void paramEvaluateOrderTest() {
   A a;
   foobar(std::move(a), a.getI()); // peaceful-note {{Object 'a' is moved}}

 }
 
 struct C : public A {
   [[clang::reinitializes]] void reinit();
 };
 
 void subRegionMoveTest() {
   {
-    A a;
+    A a;                  // aggressive-note {{Calling default constructor for 'A'}}
+                          // aggressive-note@-1 {{Returning from default constructor for 'A'}}
     B b = std::move(a.b); // aggressive-note {{Object 'b' is moved}}
     a.b.foo(); // aggressive-warning {{Method called on moved-from object 'b'}}
                // aggressive-note@-1 {{Method called on moved-from object 'b'}}
   }
   {
-    A a;
+    A a;                 // aggressive-note {{Calling default constructor for 'A'}}
+                         // aggressive-note@-1 {{Returning from default constructor for 'A'}}
     A a1 = std::move(a); // aggressive-note {{Calling move constructor for 'A'}}
                          // aggressive-note@-1 {{Returning from move constructor for 'A'}}
     a.b.foo(); // aggressive-warning{{Method called on moved-from object 'b'}}
                // aggressive-note@-1{{Method called on moved-from object 'b'}}
   }
   // Don't report a misuse if any SuperRegion is already reported.
   {
     A a;
void test() {
     // is left in unspecified state after move.
     std::packaged_task<int(void)> Task2 = std::move(Task);
     // aggressive-note@-1   {{Object 'Task' is moved}}
     std::packaged_task<int(void)> Task3 = std::move(Task);
     // aggressive-warning@-1{{Moved-from object 'Task' is moved}}
     // aggressive-note@-2   {{Moved-from object 'Task' is moved}}
   }
 };
+
+void notes_for_references() {
+    A a;                // peaceful-note {{'a' initialized here}}
+    A& ref = a;         // peaceful-note {{'ref' initialized here}}
+    A b = std::move(a); // peaceful-note {{Object 'a' is moved}}
+    ref.foo();          // peaceful-warning {{Method called on moved-from object 'a'}}
+                        // peaceful-note@-1 {{Method called on moved-from object 'a'}}
+}