[clang] 76c0577 - [Analyzer] Handle unique_ptr::swap() in SmartPtrModeling

[Nithin Vadukkumchery Rajendrakumar via cfe-commits]

Author: Nithin Vadukkumchery Rajendrakumar
Date: 2020-07-21T12:05:27+02:00
New Revision: 76c0577763505ea3db1017a9aab579c1c2f135d0

URL: https://github.com/llvm/llvm-project/commit/76c0577763505ea3db1017a9aab579c1c2f135d0
DIFF: https://github.com/llvm/llvm-project/commit/76c0577763505ea3db1017a9aab579c1c2f135d0.diff

LOG:     [Analyzer] Handle unique_ptr::swap() in SmartPtrModeling

    Summary:
    Implemented modeling for unique_ptr::swap() SmartPtrModeling

    Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, martong, ASDenysPetrov, cfe-commits

    Reviewers: NoQ, Szelethus, vsavchenko, xazax.hun
    Reviewed By: NoQ, vsavchenko, xazax.hun

    Tags: #clang

    Differential Revision: https://reviews.llvm.org/D8387

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
    clang/test/Analysis/Inputs/system-header-simulator-cxx.h
    clang/test/Analysis/smart-ptr.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
index 1b2174501d6e..c36e89c3e3a9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -108,6 +108,17 @@ removeTrackedSubregions(TrackedRegionMapTy RegionMap,
   return RegionMap;
 }
 
+static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
+                                           const MemRegion *Region,
+                                           const SVal *RegionInnerPointerVal) {
+  if (RegionInnerPointerVal) {
+    State = State->set<TrackedRegionMap>(Region, *RegionInnerPointerVal);
+  } else {
+    State = State->remove<TrackedRegionMap>(Region);
+  }
+  return State;
+}
+
 bool SmartPtrModeling::isNullAfterMoveMethod(const CallEvent &Call) const {
   // TODO: Update CallDescription to support anonymous calls?
   // TODO: Handle other methods, such as .get() or .release().
@@ -129,7 +140,8 @@ bool SmartPtrModeling::evalCall(const CallEvent &Call,
         cast<CXXInstanceCall>(&Call)->getCXXThisVal().getAsRegion();
 
     if (!move::isMovedFrom(State, ThisR)) {
-      // TODO: Model this case as well. At least, avoid invalidation of globals.
+      // TODO: Model this case as well. At least, avoid invalidation of
+      // globals.
       return false;
     }
 
@@ -204,7 +216,7 @@ void SmartPtrModeling::handleReset(const CallEvent &Call,
     return;
   auto State = updateTrackedRegion(Call, C, ThisValRegion);
   C.addTransition(State);
-  // TODO: Make sure to ivalidate the the region in the Store if we don't have
+  // TODO: Make sure to ivalidate the region in the Store if we don't have
   // time to model all methods.
 }
 
@@ -232,7 +244,30 @@ void SmartPtrModeling::handleRelease(const CallEvent &Call,
 
 void SmartPtrModeling::handleSwap(const CallEvent &Call,
                                   CheckerContext &C) const {
-  // TODO: Add support to handle swap method.
+  // To model unique_ptr::swap() method.
+  const auto *IC = dyn_cast<CXXInstanceCall>(&Call);
+  if (!IC)
+    return;
+
+  const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
+  if (!ThisRegion)
+    return;
+
+  const auto *ArgRegion = Call.getArgSVal(0).getAsRegion();
+  if (!ArgRegion)
+    return;
+
+  auto State = C.getState();
+  const auto *ThisRegionInnerPointerVal =
+      State->get<TrackedRegionMap>(ThisRegion);
+  const auto *ArgRegionInnerPointerVal =
+      State->get<TrackedRegionMap>(ArgRegion);
+
+  // Swap the tracked region values.
+  State = updateSwappedRegion(State, ThisRegion, ArgRegionInnerPointerVal);
+  State = updateSwappedRegion(State, ArgRegion, ThisRegionInnerPointerVal);
+
+  C.addTransition(State);
 }
 
 ProgramStateRef

diff  --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index d5e7c4c9218d..9010ce2bb9b6 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -971,6 +971,12 @@ class unique_ptr {
   operator bool() const noexcept;
   unique_ptr<T> &operator=(unique_ptr<T> &&p) noexcept;
 };
+
+// TODO :: Once the deleter parameter is added update with additional template parameter.
+template <typename T>
+void swap(unique_ptr<T> &x, unique_ptr<T> &y) noexcept {
+  x.swap(y);
+}
 } // namespace std
 #endif
 

diff  --git a/clang/test/Analysis/smart-ptr.cpp b/clang/test/Analysis/smart-ptr.cpp
index 5645afc9b657..168682ba758f 100644
--- a/clang/test/Analysis/smart-ptr.cpp
+++ b/clang/test/Analysis/smart-ptr.cpp
@@ -201,3 +201,30 @@ void derefAfterAssignment() {
     Q->foo(); // no-warning
   }
 }
+
+void derefOnSwappedNullPtr() {
+  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> PNull;
+  P.swap(PNull);
+  PNull->foo(); // No warning.
+  (*P).foo();   // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnStdSwappedNullPtr() {
+  std::unique_ptr<A> P;
+  std::unique_ptr<A> PNull;
+  std::swap(P, PNull);
+  PNull->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo();     // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnSwappedValidPtr() {
+  std::unique_ptr<A> P(new A());
+  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.
+}