From 76c0577763505ea3db1017a9aab579c1c2f135d0 Mon Sep 17 00:00:00 2001 From: Nithin Vadukkumchery Rajendrakumar Date: Wed, 15 Jul 2020 02:54:44 +0200 Subject: [PATCH] [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 --- .../StaticAnalyzer/Checkers/SmartPtrModeling.cpp | 41 ++++++++++++++++++++-- .../Analysis/Inputs/system-header-simulator-cxx.h | 6 ++++ clang/test/Analysis/smart-ptr.cpp | 27 ++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp index 1b2174501d6..c36e89c3e3a 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(Region, *RegionInnerPointerVal); + } else { + State = State->remove(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(&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(&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(ThisRegion); + const auto *ArgRegionInnerPointerVal = + State->get(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 d5e7c4c9218..9010ce2bb9b 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 @@ public: operator bool() const noexcept; unique_ptr &operator=(unique_ptr &&p) noexcept; }; + +// TODO :: Once the deleter parameter is added update with additional template parameter. +template +void swap(unique_ptr &x, unique_ptr &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 5645afc9b65..168682ba758 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 P(new A()); + std::unique_ptr 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 P; + std::unique_ptr 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 P(new A()); + std::unique_ptr 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. +} -- 2.11.0