[clang] [analyzer] MoveChecker: correct invalidation of this-regions (PR #169626)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 28 08:06:07 PST 2025
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 <guillem.bartrina at sonarsource.com>
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 at -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 at -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 at -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 at -1{{Moved-from object 'c' is moved}}
+}
>From 611fd870977c5acd4b829299adaa0c322832aefa Mon Sep 17 00:00:00 2001
From: guillem-bartrina-sonarsource <guillem.bartrina at sonarsource.com>
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 at -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 at -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 at -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 at -1{{Moved-from object 'c' is moved}}
More information about the cfe-commits
mailing list