[PATCH] D104300: [analyzer] Handle `std::swap`

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD created this revision.
RedDocMD added reviewers: NoQ, vsavchenko, teemperor, xazax.hun.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware.
RedDocMD requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch handles the `std::swap` function specialization
for `std::unique_ptr`. Implemented to be very similar to
how `swap` method is handled


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104300

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp


Index: clang/test/Analysis/smart-ptr-text-output.cpp
===================================================================
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -76,13 +76,10 @@
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
-// FIXME: Fix this test when "std::swap" is modeled seperately.
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr<A> P; // expected-note {{Default constructed smart pointer 'P' is null}}
   std::unique_ptr<A> PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
-  // expected-note@-1 {{Calling 'swap<A>'}}
-  // expected-note@-2 {{Returning from 'swap<A>'}}
+  std::swap(P, PNull);      // expected-note {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -76,6 +76,7 @@
       {{"release"}, &SmartPtrModeling::handleRelease},
       {{"swap", 1}, &SmartPtrModeling::handleSwap},
       {{"get"}, &SmartPtrModeling::handleGet}};
+  const CallDescription StdSwapCall{{"std", "swap"}, 2};
 };
 } // end of anonymous namespace
 
@@ -91,11 +92,15 @@
     return false;
 
   const auto *RecordDecl = MethodDecl->getParent();
-  if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
+  return isStdSmartPtr(RecordDecl);
+}
+
+bool isStdSmartPtr(const CXXRecordDecl *RD) {
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
     return false;
 
-  if (RecordDecl->getDeclName().isIdentifier()) {
-    StringRef Name = RecordDecl->getName();
+  if (RD->getDeclName().isIdentifier()) {
+    StringRef Name = RD->getName();
     return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
   }
   return false;
@@ -178,6 +183,49 @@
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
   ProgramStateRef State = C.getState();
+
+  if (Call.isCalled(StdSwapCall)) {
+    // Check the first arg, if it is of std::unique_ptr type.
+    assert(Call.getNumArgs() == 2 && "std::swap should have two arguments");
+    const Expr *FirstArg = Call.getArgExpr(0);
+    if (!smartptr::isStdSmartPtr(FirstArg->getType()->getAsCXXRecordDecl())) {
+      return false;
+    }
+    const MemRegion *FirstArgThisRegion = Call.getArgSVal(0).getAsRegion();
+    if (!FirstArgThisRegion)
+      return false;
+    const MemRegion *SecondArgThisRegion = Call.getArgSVal(1).getAsRegion();
+    if (!SecondArgThisRegion)
+      return false;
+
+    const auto *FirstArgInnerPtrVal =
+        State->get<TrackedRegionMap>(FirstArgThisRegion);
+    const auto *SecondArgInnerPtrVal =
+        State->get<TrackedRegionMap>(SecondArgThisRegion);
+
+    State =
+        updateSwappedRegion(State, FirstArgThisRegion, SecondArgInnerPtrVal);
+    State =
+        updateSwappedRegion(State, SecondArgThisRegion, FirstArgInnerPtrVal);
+
+    // TODO: This thing needs a proper note!!
+    C.addTransition(
+        State,
+        C.getNoteTag([FirstArgThisRegion, SecondArgThisRegion](
+                         PathSensitiveBugReport &BR, llvm::raw_ostream &OS) {
+          if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+              !BR.isInteresting(FirstArgThisRegion))
+            return;
+          BR.markInteresting(SecondArgThisRegion);
+          OS << "Swapped null smart pointer";
+          checkAndPrettyPrintRegion(OS, SecondArgThisRegion);
+          OS << " with smart pointer";
+          checkAndPrettyPrintRegion(OS, FirstArgThisRegion);
+        }));
+
+    return true;
+  }
+
   if (!smartptr::isStdSmartPtrCall(Call))
     return false;
 
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
@@ -23,6 +23,8 @@
 /// Returns true if the event call is on smart pointer.
 bool isStdSmartPtrCall(const CallEvent &Call);
 
+bool isStdSmartPtr(const CXXRecordDecl *RD);
+
 /// Returns whether the smart pointer is null or not.
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion);
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits