https://github.com/benedekaibas created
https://github.com/llvm/llvm-project/pull/196602
This implementation detects a use-after-move for the 3-arguments std::move on
containers. This PR fixes #137157.
Since my current implementation uses `IteratorModeling` which is in alpha stage
I mark this PR as draft.
When both the `IteratorModeling` and `MoveChecker` are enabled my
implementation works to detect the use-after-move for the 3 argument std::move
case.
```cpp
std::move(l1.begin(), l1.end(), std::back_inserter(l2));
std::cout << "l1: " << *l1.cbegin() << '\n'; // <--- should have a
use-after-move
```
```text
move_iterator.cpp:14:28: warning: Method called on moved-from object 'l1' of
type 'std::list' [cplusplus.Move]
14 | std::cout << "l1: " << *l1.cbegin() << '\n'; // <--- should ...
| ^~~~~~~~~~~~
```
`evalCall` models the 3-arg `std::move` pattern and marks the source container
in `TrackedContentsMap` to avoid false positives on safe method calls. In
`checkPreCall` I recover the iterator's container through `getIteratorPosition`
and check it against `TrackedContentsMap` to emit the warning.
I have been thinking about alternative solutions that do not depend on
`IteratorModeling`, but I think it would be more time saving to ask maintainers
about possible solutions before I start my own implementation.
>From c9ad17f36d073b747f9ae52c074a221681885db1 Mon Sep 17 00:00:00 2001
From: benedekaibas <[email protected]>
Date: Mon, 4 May 2026 19:23:51 -0400
Subject: [PATCH 1/2] [analyzer] 3-arg std::move initial commit for maintainer
review.
---
.../StaticAnalyzer/Checkers/MoveChecker.cpp | 81 ++++++++++++++++++-
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index ba8281b186c5d..2cc46923c8dc5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -12,19 +12,23 @@
//
//===----------------------------------------------------------------------===//
+#include "Iterator.h"
#include "Move.h"
#include "clang/AST/Attr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/Basic/OperatorKinds.h"
#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "llvm/ADT/StringSet.h"
using namespace clang;
using namespace ento;
+using namespace iterator;
namespace {
struct RegionState {
@@ -47,11 +51,12 @@ struct RegionState {
namespace {
class MoveChecker
: public Checker<check::PreCall, check::PostCall,
- check::DeadSymbols, check::RegionChanges> {
+ check::DeadSymbols, check::RegionChanges, eval::Call> {
public:
void checkPreCall(const CallEvent &MC, CheckerContext &C) const;
void checkPostCall(const CallEvent &MC, CheckerContext &C) const;
void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
+ bool evalCall(const CallEvent &Call, CheckerContext &C) const;
ProgramStateRef
checkRegionChanges(ProgramStateRef State,
const InvalidatedSymbols *Invalidated,
@@ -205,6 +210,8 @@ class MoveChecker
private:
BugType BT{this, "Use-after-move", categories::CXXMoveSemantics};
+ const CallDescription StdMoveCall{CDM::SimpleFunc, {"std", "move"}, 3};
+
// Check if the given form of potential misuse of a given object
// should be reported. If so, get it reported. The callback from which
// this function was called should immediately return after the call
@@ -229,6 +236,7 @@ class MoveChecker
} // end anonymous namespace
REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *,
RegionState)
+REGISTER_MAP_WITH_PROGRAMSTATE(TrackedContentsMap, const MemRegion *,
RegionState)
// Define the inter-checker API.
namespace clang {
@@ -495,6 +503,48 @@ void MoveChecker::checkPostCall(const CallEvent &Call,
assert(!C.isDifferent() && "Should not have made transitions on this path!");
}
+bool MoveChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+
+ const auto *CE = dyn_cast_if_present<CallExpr>(Call.getOriginExpr());
+ if (!CE)
+ return false;
+ ProgramStateRef State = C.getState();
+
+ if (!StdMoveCall.matches(Call))
+ return false;
+
+ const auto *BeginCall =
+ dyn_cast<CXXMemberCallExpr>(CE->getArg(0)->IgnoreImpCasts());
+ if (!BeginCall)
+ return false;
+
+ const Expr *ContainerExpr = BeginCall->getImplicitObjectArgument();
+ const auto *DRE = dyn_cast<DeclRefExpr>(ContainerExpr->IgnoreImpCasts());
+ if (!DRE)
+ return false;
+
+ const auto *VD = dyn_cast<VarDecl>(DRE->getDecl());
+ if (!VD)
+ return false;
+
+ const MemRegion *Region =
+ State->getLValue(VD, C.getLocationContext()).getAsRegion();
+ if (!Region)
+ return false;
+
+ const CXXRecordDecl *RD = ContainerExpr->getType()->getAsCXXRecordDecl();
+ if (!RD)
+ return false;
+
+ ObjectKind OK = classifyObject(State, Region, RD);
+
+ if (shouldBeTracked(OK)) {
+ State = State->set<TrackedContentsMap>(Region, RegionState::getMoved());
+ }
+ C.addTransition(State);
+ return true;
+}
+
bool MoveChecker::isMoveSafeMethod(const CXXMethodDecl *MethodDec) const {
// We abandon the cases where bool/void/void* conversion happens.
if (const auto *ConversionDec =
@@ -643,6 +693,32 @@ void MoveChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
// class in which the encountered method defined.
ThisRegion = ThisRegion->getMostDerivedObjectRegion();
+ // Store class declaration as well, for bug reporting purposes.
+ const CXXRecordDecl *RD = MethodDecl->getParent();
+
+ if (MethodDecl->getOverloadedOperator() == OO_Star ||
+ MethodDecl->getOverloadedOperator() == OO_Arrow) {
+ SVal Val = IC->getCXXThisVal();
+
+ if (const auto *POS = getIteratorPosition(State, Val)) {
+ const MemRegion *ContainerRegion = POS->getContainer();
+
+ const auto *TypedRegion = cast<TypedValueRegion>(ContainerRegion);
+ QualType ObjTy = TypedRegion->getValueType();
+ const auto *R = ObjTy->getAsCXXRecordDecl();
+ if (State->get<TrackedContentsMap>(ContainerRegion)) {
+ ExplodedNode *N = tryToReportBug(ContainerRegion, R, C, MK_FunCall);
+ if (!N || N->isSink())
+ return;
+
+ State = State->set<TrackedContentsMap>(ContainerRegion,
+ RegionState::getReported());
+ C.addTransition(State, N);
+ return;
+ }
+ }
+ }
+
if (isStateResetMethod(MethodDecl)) {
State = removeFromState(State, ThisRegion);
C.addTransition(State);
@@ -652,9 +728,6 @@ void MoveChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (isMoveSafeMethod(MethodDecl))
return;
- // Store class declaration as well, for bug reporting purposes.
- const CXXRecordDecl *RD = MethodDecl->getParent();
-
if (MethodDecl->isOverloadedOperator()) {
OverloadedOperatorKind OOK = MethodDecl->getOverloadedOperator();
>From a4b82b66e85f22611644429c26033d51c8fdd60e Mon Sep 17 00:00:00 2001
From: benedekaibas <[email protected]>
Date: Fri, 8 May 2026 13:59:37 -0400
Subject: [PATCH 2/2] Implemented the 3-arg std::move analysis.
---
.../StaticAnalyzer/Checkers/MoveChecker.cpp | 1 +
.../Inputs/system-header-simulator-cxx.h | 24 +++++++++++++
clang/test/Analysis/use-after-move.cpp | 36 +++++++++++++++++++
3 files changed, 61 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
index 2cc46923c8dc5..f874b257523e8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -508,6 +508,7 @@ bool MoveChecker::evalCall(const CallEvent &Call,
CheckerContext &C) const {
const auto *CE = dyn_cast_if_present<CallExpr>(Call.getOriginExpr());
if (!CE)
return false;
+
ProgramStateRef State = C.getState();
if (!StdMoveCall.matches(Call))
diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
index c5aeb0af9d578..78fe4994b8d44 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -386,6 +386,8 @@ namespace std {
void assign(std::initializer_list<T> ilist);
void clear();
+ size_t size() const;
+ bool empty() const;
void push_back(const T &value);
void push_back(T &&value);
@@ -1002,6 +1004,28 @@ next(ForwardIterator it,
OutputIterator copy(InputIterator first, InputIterator last,
OutputIterator result);
+ template<class InputIterator, class OutputIterator>
+ OutputIterator move(InputIterator first, InputIterator last,
+ OutputIterator result);
+
+ template <typename Container> struct back_insert_iterator {
+ Container *item;
+
+ back_insert_iterator(Container& container) : item(&container) {}
+
+ back_insert_iterator& operator=(const typename Container::value_type&
value) {
+ item->push_back(value);
+ return *this;
+ }
+ back_insert_iterator& operator*() {return *this;}
+ back_insert_iterator& operator++() {return *this;}
+ back_insert_iterator operator++(int) {return *this;}
+ };
+
+ template <typename Container>
+ back_insert_iterator<Container> back_inserter(Container& c) {
+ return back_insert_iterator<Container>(c);
+ }
}
#if __cplusplus >= 201103L
diff --git a/clang/test/Analysis/use-after-move.cpp
b/clang/test/Analysis/use-after-move.cpp
index 24d5dd8a8b3d2..82f0509c0053b 100644
--- a/clang/test/Analysis/use-after-move.cpp
+++ b/clang/test/Analysis/use-after-move.cpp
@@ -1015,3 +1015,39 @@ struct OtherMoveSafeClasses {
// aggressive-note@-2 {{Moved-from object 'Task' is moved}}
}
};
+
+// This test case does not pass because my implementation uses
IteratorModeling.cpp
+// and the warning can be only emitted if IteratorModeling is enabled.
+// If both Move checker and the IteratorModeling are enabled then the analyzer
can emit a warning.
+
+/*
+void starOperatorAfterMove() {
+ std::list<std::string> l1;
+ l1.push_back("l1");
+ std::list<std::string> l2;
+
+ std::move(l1.begin(), l1.end(), std::back_inserter(l2)); // peaceful-note
{{Object 'l1' is moved}}
+ *l1.cbegin(); // peaceful-warning {{Method called on moved-from object 'l1'}}
+ // peaceful-note@-1 {{Method called on moved-from object 'l1'}}
+}
+*/
+
+void safeOperatorAfterMove() {
+ std::list<std::string> l1;
+ l1.push_back("l1");
+ std::list<std::string> l2;
+
+ std::move(l1.begin(), l1.end(), std::back_inserter(l2));
+ *l2.cbegin(); // no-warning
+}
+
+void sizeAfterMove() {
+ std::list<std::string> l1;
+ l1.push_back("l1");
+ l1.push_back("l2");
+ std::list<std::string> l2;
+
+ std::move(l1.begin(), l1.end(), std::back_inserter(l2));
+ l1.size(); // no-warning
+}
+
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
