[clang] b561bdb - [LifetimeSafety] Detect iterator invalidation through container aliases (#195231)
via cfe-commits
cfe-commits at lists.llvm.org
Sat May 2 21:14:33 PDT 2026
Author: Zeyi Xu
Date: 2026-05-03T12:14:28+08:00
New Revision: b561bdbedd7bb59112cbb3eeafda70e3493555f4
URL: https://github.com/llvm/llvm-project/commit/b561bdbedd7bb59112cbb3eeafda70e3493555f4
DIFF: https://github.com/llvm/llvm-project/commit/b561bdbedd7bb59112cbb3eeafda70e3493555f4.diff
LOG: [LifetimeSafety] Detect iterator invalidation through container aliases (#195231)
The previous heuristic in `handleInvalidatingCall` is too conservative.
The ideal way would be completely removing this, but it would introduce
~10 regressions in the existing testcases.
This commit replace the filter with a narrower guard that only skips
direct field accesses (AccessPath currently lacks field granularity and
cannot distinguish `s.v1` from `s.v2`).
Closes https://github.com/llvm/llvm-project/issues/193044
Added:
Modified:
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
clang/test/Sema/warn-lifetime-safety-invalidations.cpp
Removed:
################################################################################
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index dc80d2783fc03..32b154a972212 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -810,9 +810,11 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
if (!isInvalidationMethod(*MD))
return;
- // Heuristics to turn-down false positives.
- auto *DRE = dyn_cast<DeclRefExpr>(Args[0]);
- if (!DRE || DRE->getDecl()->getType()->isReferenceType())
+
+ // Heuristics to turn-down false positives. Skip member field expressions for
+ // now. This is not a perfect filter and will still surface some false
+ // positives (e.g. `auto& r = s.v`).
+ if (!isa<DeclRefExpr>(Args[0]->IgnoreParenImpCasts()))
return;
OriginList *ThisList = getOriginsList(*Args[0]);
diff --git a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
index f1044e2ad1cdd..e3795a7b6e746 100644
--- a/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-invalidations.cpp
@@ -236,13 +236,12 @@ void IteratorUsedAfterErase(std::vector<int> v) {
}
}
-// FIXME: Detect this. We currently skip invalidation through ref/pointers to containers.
-void IteratorUsedAfterPushBackParam(std::vector<int>& v) {
+void IteratorUsedAfterPushBackParam(std::vector<int>& v) { // expected-warning {{parameter is later invalidated}}
auto it = std::begin(v);
if (it != std::end(v) && *it == 3) {
- v.push_back(4);
+ v.push_back(4); // expected-note {{invalidated here}}
}
- ++it;
+ ++it; // expected-note {{later used here}}
}
void IteratorUsedAfterPushBack(std::vector<int> v) {
@@ -254,6 +253,46 @@ void IteratorUsedAfterPushBack(std::vector<int> v) {
}
} // namespace SimpleInvalidIterators
+namespace InvalidatingThroughContainerAliases {
+void IteratorInvalidatedThroughLocalReferenceAlias() {
+ std::vector<int> vv;
+ std::vector<int> &v = vv;
+ auto it = vv.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ v.push_back(42); // expected-note {{invalidated here}}
+ (void)it; // expected-note {{later used here}}
+}
+
+void IteratorInvalidatedThroughPointerParameter(std::vector<int> *v) { // expected-warning {{parameter is later invalidated}}
+ auto it = v->begin();
+ v->push_back(42); // expected-note {{invalidated here}}
+ (void)it; // expected-note {{later used here}}
+}
+} // namespace InvalidatingThroughContainerAliases
+
+namespace ContainerObjectAliases {
+// FIXME: Distinguish owner-borrow from content-borrow.
+void PointerParameterObjectUseIsOk(std::vector<int> *v) { // expected-warning {{parameter is later invalidated}}
+ v->push_back(42); // expected-note {{invalidated here}}
+ (void)v; // expected-note {{later used here}}
+}
+
+// FIXME: Distinguish owner-borrow from content-borrow.
+void LocalPointerAliasObjectUseIsOk() {
+ std::vector<int> vv;
+ std::vector<int> *v = &vv; // expected-warning {{object whose reference is captured is later invalidated}}
+ v->push_back(42); // expected-note {{invalidated here}}
+ (void)*v; // expected-note {{later used here}}
+}
+
+// FIXME: Distinguish owner-borrow from content-borrow.
+void LocalReferenceAliasObjectUseIsOk() {
+ std::vector<int> vv;
+ std::vector<int> &v = vv; // expected-warning {{object whose reference is captured is later invalidated}}
+ v.push_back(42); // expected-note {{invalidated here}}
+ (void)v; // expected-note {{later used here}}
+}
+} // namespace ContainerObjectAliases
+
namespace ElementReferences {
// Testing raw pointers and references to elements, not just iterators.
@@ -289,7 +328,7 @@ void SelfInvalidatingMap() {
// insertion and following is unsafe for this container.
mp[1] = "42";
mp[2] // expected-note {{invalidated here}}
- =
+ =
mp[1]; // expected-warning {{object whose reference is captured is later invalidated}} expected-note {{later used here}}
}
@@ -297,7 +336,7 @@ void InvalidateErase() {
std::flat_map<int, std::string> mp;
// None of these containers provide iterator stability. So following is unsafe:
auto it = mp.find(3); // expected-warning {{object whose reference is captured is later invalidated}}
- mp.erase(mp.find(4)); // expected-note {{invalidated here}}
+ mp.erase(mp.find(4)); // expected-note {{invalidated here}}
if (it != mp.end()) // expected-note {{later used here}}
*it;
}
@@ -340,17 +379,20 @@ void Invalidate1Use1IsInvalid() {
s.strings1.push_back("1");
*it;
}
-void Invalidate1Use2IsOk() {
+void Invalidate2Use1IsOk() {
S s;
auto it = s.strings1.begin();
s.strings2.push_back("1");
*it;
-}void Invalidate1Use2ViaRefIsOk() {
+}
+
+// FIXME: Requires field-sensitive AccessPaths to fix.
+void Invalidate1Use2ViaRefIsOk() {
S s;
- auto it = s.strings2.begin();
- auto& strings2 = s.strings2;
- strings2.push_back("1");
- *it;
+ auto it = s.strings2.begin(); // expected-warning {{object whose reference is captured is later invalidated}}
+ auto& strings1 = s.strings1;
+ strings1.push_back("1"); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
void Invalidate1UseSIsOk() {
S s;
@@ -358,24 +400,26 @@ void Invalidate1UseSIsOk() {
s.strings2.push_back("1");
(void)*p;
}
+// FIXME: Distinguish owner-borrow from content-borrow.
void PointerToContainerIsOk() {
std::vector<std::string> s;
- std::vector<std::string>* p = &s;
- p->push_back("1");
- (void)*p;
+ std::vector<std::string>* p = &s; // expected-warning {{object whose reference is captured is later invalidated}}
+ p->push_back("1"); // expected-note {{invalidated here}}
+ (void)*p; // expected-note {{later used here}}
}
void IteratorFromPointerToContainerIsInvalidated() {
- // FIXME: Detect this.
std::vector<std::string> s;
- std::vector<std::string>* p = &s;
+ std::vector<std::string>* p = &s; // expected-warning {{object whose reference is captured is later invalidated}}
auto it = p->begin();
- p->push_back("1");
- *it;
+ p->push_back("1"); // expected-note {{invalidated here}}
+ *it; // expected-note {{later used here}}
}
+// FIXME: Distinguish invalidating an element's contents from invalidating
+// iterators into the outer container.
void ChangingRegionOwnedByContainerIsOk() {
std::vector<std::string> subdirs;
- for (std::string& path : subdirs)
- path = std::string();
+ for (std::string& path : subdirs) // expected-warning {{object whose reference is captured is later invalidated}} expected-note {{later used here}}
+ path = std::string(); // expected-note {{invalidated here}}
}
} // namespace ContainersAsFields
More information about the cfe-commits
mailing list