https://github.com/guillem-bartrina-sonarsource updated https://github.com/llvm/llvm-project/pull/169626
>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 1/2] 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}} +} >From 611fd870977c5acd4b829299adaa0c322832aefa Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Fri, 28 Nov 2025 17:05:40 +0100 Subject: [PATCH 2/2] Address review comments --- .../StaticAnalyzer/Checkers/MoveChecker.cpp | 6 +++--- .../Analysis/use-after-move-invalidation.cpp | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index 95ccb183d1e6e..ba8281b186c5d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -245,11 +245,11 @@ 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, - bool strict = false) { + bool Strict = false) { if (!Region) return State; for (auto &E : State->get<TrackedRegionMap>()) { - if ((!strict || E.first != Region) && E.first->isSubRegionOf(Region)) + if ((!Strict || E.first != Region) && E.first->isSubRegionOf(Region)) State = State->remove<TrackedRegionMap>(E.first); } return State; @@ -719,7 +719,7 @@ ProgramStateRef MoveChecker::checkRegionChanges( for (const auto *Region : RequestedRegions) { if (llvm::is_contained(InvalidatedRegions, Region)) State = removeFromState(State, Region, - /*strict=*/!!dyn_cast<CXXInstanceCall>(Call)); + /*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 index 8201fed26f1e3..c46aa6eba65d9 100644 --- a/clang/test/Analysis/use-after-move-invalidation.cpp +++ b/clang/test/Analysis/use-after-move-invalidation.cpp @@ -1,30 +1,30 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.Move %s -std=c++11\ -// RUN: -analyzer-output=text -verify=expected +// RUN: -analyzer-output=text #include "Inputs/system-header-simulator-cxx.h" class C { int n; public: - void meth(); + void memberFun(); }; -void opaqueFreeFun(); +template <class... Ts> void opaqueFreeFun(Ts...); class Foo { std::unique_ptr<C> up; - void opaqueMeth(); + void opaqueMemberFun(); void testOpaqueMeth() { auto tmp = std::move(up); - opaqueMeth(); // clears region state - (void)up->meth(); // no-warning + 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->meth(); // expected-warning{{Dereference of null smart pointer 'up' of type 'std::unique_ptr'}} + (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'}} } }; @@ -34,7 +34,7 @@ void testInstanceMeth(C c) { 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 + c.memberFun(); // does not clear region state auto tmp4 = std::move(c); auto tmp5 = std::move(c); // no-warning } @@ -46,7 +46,7 @@ void testPassByRef(C c) { 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 + 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
