[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

295

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

295

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

352–394

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

clang/test/Analysis/smart-ptr.cpp

20

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

295

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

 // SmartPtrModeling.cpp - Model behavior of C++ smart pointers - C++ ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
checkRegionChanges(ProgramStateRef State,
                      ArrayRef<const MemRegion *> ExplicitRegions,
                      ArrayRef<const MemRegion *> Regions,
                      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;
+  bool handleMoveCtr(const CallEvent &Call, CheckerContext &C,
+                     const MemRegion *ThisRegion) const;
 
   using SmartPtrMethodHandlerFn =
       void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
   CallDescriptionMap<SmartPtrMethodHandlerFn> SmartPtrMethodHandlers{
       {{"reset"}, &SmartPtrModeling::handleReset},
       {{"release"}, &SmartPtrModeling::handleRelease},
       {{"swap", 1}, &SmartPtrModeling::handleSwap}};
 };
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(
           return;
         BR.markInteresting(ArgRegion);
         OS << "Swapped null smart pointer ";
         ArgRegion->printPretty(OS);
         OS << " with smart pointer ";
         ThisRegion->printPretty(OS);
       }));
 }
 
+bool SmartPtrModeling::handleMoveCtr(const CallEvent &Call, CheckerContext &C,
+                                     const MemRegion *ThisRegion) const {
+  const auto *ArgRegion = Call.getArgSVal(0).getAsRegion();
+  if (!ArgRegion)
+    return false;
+
+  ProgramStateRef State = C.getState();
+  const auto *ArgInnerPtrVal = State->get<TrackedRegionMap>(ArgRegion);
+  auto IsArgValNull = false;
+  if (ArgInnerPtrVal) {
+    State = State->set<TrackedRegionMap>(ThisRegion, *ArgInnerPtrVal);
+    IsArgValNull = ArgInnerPtrVal->isZeroConstant();
+  } else {
+    State = State->remove<TrackedRegionMap>(ThisRegion);
+  }
+  // Tracking the moved argument as null.
+  auto NullVal = C.getSValBuilder().makeNull();
+  State = State->set<TrackedRegionMap>(ArgRegion, NullVal);
+
+  C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion,
+                                       IsArgValNull](PathSensitiveBugReport &BR,
+                                                     llvm::raw_ostream &OS) {
+    if (&BR.getBugType() != smartptr::getNullDereferenceBugType())
+      return;
+    if (BR.isInteresting(ArgRegion)) {
+      OS << "Smart pointer ";
+      ArgRegion->printPretty(OS);
+      OS << " is null after moved to construct ";
+      ThisRegion->printPretty(OS);
+    }
+    if (BR.isInteresting(ThisRegion) && IsArgValNull) {
+      OS << "Smart pointer ";
+      ThisRegion->printPretty(OS);
+      OS << " is constructed with null value moved from ";
+      ArgRegion->printPretty(OS);
+      BR.markInteresting(ArgRegion);
+    }
+  }));
+  return true;
+}
+
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
   auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
   Checker->ModelSmartPtrDereference =
       Mgr.getAnalyzerOptions().getCheckerBooleanOption(
           Checker, "ModelSmartPtrDereference");
 }
 
 bool ento::shouldRegisterSmartPtrModeling(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.CPlusPlus;
 }

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

 
 void noNoteTagsForNonMatchingBugType() {
   std::unique_ptr<A> P; // No note.
   std::unique_ptr<A> P1; // No note.
   P1 = 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' [cplusplus.Move]}}
   // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
 }
+
+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 {{Smart pointer 'P' is constructed with null value moved from 'PToMove'}}
+  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 moved to construct '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 moved to construct '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 moved to construct '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'}}
 }
 
 // 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)) {
   (s->*func)(); // no-crash
void derefOnSwappedValidPtr() {
   std::unique_ptr<A> PValid(new A());
   P.swap(PValid);
   (*P).foo(); // No warning.
   PValid->foo(); // No warning.
   std::swap(P, PValid);
   P->foo(); // No warning.
   PValid->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> PToMove) {
+  std::unique_ptr<A> P(functionReturnsRValueRef());
+  P->foo();  // No warning.
+}