[PATCH] D83877: [Analyzer] Changed in SmartPtrModeling to handle Swap

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83877

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


Index: clang/test/Analysis/smart-ptr.cpp
===================================================================
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -67,14 +67,14 @@
   std::unique_ptr<A> P(new A());
   P.release();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo();                         // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterReset() {
   std::unique_ptr<A> P(new A());
   P.reset();
   clang_analyzer_numTimesReached(); // expected-warning {{1}}
-  P->foo(); // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+  P->foo();                         // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
 }
 
 void derefAfterResetWithNull() {
@@ -101,3 +101,30 @@
   A *AP = P.release();
   AP->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
 }
+
+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 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.
+}
+
+void derefOnStdSwappedNullPtr() {
+  std::unique_ptr<A> P(new A());
+  std::unique_ptr<A> PNull;
+  std::swap(P, PNull);
+  PNull->foo(); // No warning.
+  P->foo();     // expected-warning {{Dereference of null smart pointer [alpha.cplusplus.SmartPtr]}}
+}
\ No newline at end of file
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===================================================================
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -960,7 +960,12 @@
     T *operator->() const;
     operator bool() const;
   };
-}
+
+  template <typename T>
+  void swap(unique_ptr<T> &x, unique_ptr<T> &y) noexcept {
+    x.swap(y);
+  }
+  } // namespace std
 #endif
 
 #ifdef TEST_INLINABLE_ALLOCATORS
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -169,7 +169,7 @@
     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.
 }
 
@@ -197,7 +197,37 @@
 
 void SmartPtrModeling::handleSwap(const CallEvent &Call,
                                   CheckerContext &C) const {
-  // TODO: Add support to handle 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.
+  if (ArgRegionInnerPointerVal) {
+    State = State->set<TrackedRegionMap>(ThisRegion, *ArgRegionInnerPointerVal);
+  } else {
+    State = State->remove<TrackedRegionMap>(ThisRegion);
+  }
+  if (ThisRegionInnerPointerVal) {
+    State = State->set<TrackedRegionMap>(ArgRegion, *ThisRegionInnerPointerVal);
+  } else {
+    State = State->remove<TrackedRegionMap>(ArgRegion);
+  }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef

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