https://github.com/benedekaibas updated https://github.com/llvm/llvm-project/pull/196602
>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/3] [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/3] 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 +} + >From eaf09e994f991532544df7bd8b446e9689431e6b Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Tue, 26 May 2026 14:21:40 +0200 Subject: [PATCH 3/3] [StaticAnalyzer] Addressed maintainers' review. --- .../StaticAnalyzer/Checkers/MoveChecker.cpp | 37 ++++++++++++++++++- .../test/Analysis/use-after-move-iterator.cpp | 29 +++++++++++++++ clang/test/Analysis/use-after-move.cpp | 17 --------- 3 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 clang/test/Analysis/use-after-move-iterator.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp index f874b257523e8..47e9c585054ae 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp @@ -210,6 +210,7 @@ class MoveChecker private: BugType BT{this, "Use-after-move", categories::CXXMoveSemantics}; + // Modelling the 3 argument std::move calls const CallDescription StdMoveCall{CDM::SimpleFunc, {"std", "move"}, 3}; // Check if the given form of potential misuse of a given object @@ -236,6 +237,9 @@ class MoveChecker } // end anonymous namespace REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, RegionState) + +// Custom map designed to track containers whose contents were moved by 3-arg +// std::move REGISTER_MAP_WITH_PROGRAMSTATE(TrackedContentsMap, const MemRegion *, RegionState) // Define the inter-checker API. @@ -539,9 +543,38 @@ bool MoveChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { ObjectKind OK = classifyObject(State, Region, RD); - if (shouldBeTracked(OK)) { + const auto *BackInsCall = dyn_cast<CallExpr>(CE->getArg(2)->IgnoreImpCasts()); + if (!BackInsCall) + return false; + + const Expr *DestExpr = BackInsCall->getArg(0)->IgnoreImpCasts(); + if (!DestExpr) + return false; + + const auto *DestDRE = dyn_cast<DeclRefExpr>(DestExpr); + if (!DestDRE) + return false; + + const auto *DestVD = dyn_cast<VarDecl>(DestDRE->getDecl()); + if (!DestVD) + return false; + + const MemRegion *DestRegion = + State->getLValue(DestVD, C.getLocationContext()).getAsRegion(); + if (!DestRegion) + return false; + + SValBuilder &SVB = State->getStateManager().getSValBuilder(); + SVal ReturnVal = SVB.conjureSymbolVal(Call, C.blockCount()); + State = State->BindExpr(CE, C.getLocationContext(), ReturnVal); + + State = State->invalidateRegions({DestRegion}, Call.getCFGElementRef(), + C.blockCount(), C.getLocationContext(), + /*CausesPointerEscape=*/false); + + if (shouldBeTracked(OK)) State = State->set<TrackedContentsMap>(Region, RegionState::getMoved()); - } + C.addTransition(State); return true; } diff --git a/clang/test/Analysis/use-after-move-iterator.cpp b/clang/test/Analysis/use-after-move-iterator.cpp new file mode 100644 index 0000000000000..50dd7e57b42e4 --- /dev/null +++ b/clang/test/Analysis/use-after-move-iterator.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus.Move,alpha.cplusplus.IteratorModeling -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify -analyzer-config display-checker-name=false + +#include "Inputs/system-header-simulator-cxx.h" + + +//===----------------------------------------------------------------------===// +// Test suite for test functions that require both MoveChecker.cpp and +// IteratorModeling.cpp to be enabled. +// NOTE: Currently the iterator dereference detection is only working when +// IteratorModeling is enabled. +//===----------------------------------------------------------------------===// + +void iteratorDerefSource() { + std::list<std::string> l1; + l1.push_back("l1"); + std::list<std::string> l2; + + std::move(l1.begin(), l1.end(), std::back_inserter(l2)); + *l1.cbegin(); // expected-warning {{Method called on moved-from object 'l1'}} +} + +void iteratorDerefDest() { + 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 +} diff --git a/clang/test/Analysis/use-after-move.cpp b/clang/test/Analysis/use-after-move.cpp index 82f0509c0053b..b26f0320f014b 100644 --- a/clang/test/Analysis/use-after-move.cpp +++ b/clang/test/Analysis/use-after-move.cpp @@ -1016,22 +1016,6 @@ struct OtherMoveSafeClasses { } }; -// 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"); @@ -1050,4 +1034,3 @@ void sizeAfterMove() { 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
