[analyzer] MoveChecker Pt.2: Restrict the warning to STL objects and locals.

[NoQ]

Summary

This patch implements the post important part of the plan proposed in D54556. Under alpha.cplusplus.Move:Aggressive=true, the checker retains the original behavior, but when that option is equal to false (which is the proposed default behavior), the checker only warns in the following three cases:

1. If the object that's being used after move is of a known STL type (the word "known" here makes no sense; what i'm trying to say is that we'll rant about objects of any STL type, unless we encounter false positives even there and it'll cause us to restrict the check even further). The point here is that according to the C++ Standard, STL objects are known to remain in "valid but unspecified state", which means that the state is going to be internally consistent state which is not known. A lot of operations clearly make no sense (eg., asking for the value of the first character in a string that has just been moved makes little sense), and we know it and we can warn in these cases reliably. If some operations do make sense or, moreover, if some operations return the object into a valid state, we can easily hardcode these operations in the checker because STL has limited amount of stuff in it.
2. If the object is a local or parameter variable. This not only correlates with the true positives that i've seen so far, but also kinda makes sense, though, as usual, i'd love to be proven wrong. When an object is, say, a field in a class, or, even worse, a global, then even if you move from that object, the object is still there. It won't go anywhere; it *will* have to be re-used eventually. Which means that if the object is left in a well-specified state after getting moved, there's no good reason to enforce the developer to reset it manually. However, if the object is a local that is going to be cleaned up soon anyway, it is very unlikely that the desire to re-use its storage is well-justified. It should be totally fine to create another local and use that as a storage for the next object that you need. Hence the idea.
3. If the object is an rvalue reference. The idea is exactly the same as in 2: an xvalue is something that's near the end of its lifetime, so there's no temptation to re-use the storage. Whoever passed us that value is already fine with it being moved away, we have no obligation to fill in the space again.

[NoQ]

So, like, with locals it should be possible to easily and intuitively suppress the false positive, even if there is one, by simply using an extra variable, and additionally there's the [[clang::reinitializes]] attribute added by @xazax.hun that will help people suppress the warning when their object has a popular state-reset method that Static Analyzer doesn't understand. I believe this is a good trade-off between usefulness and usability, given that most positives with locals i've seen so far looked like real bugs.

[xazax.hun]

It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration options right now.

[Szelethus]

I think we should either remove the non-default functionality (which wouldn't be ideal), or emphasise somewhere (open projects?) that there is still work to be done, but leaving it to be forgotten and essentially making it an extra maintenance work would be, in my optinion, the worst case scenario. Aggressive isn't Pedantic because it actually emits warnings on correct code, and it's not a simple matter of too many reports being emitted, let's also document that this is an experimental feature, not a power-user-only thing.

Whenever I compile Rtags, I get a bunch of move related warnings, I'm curious how this patch behaves on that project. I'll take a look.

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

93–96

How about a simple public data member? Since all callbacks are const, it wouldn't be modified after registration anyways.

135

You use dyn_cast_or_null, which to me implies that the returned value may be nullptr, but you never check it on the call sites.

274–283

I've seen checker option descriptions in registry functions, in-class where they are declared, and on top of the file, I think any of them would be more ideal. Since my checker option related efforts are still far in the distance (which will put all of them in Checkers.td), let's move (or at least copy) this somewhere else.

test/Analysis/use-after-move.cpp

47–52

Any particular reason for not using cxx-system-header-simulator.h?

[Szelethus]

Alpha checkers, at least to developers, are clearly stating "Please finish me!", but a non-alpha, enabled-by-default checker with a flag that you'd never know about unless you looked at the source code (at least up until I fix these) sounds like it'll be forgotten like many other similar flags.

[NoQ]

In D54557#1299899, @Szelethus wrote:

Whenever I compile Rtags, I get a bunch of move related warnings, I'm curious how this patch behaves on that project. I'll take a look.

Whoops, sry, i accidentally had a look because i misread your message as if you wanted me to take a look >.<

Spoiler alert!

It finds one positive (and one duplicate of that one positive):

rtags-move-positive.html1 MBDownload

I believe this positive is a real bug, but we can still do better here. We find it as a use-after-move of a local variable, which is not a bug on its own, i.e. the user intended to empty and then re-use the storage and it's fine, this is not a usual kind of typo where the user uses the pre-move variable instead of the post-move variable. But the real bug here is that this local variable uses an std::string as a field under the hood, which is, well, not guaranteed to be empty after move, like all other STL objects: https://stackoverflow.com/questions/27376623/can-you-reuse-a-moved-stdstring

NOTE: As also mentioned in this stackoverflow thread, we also need to exclude smart pointers from the STL check because they don't end up in unspecified state, and see if there are other cornercases here.

Unfortunately, we don't find this bug as an STL use-after-move bug because inlining isn't happening. Why? I guess i'll leave it as an excercise ^.^ It's a combination of running out of budget and a specific feature that we have. By flipping a single -analyzer-config flag that represents that feature, we are able to find such bugs as STL bugs (when local variable bugs are disabled in the checker). We're still not able to find the original bug, most likely due to running out of budget (i didn't debug this further), but we can find it in a minimal example:

#include "rct/Rct.h"

void foo() {
  String S1;
  String S2 = std::move(S1);
  S1 += "asdfg"; // use-after-move of a std::string
}

Here's the report that we are able to obtain for this trivial code snippet, and you can look up the answer to the exercise in the collapsed run-line :)

rtags-move-positive-simplified.html162 KBDownload

[NoQ]

In D54557#1299899, @Szelethus wrote:

I think we should either remove the non-default functionality (which wouldn't be ideal), or emphasise somewhere (open projects?) that there is still work to be done, but leaving it to be forgotten and essentially making it an extra maintenance work would be, in my optinion, the worst case scenario. Aggressive isn't Pedantic because it actually emits warnings on correct code, and it's not a simple matter of too many reports being emitted, let's also document that this is an experimental feature, not a power-user-only thing.

I only kept the option around because i was under an impression that i'm intruding into a checker that already has some happy users, probably breaking existing workflows. If this option is unnecessary, i'd be happy to remove it :)

[NoQ]

In D54557#1299736, @xazax.hun wrote:

It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration options right now.

Do you envision room for another attribute here? I.e., a class attribute that says "this object is always unsafe to use after move, unless a method annotated with reinitializes is called"?

[Szelethus]

In D54557#1300581, @NoQ wrote:

In D54557#1299899, @Szelethus wrote:

Whenever I compile Rtags, I get a bunch of move related warnings, I'm curious how this patch behaves on that project. I'll take a look.

Whoops, sry, i accidentally had a look because i misread your message as if you wanted me to take a look >.<

Spoiler alert!

It finds one positive (and one duplicate of that one positive):

rtags-move-positive.html1 MBDownload

I believe this positive is a real bug, but we can still do better here. We find it as a use-after-move of a local variable, which is not a bug on its own, i.e. the user intended to empty and then re-use the storage and it's fine, this is not a usual kind of typo where the user uses the pre-move variable instead of the post-move variable. But the real bug here is that this local variable uses an std::string as a field under the hood, which is, well, not guaranteed to be empty after move, like all other STL objects: https://stackoverflow.com/questions/27376623/can-you-reuse-a-moved-stdstring

NOTE: As also mentioned in this stackoverflow thread, we also need to exclude smart pointers from the STL check because they don't end up in unspecified state, and see if there are other cornercases here.

OMG That is cool. :D That project was fishy. Nice catch, would you like to make an issue on their project or shall I?

Unfortunately, we don't find this bug as an STL use-after-move bug because inlining isn't happening. Why? I guess i'll leave it as an excercise ^.^ It's a combination of running out of budget and a specific feature that we have. By flipping a single -analyzer-config flag that represents that feature, we are able to find such bugs as STL bugs (when local variable bugs are disabled in the checker). We're still not able to find the original bug, most likely due to running out of budget (i didn't debug this further), but we can find it in a minimal example:

#include "rct/Rct.h"

void foo() {
  String S1;
  String S2 = std::move(S1);
  S1 += "asdfg"; // use-after-move of a std::string
}

Here's the report that we are able to obtain for this trivial code snippet, and you can look up the answer to the exercise in the collapsed run-line :)

rtags-move-positive-simplified.html162 KBDownload

Thanks! :) *throws confetti*

In D54557#1300653, @NoQ wrote:

In D54557#1299899, @Szelethus wrote:

I think we should either remove the non-default functionality (which wouldn't be ideal), or emphasise somewhere (open projects?) that there is still work to be done, but leaving it to be forgotten and essentially making it an extra maintenance work would be, in my optinion, the worst case scenario. Aggressive isn't Pedantic because it actually emits warnings on correct code, and it's not a simple matter of too many reports being emitted, let's also document that this is an experimental feature, not a power-user-only thing.

I only kept the option around because i was under an impression that i'm intruding into a checker that already has some happy users, probably breaking existing workflows. If this option is unnecessary, i'd be happy to remove it :)

Hmm, I'll ask around, but I'm not aware of any ongoing (or planned in the near future) work on this particular checker.

[Szelethus]

In D54557#1300653, @NoQ wrote:

In D54557#1299899, @Szelethus wrote:

I think we should either remove the non-default functionality (which wouldn't be ideal), or emphasise somewhere (open projects?) that there is still work to be done, but leaving it to be forgotten and essentially making it an extra maintenance work would be, in my optinion, the worst case scenario. Aggressive isn't Pedantic because it actually emits warnings on correct code, and it's not a simple matter of too many reports being emitted, let's also document that this is an experimental feature, not a power-user-only thing.

I only kept the option around because i was under an impression that i'm intruding into a checker that already has some happy users, probably breaking existing workflows. If this option is unnecessary, i'd be happy to remove it :)

Hmm, I'll ask around, but I'm not aware of any ongoing (or planned in the near future) work on this particular checker.

Yup, there seems to be a desire to keep it around. Let's add an entry to the open projects maybe?

[NoQ]

Give the rvalue reference test function a sensible name.

[NoQ]

In D54557#1300671, @Szelethus wrote:

Nice catch, would you like to make an issue on their project or shall I?

I guess it can be an outright pull request :) I'll see if i have time for that soon, please feel free to take initiative><

In D54557#1301829, @Szelethus wrote:

Yup, there seems to be a desire to keep it around. Let's add an entry to the open projects maybe?

Mmm, what is the project here? What changes are still necessary?

[Szelethus]

In D54557#1301869, @NoQ wrote:

In D54557#1300671, @Szelethus wrote:

Nice catch, would you like to make an issue on their project or shall I?

I guess it can be an outright pull request :) I'll see if i have time for that soon, please feel free to take initiative><

I'll do the honors then. :)

In D54557#1301829, @Szelethus wrote:

Yup, there seems to be a desire to keep it around. Let's add an entry to the open projects maybe?

Mmm, what is the project here? What changes are still necessary?

Wasn't the point of this patch to turn off part of this checkers functionality because it's immature just yet? From what I understand it is desired, but the FP rate is a little too high. I guess fixing that is the project.

Hmm, actually, tinkering with HTML files might be overkill, especially since sphinx will hopefully end that era. Let's just add a TODO and let me deal with it later when I reorganize the checker options. Sorry for talking nonsense :D

[xazax.hun]

In D54557#1300654, @NoQ wrote:

In D54557#1299736, @xazax.hun wrote:

It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration options right now.

Do you envision room for another attribute here? I.e., a class attribute that says "this object is always unsafe to use after move, unless a method annotated with reinitializes is called"?

Exactly :) My only concern is that I doubt users will end up passing such options using command line options. Having file base configuration options would be more convenient. They can be easily checked in the repository and evolve together with the product (like the .clang-tidy files in the LLVM repos).

[NoQ]

In D54557#1301896, @Szelethus wrote:

Wasn't the point of this patch to turn off part of this checkers functionality because it's immature just yet? From what I understand it is desired, but the FP rate is a little too high. I guess fixing that is the project.

Hmm, actually, tinkering with HTML files might be overkill, especially since sphinx will hopefully end that era. Let's just add a TODO and let me deal with it later when I reorganize the checker options. Sorry for talking nonsense :D

I would probably fine with shipping this other part as a lint check if we start shipping lint checks. I don't have any specific improvements in mind for that part. If somebody is willing to enforce "don't ever use things after move" policy in his code, this will be a good checker for such person. At the same time, i don't see how to improve that part so much that it can be enabled by default. I think projects that consist of "i believe it's impossible but you can try to come up with something" kind aren't very good for the open projects list because the person who will try to work on them may get disappointed when he realizes that his few at-a-glance solutions are bad and in fact nobody ever had any better solutions in mind.

In D54557#1302069, @xazax.hun wrote:

In D54557#1300654, @NoQ wrote:

In D54557#1299736, @xazax.hun wrote:

It would be great to have a way to extend the list of (possibly non-stl) types to check. But I do understand that the analyzer does not have a great way to set such configuration options right now.

Do you envision room for another attribute here? I.e., a class attribute that says "this object is always unsafe to use after move, unless a method annotated with reinitializes is called"?

Exactly :) My only concern is that I doubt users will end up passing such options using command line options. Having file base configuration options would be more convenient. They can be easily checked in the repository and evolve together with the product (like the .clang-tidy files in the LLVM repos).

I mean, attribute, like, annotation in the source code.

[Szelethus]

In D54557#1313382, @NoQ wrote:

In D54557#1301896, @Szelethus wrote:

Wasn't the point of this patch to turn off part of this checkers functionality because it's immature just yet? From what I understand it is desired, but the FP rate is a little too high. I guess fixing that is the project.

Hmm, actually, tinkering with HTML files might be overkill, especially since sphinx will hopefully end that era. Let's just add a TODO and let me deal with it later when I reorganize the checker options. Sorry for talking nonsense :D

I would probably fine with shipping this other part as a lint check if we start shipping lint checks. I don't have any specific improvements in mind for that part. If somebody is willing to enforce "don't ever use things after move" policy in his code, this will be a good checker for such person. At the same time, i don't see how to improve that part so much that it can be enabled by default. I think projects that consist of "i believe it's impossible but you can try to come up with something" kind aren't very good for the open projects list because the person who will try to work on them may get disappointed when he realizes that his few at-a-glance solutions are bad and in fact nobody ever had any better solutions in mind.

Yea, that makes sense.

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      const LocationContext *LCtx, const CallEvent *Call) const;
   void printState(raw_ostream &Out, ProgramStateRef State,
                   const char *NL, const char *Sep) const override;
 
 private:
-  enum MisuseKind {MK_FunCall, MK_Copy, MK_Move};
+  enum MisuseKind { MK_FunCall, MK_Copy, MK_Move };
+
+  struct ObjectKind {
+    bool Local : 1; // Is this a local variable or a local rvalue reference?
+    bool STL : 1; // Is this an object of a standard type?
+  };
+
+  static ObjectKind classifyObject(const MemRegion *MR,
+                                   const CXXRecordDecl *RD);
+
   class MovedBugVisitor : public BugReporterVisitor {
   public:
     MovedBugVisitor(const MemRegion *R) : Region(R), Found(false) {}
 
     void Profile(llvm::FoldingSetNodeID &ID) const override {
       static int X = 0;
       ID.AddPointer(&X);
       ID.AddPointer(Region);
     }
 
     std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
                                                    BugReporterContext &BRC,
                                                    BugReport &BR) override;
 
   private:
     // The tracked region.
     const MemRegion *Region;
     bool Found;
   };
 
+  bool IsAggressive = false;
+
+public:
+  void setAggressiveness(bool Aggressive) { IsAggressive = Aggressive; }
+
+private:
   mutable std::unique_ptr<BugType> BT;
-  ExplodedNode *reportBug(const MemRegion *Region, const CallEvent &Call,
+  ExplodedNode *reportBug(const MemRegion *Region, 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;
 };
static bool isAnyBaseRegionReported(ProgramStateRef State,
                                     const MemRegion *Region) {
   for (auto &E : State->get<TrackedRegionMap>()) {
     if (Region->isSubRegionOf(E.first) && E.second.isReported())
       return true;
   }
   return false;
 }
 
+static const MemRegion *unwrapRValueReferenceIndirection(const MemRegion *MR) {
+  if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(MR)) {
+    SymbolRef Sym = SR->getSymbol();
+    if (Sym->getType()->isRValueReferenceType())
+      if (const MemRegion *OriginMR = Sym->getOriginRegion())
+        return OriginMR;
+  }
+  return MR;
+}
+
 std::shared_ptr<PathDiagnosticPiece>
 MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
                                         BugReporterContext &BRC, BugReport &) {
   // We need only the last move of the reported object's region.
   // The visitor walks the ExplodedGraph backwards.
   if (Found)
     return nullptr;
   ProgramStateRef State = N->getState();
   ProgramStateRef StatePrev = N->getFirstPred()->getState();
   const RegionState *TrackedObject = State->get<TrackedRegionMap>(Region);
   const RegionState *TrackedObjectPrev =
       StatePrev->get<TrackedRegionMap>(Region);
   if (!TrackedObject)
     return nullptr;
   if (TrackedObjectPrev && TrackedObject)
     return nullptr;
 
   // Retrieve the associated statement.
   const Stmt *S = PathDiagnosticLocation::getStmt(N);
   if (!S)
     return nullptr;
   Found = true;
 
   std::string ObjectName;
-  if (const auto DecReg = Region->getAs<DeclRegion>()) {
+  if (const auto DecReg =
+          unwrapRValueReferenceIndirection(Region)->getAs<DeclRegion>()) {
     const auto *RegionDecl = dyn_cast<NamedDecl>(DecReg->getDecl());
     ObjectName = RegionDecl->getNameAsString();
   }
   std::string InfoText;
   if (ObjectName != "")
     InfoText = "'" + ObjectName + "' became 'moved-from' here";
   else
     InfoText = "Became 'moved-from' here";
if (!State->get<TrackedRegionMap>(Region))
       break;
     MoveNode = N;
     N = N->pred_empty() ? nullptr : *(N->pred_begin());
   }
   return MoveNode;
 }
 
 ExplodedNode *MoveChecker::reportBug(const MemRegion *Region,
-                                     const CallEvent &Call, CheckerContext &C,
+                                     const CXXRecordDecl *RD,
+                                     CheckerContext &C,
                                      MisuseKind MK) const {
   if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
     if (!BT)
       BT.reset(new BugType(this, "Usage of a 'moved-from' object",
                            "C++ move semantics"));
 
     // Uniqueing report to the same object.
     PathDiagnosticLocation LocUsedForUniqueing;
void MoveChecker::checkPostCall(const CallEvent &Call,
 
   if (!ConstructorDecl && !MethodDecl->isMoveAssignmentOperator())
     return;
 
   const auto ArgRegion = AFC->getArgSVal(0).getAsRegion();
   if (!ArgRegion)
     return;
 
+  // In non-aggressive mode, only warn on use-after-move of local variables (or
+  // local rvalue references) and of STL objects. The former is possible because
+  // local variables (or local rvalue references) are not tempting their user to
+  // re-use the storage. The latter is possible because STL objects are known
+  // to end up in a valid but unspecified state after the move and their
+  // state-reset methods are also known, which allows us to predict
+  // precisely when use-after-move is invalid.
+  // In aggressive mode, warn on any use-after-move because the user
+  // has intentionally asked us to completely eliminate use-after-move
+  // in his code.
+  ObjectKind OK = classifyObject(ArgRegion, MethodDecl->getParent());
+  if (!IsAggressive && !OK.Local && !OK.STL)
+    return;
+
   // Skip moving the object to itself.
   if (CC && CC->getCXXThisVal().getAsRegion() == ArgRegion)
     return;
   if (const auto *IC = dyn_cast<CXXInstanceCall>(AFC))
     if (IC->getCXXThisVal().getAsRegion() == ArgRegion)
       return;
 
   const MemRegion *BaseRegion = ArgRegion->getBaseRegion();
if (DtorDec || (CtorDec && CtorDec->isCopyOrMoveConstructor()) ||
         (MethodDec && MethodDec->isOverloadedOperator() &&
          MethodDec->getOverloadedOperator() == OO_Equal) ||
         isStateResetMethod(MethodDec) || isMoveSafeMethod(MethodDec))
       return true;
   } while ((LC = LC->getParent()));
   return false;
 }
 
+MoveChecker::ObjectKind MoveChecker::classifyObject(const MemRegion *MR,
+                                                    const CXXRecordDecl *RD) {
+  MR = unwrapRValueReferenceIndirection(MR);
+  return {
+    /*Local=*/
+        MR && isa<VarRegion>(MR) && isa<StackSpaceRegion>(MR->getMemorySpace()),
+    /*STL=*/
+        RD && RD->getDeclContext()->isStdNamespace()
+  };
+}
+
 void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   const LocationContext *LC = C.getLocationContext();
   ExplodedNode *N = nullptr;
 
   // Remove the MemRegions from the map on which a ctor/dtor call or assignment
   // happened.
 
   // Checking constructor calls.
   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 RegionState *ArgState = State->get<TrackedRegionMap>(ArgRegion);
       if (ArgState && ArgState->isMoved()) {
         if (!isInMoveSafeContext(LC)) {
+          const CXXRecordDecl *RD = CtorDec->getParent();
           if(CtorDec->isMoveConstructor())
-            N = reportBug(ArgRegion, Call, C, MK_Move);
+            N = reportBug(ArgRegion, RD, C, MK_Move);
           else
-            N = reportBug(ArgRegion, Call, C, MK_Copy);
+            N = reportBug(ArgRegion, RD, C, MK_Copy);
           State = State->set<TrackedRegionMap>(ArgRegion,
                                                RegionState::getReported());
         }
       }
     }
     C.addTransition(State, N);
     return;
   }
void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
   const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
   if (!ThisRegion)
     return;
 
   const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(IC->getDecl());
   if (!MethodDecl)
     return;
 
+  // Store class declaration as well, for bug reporting purposes.
+  const CXXRecordDecl *RD = MethodDecl->getParent();
+
   // Checking assignment operators.
   bool OperatorEq = MethodDecl->isOverloadedOperator() &&
                     MethodDecl->getOverloadedOperator() == OO_Equal;
   // Remove the tracked object for every assignment operator, but report bug
   // only for move or copy assignment's argument.
   if (OperatorEq) {
     State = removeFromState(State, ThisRegion);
     if (MethodDecl->isCopyAssignmentOperator() ||
         MethodDecl->isMoveAssignmentOperator()) {
       const RegionState *ArgState =
           State->get<TrackedRegionMap>(IC->getArgSVal(0).getAsRegion());
       if (ArgState && ArgState->isMoved() && !isInMoveSafeContext(LC)) {
         const MemRegion *ArgRegion = IC->getArgSVal(0).getAsRegion();
         if(MethodDecl->isMoveAssignmentOperator())
-          N = reportBug(ArgRegion, Call, C, MK_Move);
+          N = reportBug(ArgRegion, RD, C, MK_Move);
         else
-          N = reportBug(ArgRegion, Call, C, MK_Copy);
+          N = reportBug(ArgRegion, RD, C, MK_Copy);
         State =
             State->set<TrackedRegionMap>(ArgRegion, RegionState::getReported());
       }
     }
     C.addTransition(State, N);
     return;
   }
 
void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
 
   // Don't report it in case if any base region is already reported
   if (isAnyBaseRegionReported(State, ThisRegion))
     return;
 
   if (isInMoveSafeContext(LC))
     return;
 
-  N = reportBug(ThisRegion, Call, C, MK_FunCall);
+  N = reportBug(ThisRegion, RD, C, MK_FunCall);
   State = State->set<TrackedRegionMap>(ThisRegion, RegionState::getReported());
   C.addTransition(State, N);
 }
 
 void MoveChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();
   TrackedRegionMapTy TrackedRegions = State->get<TrackedRegionMap>();
for (auto I: RS) {
         Out << ": moved";
       else
         Out << ": moved and reported";
       Out << NL;
     }
   }
 }
 void ento::registerMoveChecker(CheckerManager &mgr) {
-  mgr.registerChecker<MoveChecker>();
+  MoveChecker *chk = mgr.registerChecker<MoveChecker>();
+  chk->setAggressiveness(mgr.getAnalyzerOptions().getCheckerBooleanOption(
+      "Aggressive", false, chk));
 }

test/Analysis/use-after-move.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=unexplored_first_queue
 // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
 // RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
 // RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
+// RUN:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.Move -verify %s\
+// RUN:  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false\
+// RUN:  -analyzer-config exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE
 
 namespace std {
 
 template <typename>
 struct remove_reference;
 
 template <typename _Tp>
 struct remove_reference { typedef _Tp type; };

 
 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
 
 class B {
 public:
   B() = default;
   B(const B &) = default;
   B(B &&) = default;
   B& operator=(const B &q) = default;
public:
   static A get() {
     A v(12, 13);
     return v;
   }
   A(A *a) {
     moveconstruct(std::move(*a));
   }
   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)) { // expected-note {{'b' became 'moved-from' here}}
+  A(A &&other) : i(other.i), d(other.d), b(std::move(other.b)) {
+#ifdef AGGRESSIVE
+    // expected-note@-2{{'b' became 'moved-from' here}}
+#endif
   }
   A(A &&other, char *k) {
     moveconstruct(std::move(other));
   }
   void operator=(const A &other) {
     i = other.i;
     d = other.d;
     b = other.b;

 
 // Moves of member variables.
 class memberVariablesTest {
   A a;
   static A static_a;
 
   void f() {
     A b;
-    b = std::move(a); // expected-note {{'a' became 'moved-from' here}}
-    a.foo();          // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object 'a'}}
+    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'}}
+#endif
 
-    b = std::move(static_a); // expected-note {{'static_a' became 'moved-from' here}}
-    static_a.foo();          // expected-warning {{Method call on a 'moved-from' object 'static_a'}} expected-note {{Method call on a 'moved-from' object 'static_a'}}
+    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'}}
+#endif
   }
 };
 
 void PtrAndArrayTest() {
   A *Ptr = new A(1, 1.5);
   A Arr[10];
-  Arr[2] = std::move(*Ptr); // expected-note {{Became 'moved-from' here}}
-  (*Ptr).foo();             // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object}}
+  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}}
+#endif
 
   Ptr = &Arr[1];
-  Arr[3] = std::move(Arr[1]); // expected-note {{Became 'moved-from' here}}
-  Ptr->foo();                 // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object}}
+  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}}
+#endif
 
-  Arr[3] = std::move(Arr[2]); // expected-note {{Became 'moved-from' here}}
-  Arr[2].foo();               // expected-warning {{Method call on a 'moved-from' object}} expected-note {{Method call on a 'moved-from' object}}
+  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}}
+#endif
 
   Arr[2] = std::move(Arr[3]); // reinitialization
   Arr[2].foo();               // no-warning
 }
 
 void exclusiveConditionsTest(bool cond) {
   A a;
   if (cond) {

 
 struct C : public A {
   [[clang::reinitializes]] void reinit();
 };
 
 void subRegionMoveTest() {
   {
     A a;
-    B b = std::move(a.b); // expected-note {{'b' became 'moved-from' here}}
-    a.b.foo();            // expected-warning {{Method call on a 'moved-from' object 'b'}} expected-note {{Method call on a 'moved-from' object 'b'}}
+    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'}}
+#endif
   }
   {
     A a;
-    A a1 = std::move(a); // expected-note {{Calling move constructor for 'A'}} expected-note {{Returning from move constructor for 'A'}}
-    a.b.foo();           // expected-warning {{Method call on a 'moved-from' object 'b'}} expected-note {{Method call on a 'moved-from' object 'b'}}
+    A a1 = std::move(a);
+    a.b.foo();
+#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'}}
+#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.b.foo();           // no-warning
   }
struct MoveOnlyWithDestructor {
   MoveOnlyWithDestructor(const MoveOnlyWithDestructor &m) = delete;
   MoveOnlyWithDestructor(MoveOnlyWithDestructor &&m);
 };
 
 MoveOnlyWithDestructor foo() {
   MoveOnlyWithDestructor m;
   return m;
 }
+
+class HasSTLField {
+  std::vector<int> V;
+  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<int> 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'}}
+  }
+};
+
+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}}
+}