https://github.com/benedekaibas created https://github.com/llvm/llvm-project/pull/200143
I have implemented the `checkPostCall` function to understand the `lifetimebound` annotations when a parameter is annotated with it. I have generated the ExplodedGraph as well and the annotation is present. This PR is just the very base of the whole implementation of making the annotations present in the program state and I do not cover all the cases yet, but I just want to see if you have any suggestions for me. Here is the `Compiler Explorer` link to the code I have used as a test: https://godbolt.org/z/K896aceMf >From c35bed8a535d8b4198726f723b64943530fbb352 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Mon, 4 May 2026 19:23:51 -0400 Subject: [PATCH 1/5] [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 ad3664e268b7c6b564c15e0243dbd4e512d02455 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Fri, 8 May 2026 13:59:37 -0400 Subject: [PATCH 2/5] 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 f0447ee85379a6334d92f1c0867148ee17289c88 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Tue, 26 May 2026 14:21:40 +0200 Subject: [PATCH 3/5] [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 } - >From 58f0ffdc99b31b355befc2fbb7b9adcd5ac8a115 Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Wed, 27 May 2026 20:25:42 +0200 Subject: [PATCH 4/5] [analyzer] Implemented the core body for the lifetimebound annotation. --- .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 + .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../Checkers/LifetimeAnnotations.cpp | 74 +++++++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index eca2afbe340a9..85d59fc139728 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -788,6 +788,10 @@ def SmartPtrChecker: Checker<"SmartPtr">, Dependencies<[SmartPtrModeling]>, Documentation<HasDocumentation>; +def LifetimeAnnotations : Checker<"LifetimeAnnotations">, + HelpText<"Check for lifetime violations using lifetime annotations">, + Documentation<NotDocumented>; + } // end: "alpha.cplusplus" //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 8a0621077b977..3f426186189fa 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -55,6 +55,7 @@ add_clang_library(clangStaticAnalyzerCheckers IteratorModeling.cpp IteratorRangeChecker.cpp IvarInvalidationChecker.cpp + LifetimeAnnotations.cpp LLVMConventionsChecker.cpp LocalizationChecker.cpp MacOSKeychainAPIChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp new file mode 100644 index 0000000000000..797f32a23860f --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp @@ -0,0 +1,74 @@ +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include <AllocationState.h> + +using namespace clang; +using namespace ento; + +REGISTER_MAP_WITH_PROGRAMSTATE(LifetimeBoundMap, const MemRegion *, + const MemRegion *); + +class LifetimeAnnotations : public Checker<check::PostCall> { +public: + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, + const char *Sep) const override; +}; + +void LifetimeAnnotations::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + // create the initial state I will work with + ProgramStateRef State = C.getState(); + + const auto *MethodDecl = dyn_cast_if_present<CXXMethodDecl>(Call.getDecl()); + + if (!MethodDecl) + return; + + unsigned LBParamIdx = MethodDecl->getNumParams(); + for (unsigned i = 0; i < MethodDecl->getNumParams(); i++) { + if (MethodDecl->getParamDecl(i)->hasAttr<LifetimeBoundAttr>()) { + LBParamIdx = i; + break; + } + } + if (LBParamIdx == MethodDecl->getNumParams()) + return; + + SVal RetVal = Call.getReturnValue(); + const MemRegion *RetValRegion = RetVal.getAsRegion(); + if (!RetValRegion) + return; + + SVal ThisVal = Call.getArgSVal(LBParamIdx); + const MemRegion *ThisValRegion = ThisVal.getAsRegion(); + if (!ThisValRegion) + return; + + State = State->set<LifetimeBoundMap>(RetValRegion, ThisValRegion); + C.addTransition(State); +} + +void LifetimeAnnotations::printState(raw_ostream &Out, ProgramStateRef State, + const char *NL, const char *Sep) const { + auto LBTy = State->get<LifetimeBoundMap>(); + + if (!LBTy.isEmpty()) { + Out << Sep << "LifetimeBound objects: "; + + for (auto I : LBTy) { + Out << I.first << " bound to " << I.second << NL; + } + } +} + +void ento::registerLifetimeAnnotations(CheckerManager &mgr) { + mgr.registerChecker<LifetimeAnnotations>(); +} + +bool ento::shouldRegisterLifetimeAnnotations(const CheckerManager &mgr) { + return true; +} >From be99d9cc44a87e767cfc1854b7f2984f8743351c Mon Sep 17 00:00:00 2001 From: benedekaibas <[email protected]> Date: Wed, 27 May 2026 23:10:06 +0200 Subject: [PATCH 5/5] [analyzer] Corrected place holder name to display the correct behavior. --- .../lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp index 797f32a23860f..54e98b945c7b3 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeAnnotations.cpp @@ -20,7 +20,6 @@ class LifetimeAnnotations : public Checker<check::PostCall> { void LifetimeAnnotations::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - // create the initial state I will work with ProgramStateRef State = C.getState(); const auto *MethodDecl = dyn_cast_if_present<CXXMethodDecl>(Call.getDecl()); @@ -43,12 +42,12 @@ void LifetimeAnnotations::checkPostCall(const CallEvent &Call, if (!RetValRegion) return; - SVal ThisVal = Call.getArgSVal(LBParamIdx); - const MemRegion *ThisValRegion = ThisVal.getAsRegion(); - if (!ThisValRegion) + SVal ArgVal = Call.getArgSVal(LBParamIdx); + const MemRegion *ArgValRegion = ArgVal.getAsRegion(); + if (!ArgValRegion) return; - State = State->set<LifetimeBoundMap>(RetValRegion, ThisValRegion); + State = State->set<LifetimeBoundMap>(RetValRegion, ArgValRegion); C.addTransition(State); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
