[analyzer] Add modeling for unique_ptr move constructor

[vrnithinkumar]

Summary

• Modeling unique_ptr move constructor unique_ptr( unique_ptr&& u ) noexcept

[vrnithinkumar]

clang/test/Analysis/smart-ptr.cpp

296

I was trying to test the below code.

void foo_() {
  std::unique_ptr<A> PToMove;
  std::unique_ptr<A>&& AfterRValeRefCast = std::move(functionReturnsRValueRef());
  std::unique_ptr<A> P(AfterRValeRefCast);
  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
}

But passing the local RValue reference variable to move constructor is always invoking the deleted copy constructor.

[NoQ]

This is easier than D86293 because there's no old value in the freshly constructed smart pointer, right? I suspect that you can still re-use a lot of the implementation with D86293, at least partially.

clang/test/Analysis/smart-ptr.cpp

296

Yeah, you still need an explicit move over it. Implicit moves don't happen because it's an rvalue reference, it has more to do with the anonymity of the object; the compiler only auto-moves when there's no possibility for accidental implicit use-after-move, and in presence of a named reference to the value, accidental implicit use-after-move is still very much possible, so an explicit move is required. That's not the exact rules but that's, as far as i understand, the logic behind the exact rules.

[vrnithinkumar]

• Refactoring to reuse common duplicated code

[vrnithinkumar]

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

456

Same note tag is used for move assignment and move constructor here.

clang/test/Analysis/smart-ptr.cpp

21

Now both "use after move" and "null pointer dereference" warnings are coming. I hope it will be only "null pointer dereference" after smart pointer modeling is complete

[vrnithinkumar]

clang/test/Analysis/smart-ptr.cpp

296

Okay
Thanks for the clarification.

[vrnithinkumar]

In D86373#2231605, @NoQ wrote:

This is easier than D86293 because there's no old value in the freshly constructed smart pointer, right? I suspect that you can still re-use a lot of the implementation with D86293, at least partially.

Refactored to reuse the implementation.

[NoQ]

Yup, that's more like it!~

clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

checkRegionChanges(ProgramStateRef State,
                      const LocationContext *LCtx, const CallEvent *Call) const;
 
 private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
   void handleRelease(const CallEvent &Call, CheckerContext &C) const;
   void handleSwap(const CallEvent &Call, CheckerContext &C) const;
   void handleGet(const CallEvent &Call, CheckerContext &C) const;
   bool handleAssignOp(const CallEvent &Call, CheckerContext &C) const;
+  bool handleMoveCtr(const CallEvent &Call, CheckerContext &C,
+                     const MemRegion *ThisRegion) const;
+  bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
+                                const MemRegion *OtherSmartPtrRegion) const;
 
   using SmartPtrMethodHandlerFn =
       void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
       {{"reset"}, &SmartPtrModeling::handleReset},
       {{"release"}, &SmartPtrModeling::handleRelease},
       {{"swap", 1}, &SmartPtrModeling::handleSwap},
       {{"get"}, &SmartPtrModeling::handleGet}};
C.addTransition(
                         C.getSValBuilder().makeZeroVal(Call.getResultType())));
     return true;
   }
 
   if (!ModelSmartPtrDereference)
     return false;
 
   if (const auto *CC = dyn_cast<CXXConstructorCall>(&Call)) {
-    if (CC->getDecl()->isCopyOrMoveConstructor())
+    if (CC->getDecl()->isCopyConstructor())
       return false;
 
     const MemRegion *ThisRegion = CC->getCXXThisVal().getAsRegion();
     if (!ThisRegion)
       return false;
 
+    if (CC->getDecl()->isMoveConstructor())
+      return handleMoveCtr(Call, C, ThisRegion);
+
     if (Call.getNumArgs() == 0) {
       auto NullVal = C.getSValBuilder().makeNull();
       State = State->set<TrackedRegionMap>(ThisRegion, NullVal);
 
       C.addTransition(
           State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
                                            llvm::raw_ostream &OS) {
             if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
C.addTransition(State, C.getNoteTag([ThisRegion](PathSensitiveBugReport &BR,
         return;
       OS << "Smart pointer ";
       ThisRegion->printPretty(OS);
       OS << " is assigned to null";
     }));
     return true;
   }
 
+  return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion);
+}
+
+bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C,
+                                     const MemRegion *ThisRegion) const {
+  const auto *OtherSmartPtrRegion = Call.getArgSVal(0).getAsRegion();
+  if (!OtherSmartPtrRegion)
+    return false;
+
+  return updateMovedSmartPointers(C, ThisRegion, OtherSmartPtrRegion);
+}
+
+bool SmartPtrModeling::updateMovedSmartPointers(
+    CheckerContext &C, const MemRegion *ThisRegion,
+    const MemRegion *OtherSmartPtrRegion) const {
+  ProgramStateRef State = C.getState();
   const auto *OtherInnerPtr = State->get<TrackedRegionMap>(OtherSmartPtrRegion);
   if (OtherInnerPtr) {
     State = State->set<TrackedRegionMap>(ThisRegion, *OtherInnerPtr);
     auto NullVal = C.getSValBuilder().makeNull();
     State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
     bool IsArgValNull = OtherInnerPtr->isZeroConstant();
 
     C.addTransition(
         State,
         C.getNoteTag([ThisRegion, OtherSmartPtrRegion, IsArgValNull](
                          PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
           if (&BR.getBugType() != smartptr::getNullDereferenceBugType())
             return;
           if (BR.isInteresting(OtherSmartPtrRegion)) {
             OS << "Smart pointer ";
             OtherSmartPtrRegion->printPretty(OS);
             OS << " is null after being moved to ";
             ThisRegion->printPretty(OS);
           }
           if (BR.isInteresting(ThisRegion) && IsArgValNull) {
-            OS << "Null pointer value move-assigned to ";
+            OS << "A null pointer value is moved to ";
             ThisRegion->printPretty(OS);
             BR.markInteresting(OtherSmartPtrRegion);
           }
         }));
     return true;
   } else {
     // In case we dont know anything about value we are moving from
     // remove the entry from map for which smart pointer got moved to.

clang/test/Analysis/smart-ptr-text-output.cpp

void derefOnMovedToNullPtr() {
   P = std::move(PToMove); // No note.
   P->foo(); // No warning.
 }
 
 void derefOnNullPtrGotMovedFromValidPtr() {
   std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
   // FIXME: above note should go away once we fix marking region not interested. 
   std::unique_ptr<A> PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
-  P = std::move(PToMove); // expected-note {{Null pointer value move-assigned to 'P'}}
+  P = std::move(PToMove); // expected-note {{A null pointer value is moved to 'P'}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P'}}
 }
 
 void derefOnMovedUnknownPtr(std::unique_ptr<A> PToMove) {
   std::unique_ptr<A> P;
   P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null after; previous value moved to 'P'}}
   PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}

 
 void derefOnAssignedZeroToNullSmartPtr() {
   std::unique_ptr<A> P(new A()); // expected-note {{Smart pointer 'P' is constructed}}
   // FIXME: above note should go away once we fix marking region not interested. 
   P = 0; // expected-note {{Smart pointer 'P' is assigned to null}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P'}}
 }
+
+void derefMoveConstructedWithNullPtr() {
+  std::unique_ptr<A> PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
+  std::unique_ptr<A> P(std::move(PToMove)); // expected-note {{A null pointer value is moved to 'P'}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void derefValidPtrMovedToConstruct() {
+  std::unique_ptr<A> PToMove(new A()); // expected-note {{Smart pointer 'PToMove' is constructed}}
+  // FIXME: above note should go away once we fix marking region not interested. 
+  std::unique_ptr<A> P(std::move(PToMove)); // expected-note {{Smart pointer 'PToMove' is null after being moved to 'P'}}
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
+}
+
+void derefNullPtrMovedToConstruct() {
+  std::unique_ptr<A> PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
+  // FIXME: above note should go away once we fix marking region not interested. 
+  std::unique_ptr<A> P(std::move(PToMove)); // expected-note {{Smart pointer 'PToMove' is null after being moved to 'P'}}
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
+}
+
+void derefUnknownPtrMovedToConstruct(std::unique_ptr<A> PToMove) {
+  std::unique_ptr<A> P(std::move(PToMove)); // expected-note {{Smart pointer 'PToMove' is null after; previous value moved to 'P'}}
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+  // expected-note@-1{{Dereference of null smart pointer 'PToMove'}}
+}

clang/test/Analysis/smart-ptr.cpp

 void derefAfterMove(std::unique_ptr<int> P) {
   std::unique_ptr<int> Q = std::move(P);
   if (Q)
     clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
   *Q.get() = 1; // no-warning
   if (P)
     clang_analyzer_warnIfReached(); // no-warning
   // TODO: Report a null dereference (instead).
-  *P.get() = 1; // expected-warning {{Method called on moved-from object 'P'}}
+  *P.get() = 1; // expected-warning {{Method called on moved-from object 'P' [cplusplus.Move]}}
+  // expected-warning@-1 {{Dereference of null pointer [core.NullDereference]}}
 }
 
 // Don't crash when attempting to model a call with unknown callee.
 namespace testUnknownCallee {
 struct S {
   void foo();
 };
 void bar(S *s, void (S::*func)(void)) {
void derefOnMovedToNullPtr() {
   std::unique_ptr<A> PToMove(new A());
   std::unique_ptr<A> P;
   P = std::move(PToMove); // No note.
   P->foo(); // No warning.
 }
 
 void derefOnNullPtrGotMovedFromValidPtr() {
   std::unique_ptr<A> P(new A());
   std::unique_ptr<A> PToMove;
   P = std::move(PToMove);
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefOnMovedFromUnknownPtr(std::unique_ptr<A> PToMove) {
   std::unique_ptr<A> P;
   P = std::move(PToMove);
   P->foo(); // No warning.

 
 std::unique_ptr<A> &&returnRValRefOfUniquePtr();
 
 void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
   std::unique_ptr<A> P(new A());
   P = returnRValRefOfUniquePtr();
   P->foo(); // No warning. 
 }
+
+void derefMoveConstructedWithValidPtr() {
+  std::unique_ptr<A> PToMove(new A());
+  std::unique_ptr<A> P(std::move(PToMove));
+  P->foo(); // No warning.
+}
+
+void derefMoveConstructedWithNullPtr() {
+  std::unique_ptr<A> PToMove;
+  std::unique_ptr<A> P(std::move(PToMove));
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefMoveConstructedWithUnknownPtr(std::unique_ptr<A> PToMove) {
+  std::unique_ptr<A> P(std::move(PToMove));
+  P->foo(); // No warning.
+}
+
+void derefValidPtrMovedToConstruct() {
+  std::unique_ptr<A> PToMove(new A());
+  std::unique_ptr<A> P(std::move(PToMove));
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefNullPtrMovedToConstruct() {
+  std::unique_ptr<A> PToMove;
+  std::unique_ptr<A> P(std::move(PToMove));
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefUnknownPtrMovedToConstruct(std::unique_ptr<A> PToMove) {
+  std::unique_ptr<A> P(std::move(PToMove));
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+std::unique_ptr<A> &&functionReturnsRValueRef();
+
+void derefMoveConstructedWithRValueRefReturn() {
+  std::unique_ptr<A> P(functionReturnsRValueRef());
+  P->foo();  // No warning.
+}