Author: guillem-bartrina-sonarsource Date: 2025-12-08T11:00:54Z New Revision: f9e0fa8ba4100e8acb8907a5a32c0175d0711162
URL: https://github.com/llvm/llvm-project/commit/f9e0fa8ba4100e8acb8907a5a32c0175d0711162 DIFF: https://github.com/llvm/llvm-project/commit/f9e0fa8ba4100e8acb8907a5a32c0175d0711162.diff LOG: [analyzer] MoveChecker: correct invalidation of this-regions (#169626) By completely omitting invalidation in the case of InstanceCall, we do not clear the moved state of the fields of the this object after an opaque call to a member function of the object itself. Added: clang/test/Analysis/use-after-move-invalidation.cpp Modified: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index f5c34078df2ee..ba8281b186c5d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -244,11 +244,12 @@ bool isMovedFrom(ProgramStateRef State, const MemRegion *Region) { // If a region is removed all of the subregions needs to be removed too. static ProgramStateRef removeFromState(ProgramStateRef State, - const MemRegion *Region) { + const MemRegion *Region, + bool Strict = false) { if (!Region) return State; for (auto &E : State->get<TrackedRegionMap>()) { - if (E.first->isSubRegionOf(Region)) + if ((!Strict || E.first != Region) && E.first->isSubRegionOf(Region)) State = State->remove<TrackedRegionMap>(E.first); } return State; @@ -709,18 +710,16 @@ ProgramStateRef MoveChecker::checkRegionChanges( // that are passed directly via non-const pointers or non-const references // or rvalue references. // In case of an InstanceCall don't invalidate the this-region since - // it is fully handled in checkPreCall and checkPostCall. - const MemRegion *ThisRegion = nullptr; - if (const auto *IC = dyn_cast<CXXInstanceCall>(Call)) - ThisRegion = IC->getCXXThisVal().getAsRegion(); + // it is fully handled in checkPreCall and checkPostCall, but do invalidate + // its strict subregions, as they are not handled. // Requested ("explicit") regions are the regions passed into the call // directly, but not all of them end up being invalidated. // But when they do, they appear in the InvalidatedRegions array as well. for (const auto *Region : RequestedRegions) { - if (ThisRegion != Region && - llvm::is_contained(InvalidatedRegions, Region)) - State = removeFromState(State, Region); + if (llvm::is_contained(InvalidatedRegions, Region)) + State = removeFromState(State, Region, + /*Strict=*/isa<CXXInstanceCall>(Call)); } } else { // For invalidations that aren't caused by calls, assume nothing. In diff --git a/clang/test/Analysis/use-after-move-invalidation.cpp b/clang/test/Analysis/use-after-move-invalidation.cpp new file mode 100644 index 0000000000000..cf560d57921cb --- /dev/null +++ b/clang/test/Analysis/use-after-move-invalidation.cpp @@ -0,0 +1,51 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s -std=c++11\ +// RUN: -analyzer-output=text + +#include "Inputs/system-header-simulator-cxx.h" + +class C { + int n; +public: + void memberFun(); +}; + +template <class... Ts> void opaqueFreeFun(Ts...); + +class Foo { + std::unique_ptr<C> up; + void opaqueMemberFun(); + + void testOpaqueMemberFun() { + auto tmp = std::move(up); + opaqueMemberFun(); // clears region state + (void)up->memberFun(); // no-warning + } + + void testOpaqueFreeFun() { + auto tmp = std::move(up); // expected-note{{Smart pointer 'up' of type 'std::unique_ptr' is reset to null when moved from}} + opaqueFreeFun(); // does not clear region state + (void)up->memberFun(); // expected-warning{{Dereference of null smart pointer 'up' of type 'std::unique_ptr'}} + // expected-note@-1{{Dereference of null smart pointer 'up' of type 'std::unique_ptr'}} + } +}; + +void testInstanceMemberFun(C c) { + auto tmp1 = std::move(c); // expected-note{{Object 'c' is moved}} + auto tmp2 = std::move(c); // expected-warning{{Moved-from object 'c' is moved}} + // expected-note@-1{{Moved-from object 'c' is moved}} + auto tmp3 = std::move(c); + c.memberFun(); // does not clear region state + auto tmp4 = std::move(c); + auto tmp5 = std::move(c); // no-warning +} + +void testPassByRef(C c) { + auto tmp1 = std::move(c); // expected-note{{Object 'c' is moved}} + auto tmp2 = std::move(c); // expected-warning{{Moved-from object 'c' is moved}} + // expected-note@-1{{Moved-from object 'c' is moved}} + auto tmp3 = std::move(c); + opaqueFreeFun<C&>(c); // clears region state + auto tmp4 = std::move(c); // expected-note{{Object 'c' is moved}} + auto tmp5 = std::move(c); // expected-warning{{Moved-from object 'c' is moved}} + // expected-note@-1{{Moved-from object 'c' is moved}} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
