[analyzer] MoveChecker Pt.8: Add checks for dereferencing a smart pointer after move.

[NoQ]

Summary

Calling operator*() or operator->() on a null STL smart pointer is undefined behavior.

Smart pointers are specified to become null after being moved from. So we can't warn on arbitrary method calls, but these two operators definitely make no sense.

The new bug is fatal, because it's UB unlike other use-after-move bugs.

Generally, we should also make a checker for dereferencing smart pointers that are known to be null, regardless of why they are null. Probably these checkers could interact somehow. But this case is kinda handy and is available to us almost for free.

[MTC]

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

89–90

Outdated TODO?

[xazax.hun]

Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning.

[NoQ]

Fxd!

[NoQ]

Fix typo.

[MTC]

In D55388#1322601, @xazax.hun wrote:

Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning.

Great idea!

IIRC, we haven't model smart-pointer yet. There are no warnings fired in the simple code below.

int foo() {
    auto ptr = std::make_unique<int>(0);
    ptr = nullptr;
    return *ptr;              // <--------- Should emit a warning here
}

If we want to model the get() method, not only the move action but also the assignment should be considered. std::unique_ptr may be fine, but is the reference count stuff too complex for std::shared_ptr to get the information available for analysis? Can the lifetime analysis cover the smart pointers?

int unique_ptr_bad() {
    auto ptr = std::make_unique<int>(0);
    ptr = nullptr;
    int *raw_p = ptr.get();   // <--------- 'raw_p' is definitely null here
    return *raw_p;              // <--------- Should emit a warning here
}

Related dev-mail about smart pointer checkers, see http://lists.llvm.org/pipermail/cfe-dev/2012-January/019446.html

[NoQ]

In D55388#1322601, @xazax.hun wrote:

Hm. I wonder if it would also make sense to model e.g. the get method to return nullptr for moved from smart ptrs. This could help null dereference checker and also aid false path prunning.

Apart from "yeah, totally, and we should also model the whole standard library", i'm also a bit uncomfy to rely on this generic checker for something as specific as evalCall(). This whole patch should probably be split out into a separate checker that shares state with the move checker but is dedicated to smart pointers specifically. I'm pushing for this before coming up with generic standard library modeling because i was asked to do this specific check. Not sure how much should i push it though. Null dereferences via .get() are definitely a tasty addition.

Another random thought: should we make a mechanism through which checkers could send events to each other? Imagine move checker notifying other checkers that an object is being moved, and checkers subscribed to such custom event could immediately react accordingly. Kinda checker-defined checker callbacks.

[NoQ]

Rebase.

[NoQ]

test/Analysis/use-after-move.cpp

239

Ugh. Why are these diagnostic pieces incorrect? We're not jumping to the end of the function yet. At the very least, we should evaluate the next A a;.

[NoQ]

Reverted in rC349233 - fails on a Windows buildbot (http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/2642/steps/annotate/logs/stdio).

Surprisingly, the next build is fine, and the build after it is broken again. Probably some UB.

******************** TEST 'Clang :: Analysis/use-after-move.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.EXE -cc1 -internal-isystem c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\8.0.0\include -nostdsysteminc -analyze -analyzer-constraints=range -analyzer-checker=alpha.cplusplus.Move -verify C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false  -analyzer-config exploration_strategy=unexplored_first_queue  -analyzer-checker debug.ExprInspection
: 'RUN: at line 5';   c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.EXE -cc1 -internal-isystem c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\8.0.0\include -nostdsysteminc -analyze -analyzer-constraints=range -analyzer-checker=alpha.cplusplus.Move -verify C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false  -analyzer-config exploration_strategy=dfs -DDFS=1  -analyzer-checker debug.ExprInspection
: 'RUN: at line 9';   c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.EXE -cc1 -internal-isystem c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\8.0.0\include -nostdsysteminc -analyze -analyzer-constraints=range -analyzer-checker=alpha.cplusplus.Move -verify C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false  -analyzer-config exploration_strategy=unexplored_first_queue  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE  -analyzer-checker debug.ExprInspection
: 'RUN: at line 14';   c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.EXE -cc1 -internal-isystem c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\8.0.0\include -nostdsysteminc -analyze -analyzer-constraints=range -analyzer-checker=alpha.cplusplus.Move -verify C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp  -std=c++11 -analyzer-output=text -analyzer-config eagerly-assume=false  -analyzer-config exploration_strategy=dfs -DDFS=1  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE  -analyzer-checker debug.ExprInspection
--
Exit Code: 1

Command Output (stdout):
--
$ ":" "RUN: at line 1"
$ "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\bin\clang.EXE" "-cc1" "-internal-isystem" "c:\b\slave\clang-x64-windows-msvc\build\build\stage1\lib\clang\8.0.0\include" "-nostdsysteminc" "-analyze" "-analyzer-constraints=range" "-analyzer-checker=alpha.cplusplus.Move" "-verify" "C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp" "-std=c++11" "-analyzer-output=text" "-analyzer-config" "eagerly-assume=false" "-analyzer-config" "exploration_strategy=unexplored_first_queue" "-analyzer-checker" "debug.ExprInspection"
# command stderr:
error: 'warning' diagnostics expected but not seen: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 823 (directive at C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp:825): Dereference of null smart pointer 'P' of type 'std::unique_ptr'

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 851: Dereference of null smart pointer 'P' of type 'std::unique_ptr'

error: 'warning' diagnostics seen but not expected: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 831: REACHABLE

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 851: Method called on moved-from object 'P'

error: 'note' diagnostics expected but not seen: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 812 (directive at C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp:826): Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 823 (directive at C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp:827): Dereference of null smart pointer 'P' of type 'std::unique_ptr'

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 844: Object 'P' is moved

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 850: Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 851 (directive at C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp:852): Dereference of null smart pointer 'P' of type 'std::unique_ptr'

error: 'note' diagnostics seen but not expected: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 831: REACHABLE

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 844: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 850: 

  File C:\b\slave\clang-x64-windows-msvc\build\llvm.src\tools\clang\test\Analysis\use-after-move.cpp Line 851: Method called on moved-from object 'P'

13 errors generated.


error: command failed with exit status: 1

lib/StaticAnalyzer/Checkers/MoveChecker.cpp

checkRegionChanges(ProgramStateRef State,
                      const InvalidatedSymbols *Invalidated,
                      ArrayRef<const MemRegion *> RequestedRegions,
                      ArrayRef<const MemRegion *> InvalidatedRegions,
                      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, MK_Dereference };
+  enum StdObjectKind : unsigned { SK_NonStd, SK_Unsafe, SK_Safe, SK_SmartPtr };
+
+  static bool misuseCausesCrash(MisuseKind MK) {
+    return MK == MK_Dereference;
+  }
 
   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?
+    // Is this a local variable or a local rvalue reference?
+    bool IsLocal : 1;
+    // Is this an STL object? If so, of what kind?
+    StdObjectKind StdKind : 2;
+  };
+
+  // STL smart pointers are automatically re-initialized to null when moved
+  // from. So we can't warn on many methods, but we can warn when it is
+  // dereferenced, which is UB even if the resulting lvalue never gets read.
+  const llvm::StringSet<> StdSmartPtrClasses = {
+      "shared_ptr",
+      "unique_ptr",
+      "weak_ptr",
   };
 
   // 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 = {
+  // TODO: We can still try to identify *unsafe* use after move,
+  // like we did with smart pointers.
+  const llvm::StringSet<> StdSafeClasses = {
       "basic_filebuf",
       "basic_ios",
       "future",
       "optional",
       "packaged_task"
       "promise",
       "shared_future",
       "shared_lock",
-      "shared_ptr",
       "thread",
-      "unique_ptr",
       "unique_lock",
-      "weak_ptr",
   };
 
   // Should we bother tracking the state of the object?
   bool shouldBeTracked(ObjectKind OK) const {
     // 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.
-    return IsAggressive || OK.Local || OK.STL;
+    // predict precisely when use-after-move is invalid.
+    // Some STL objects are known to conform to additional contracts after move,
+    // so they are not tracked. However, smart pointers specifically are tracked
+    // because we can perform extra checking over them.
+    // 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.
+    return IsAggressive || OK.IsLocal
+                        || OK.StdKind == SK_Unsafe || OK.StdKind == SK_SmartPtr;
+  }
+
+  // Some objects only suffer from some kinds of misuses, but we need to track
+  // them anyway because we cannot know in advance what misuse will we find.
+  bool shouldWarnAbout(ObjectKind OK, MisuseKind MK) const {
+    // Additionally, only warn on smart pointers when they are dereferenced (or
+    // local or we are aggressive).
+    return shouldBeTracked(OK) &&
+           (IsAggressive || OK.IsLocal
+                         || OK.StdKind != SK_SmartPtr || MK == MK_Dereference);
   }
 
   // Obtains ObjectKind of an object. Because class declaration cannot always
   // be easily obtained from the memory region, it is supplied separately.
   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.
-  ObjectKind explainObject(llvm::raw_ostream &OS,
-                           const MemRegion *MR, const CXXRecordDecl *RD) const;
+  // the stream.
+  void explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
+                     const CXXRecordDecl *RD, MisuseKind MK) const;
 
-  bool isStandardMoveSafeClass(const CXXRecordDecl *RD) const;
+  bool belongsTo(const CXXRecordDecl *RD, const llvm::StringSet<> &Set) const;
 
   class MovedBugVisitor : public BugReporterVisitor {
   public:
-    MovedBugVisitor(const MoveChecker &Chk,
-                    const MemRegion *R, const CXXRecordDecl *RD)
-        : Chk(Chk), Region(R), RD(RD), Found(false) {}
+    MovedBugVisitor(const MoveChecker &Chk, const MemRegion *R,
+                    const CXXRecordDecl *RD, MisuseKind MK)
+        : Chk(Chk), Region(R), RD(RD), MK(MK), 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<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
                                                    BugReporterContext &BRC,
                                                    BugReport &BR) override;
 
   private:
     const MoveChecker &Chk;
     // The tracked region.
     const MemRegion *Region;
     // The class of the tracked object.
     const CXXRecordDecl *RD;
+    // How exactly the object was misused.
+    const MisuseKind MK;
     bool Found;
   };
 
   bool IsAggressive = false;
 
 public:
   void setAggressiveness(bool Aggressive) { IsAggressive = Aggressive; }
 
MoveChecker::MovedBugVisitor::VisitNode(const ExplodedNode *N,
   const Stmt *S = PathDiagnosticLocation::getStmt(N);
   if (!S)
     return nullptr;
   Found = true;
 
   SmallString<128> Str;
   llvm::raw_svector_ostream OS(Str);
 
+  ObjectKind OK = Chk.classifyObject(Region, RD);
+  switch (OK.StdKind) {
+    case SK_SmartPtr:
+      if (MK == MK_Dereference) {
+        OS << "Smart pointer";
+        Chk.explainObject(OS, Region, RD, MK);
+        OS << " is reset to null when moved from";
+        break;
+      }
+
+      // If it's not a dereference, we don't care if it was reset to null
+      // or that it is even a smart pointer.
+      LLVM_FALLTHROUGH;
+    case SK_NonStd:
+    case SK_Safe:
       OS << "Object";
-  ObjectKind OK = Chk.explainObject(OS, Region, RD);
-  if (OK.STL)
-    OS << " is left in a valid but unspecified state after move";
-  else
+      Chk.explainObject(OS, Region, RD, MK);
       OS << " is moved";
+      break;
+    case SK_Unsafe:
+      OS << "Object";
+      Chk.explainObject(OS, Region, RD, MK);
+      OS << " is left in a valid but unspecified state after move";
+      break;
+  }
 
   // Generate the extra diagnostic.
   PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
                              N->getLocationContext());
   return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true);
-  }
 
 const ExplodedNode *MoveChecker::getMoveLocation(const ExplodedNode *N,
                                                  const MemRegion *Region,
                                                  CheckerContext &C) const {
   // Walk the ExplodedGraph backwards and find the first node that referred to
   // the tracked region.
   const ExplodedNode *MoveNode = N;
 
   while (N) {
     ProgramStateRef State = N->getState();
     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,
                            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)
+    MK = MK_FunCall;
 
-  if (!RS || isAnyBaseRegionReported(State, Region)
+  if (!RS || !shouldWarnAbout(OK, MK)
           || isInMoveSafeContext(C.getLocationContext())) {
     // Finalize changes made by the caller.
     C.addTransition(State);
     return;
   }
 
+  // Don't report it in case if any base region is already reported.
+  // But still generate a sink in case of UB.
+  // And still finalize changes made by the caller.
+  if (isAnyBaseRegionReported(State, Region)) {
+    if (misuseCausesCrash(MK)) {
+      C.generateSink(State, C.getPredecessor());
+    } else {
+      C.addTransition(State);
+    }
+    return;
+  }
+
   ExplodedNode *N = reportBug(Region, 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,
-                                     const CXXRecordDecl *RD,
-                                     CheckerContext &C,
+                                     const CXXRecordDecl *RD, CheckerContext &C,
                                      MisuseKind MK) const {
-  if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+  if (ExplodedNode *N = misuseCausesCrash(MK) ? C.generateErrorNode()
+                                              : C.generateNonFatalErrorNode()) {
+
     if (!BT)
       BT.reset(new BugType(this, "Use-after-move",
                            "C++ move semantics"));
 
     // Uniqueing report to the same object.
     PathDiagnosticLocation LocUsedForUniqueing;
     const ExplodedNode *MoveNode = getMoveLocation(N, Region, C);
 
     if (const Stmt *MoveStmt = PathDiagnosticLocation::getStmt(MoveNode))
       LocUsedForUniqueing = PathDiagnosticLocation::createBegin(
           MoveStmt, C.getSourceManager(), MoveNode->getLocationContext());
 
     // Creating the error message.
     llvm::SmallString<128> Str;
     llvm::raw_svector_ostream OS(Str);
     switch(MK) {
       case MK_FunCall:
         OS << "Method called on moved-from object";
-        explainObject(OS, Region, RD);
+        explainObject(OS, Region, RD, MK);
         break;
       case MK_Copy:
         OS << "Moved-from object";
-        explainObject(OS, Region, RD);
+        explainObject(OS, Region, RD, MK);
         OS << " is copied";
         break;
       case MK_Move:
         OS << "Moved-from object";
-        explainObject(OS, Region, RD);
+        explainObject(OS, Region, RD, MK);
         OS << " is moved";
         break;
+      case MK_Dereference:
+        OS << "Dereference of null smart pointer";
+        explainObject(OS, Region, RD, MK);
+        break;
     }
 
     auto R =
         llvm::make_unique<BugReport>(*BT, OS.str(), N, LocUsedForUniqueing,
                                      MoveNode->getLocationContext()->getDecl());
-    R->addVisitor(llvm::make_unique<MovedBugVisitor>(*this, Region, RD));
+    R->addVisitor(llvm::make_unique<MovedBugVisitor>(*this, Region, RD, MK));
     C.emitReport(std::move(R));
     return N;
   }
   return nullptr;
 }
 
 void MoveChecker::checkPostCall(const CallEvent &Call,
                                 CheckerContext &C) const {
if (DtorDec || (CtorDec && CtorDec->isCopyOrMoveConstructor()) ||
         (MethodDec && MethodDec->isOverloadedOperator() &&
          MethodDec->getOverloadedOperator() == OO_Equal) ||
         isStateResetMethod(MethodDec) || isMoveSafeMethod(MethodDec))
       return true;
   } while ((LC = LC->getParent()));
   return false;
 }
 
-bool MoveChecker::isStandardMoveSafeClass(const CXXRecordDecl *RD) const {
+bool MoveChecker::belongsTo(const CXXRecordDecl *RD,
+                            const llvm::StringSet<> &Set) const {
   const IdentifierInfo *II = RD->getIdentifier();
-  return II && StandardMoveSafeClasses.count(II->getName());
+  return II && Set.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() &&
-        !isStandardMoveSafeClass(RD)
-  };
+  bool IsLocal =
+      MR && isa<VarRegion>(MR) && isa<StackSpaceRegion>(MR->getMemorySpace());
+
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
+    return { IsLocal, SK_NonStd };
+
+  if (belongsTo(RD, StdSmartPtrClasses))
+    return { IsLocal, SK_SmartPtr };
+
+  if (belongsTo(RD, StdSafeClasses))
+    return { IsLocal, SK_Safe };
+
+  return { IsLocal, SK_Unsafe };
 }
 
-MoveChecker::ObjectKind
-MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
-                           const CXXRecordDecl *RD) const {
+void MoveChecker::explainObject(llvm::raw_ostream &OS, const MemRegion *MR,
+                                const CXXRecordDecl *RD, MisuseKind MK) 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 =
           dyn_cast_or_null<DeclRegion>(unwrapRValueReferenceIndirection(MR))) {
     const auto *RegionDecl = cast<NamedDecl>(DR->getDecl());
     OS << " '" << RegionDecl->getNameAsString() << "'";
   }
+
   ObjectKind OK = classifyObject(MR, RD);
-  if (OK.STL) {
+  switch (OK.StdKind) {
+    case SK_NonStd:
+    case SK_Safe:
+      break;
+    case SK_SmartPtr:
+      if (MK != MK_Dereference)
+        break;
+
+      // We only care about the type if it's a dereference.
+      LLVM_FALLTHROUGH;
+    case SK_Unsafe:
       OS << " of type '" << RD->getQualifiedNameAsString() << "'";
-  }
-  return OK;
+      break;
+  };
 }
 
 void MoveChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   // Remove the MemRegions from the map on which a ctor/dtor call or assignment
   // happened.
 
if (OOK == OO_Equal) {
         MisuseKind MK =
             MethodDecl->isMoveAssignmentOperator() ? MK_Move : MK_Copy;
         modelUse(State, ArgRegion, RD, MK, C);
         return;
       }
       C.addTransition(State);
       return;
     }
+
+    if (OOK == OO_Star || OOK == OO_Arrow) {
+      modelUse(State, ThisRegion, RD, MK_Dereference, C);
+      return;
+    }
   }
 
   modelUse(State, ThisRegion, RD, MK_FunCall, C);
 }
 
 void MoveChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                    CheckerContext &C) const {
   ProgramStateRef State = C.getState();

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:  -analyzer-config exploration_strategy=unexplored_first_queue\
+// RUN:  -analyzer-checker debug.ExprInspection
 // 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 exploration_strategy=dfs -DDFS=1\
+// RUN:  -analyzer-checker debug.ExprInspection
 // 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:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 // 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
+// RUN:  -analyzer-config alpha.cplusplus.Move:Aggressive=true -DAGGRESSIVE\
+// RUN:  -analyzer-checker debug.ExprInspection
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void clang_analyzer_warnIfReached();
+
 class B {
 public:
   B() = default;
   B(const B &) = default;
   B(B &&) = default;
   B& operator=(const B &q) = default;
   void operator=(B &&b) {
     return;
void decltypeIsNotUseTest() {
   A a;
   // A b(std::move(a));
   decltype(a) other_a; // no-warning
 }
 
 void loopTest() {
   {
     A a;
     for (int i = 0; i < bignum(); i++) { // expected-note {{Loop condition is false. Execution jumps to the end of the function}}
       rightRefCall(std::move(a));        // no-warning
     }
   }
   {
     A a;
     for (int i = 0; i < 2; i++) { // expected-note {{Loop condition is true.  Entering loop body}}
       //expected-note@-1 {{Loop condition is true.  Entering loop body}}
       //expected-note@-2 {{Loop condition is false. Execution jumps to the end of the function}}
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@-2{{Method called on moved-from object 'P'}}
     // expected-note@-4{{Object 'P' is moved}}
     // expected-note@-4{{Method called on moved-from object 'P'}}
 #endif
-    *P += 1; // FIXME: Should warn that the pointer is null.
+
+    // Because that well-defined state is null, dereference is still UB.
+    // Note that in aggressive mode we already warned about 'P',
+    // so no extra warning is generated.
+    *P += 1;
+#ifndef AGGRESSIVE
+    // expected-warning@-2{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+    // expected-note@-14{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+    // expected-note@-4{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+#endif
+
+    // The program should have crashed by now.
+    clang_analyzer_warnIfReached(); // no-warning
   }
 };
 
 void localRValueMove(A &&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@-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@-1{{Method called on moved-from object 'P'}}
 }
+
+void localUniquePtrWithArrow(std::unique_ptr<A> P) {
+  std::unique_ptr<A> Q = std::move(P); // expected-note{{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
+  P->foo(); // expected-warning{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+            // expected-note@-1{{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
+}