https://github.com/guillem-bartrina-sonarsource created https://github.com/llvm/llvm-project/pull/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. >From b8494cfa06a06f2cf40fc1d8a1a1ef14303edb59 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 26 Nov 2025 10:58:24 +0100 Subject: [PATCH] MoveChecker: correct invalidation of this-regions --- .../StaticAnalyzer/Checkers/MoveChecker.cpp | 17 +++--- .../Analysis/use-after-move-invalidation.cpp | 53 +++++++++++++++++++ 2 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 clang/test/Analysis/use-after-move-invalidation.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index f5c34078df2ee..95ccb183d1e6e 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=*/!!dyn_cast<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..8201fed26f1e3 --- /dev/null +++ b/clang/test/Analysis/use-after-move-invalidation.cpp @@ -0,0 +1,53 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s -std=c++11\ +// RUN: -analyzer-output=text -verify=expected + +#include "Inputs/system-header-simulator-cxx.h" + +class C { + int n; +public: + void meth(); +}; + +void opaqueFreeFun(); + +class Foo { + std::unique_ptr<C> up; + void opaqueMeth(); + + void testOpaqueMeth() { + auto tmp = std::move(up); + opaqueMeth(); // clears region state + (void)up->meth(); // 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->meth(); // 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 testInstanceMeth(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.meth(); // does not clear region state + auto tmp4 = std::move(c); + auto tmp5 = std::move(c); // no-warning +} + +void modify(C&); + +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); + modify(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
