[llvm-branch-commits] [clang] [LifetimeSafety] Support container interior paths and invalidations (PR #207523)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Jul 4 09:29:59 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-temporal-safety
Author: Utkarsh Saxena (usx95)
<details>
<summary>Changes</summary>
This patch completes the implementation of path-sensitive lifetime tracking by supporting container interior paths (`.*`) and deep-nested invalidation.
- Enables `PathElement::getInterior` generation in `FactsGenerator` for GSL Owners and Views (e.g. member functions, function parameters, lambda captures).
- Removes bypass checks in `FactsGenerator::handleInvalidatingCall` to track container invalidation on fields.
- Updates `Checker` to use strict prefix comparison (`isStrictPrefixOf`) for container invalidations, ensuring invalidation of container contents (interior) correctly invalidates iterators but not other sibling fields.
- Reorganizes tests in `invalidations.cpp` by resolving duplicates and distributing them logically.
- Updates unit tests and sema tests with correct expectations for interior paths.
TAG=agy
CONV=2cfd8d00-18d7-4a03-8d78-2aba2f9a8f23
---
Patch is 37.90 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/207523.diff
5 Files Affected:
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+9-2)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+17-6)
- (modified) clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h (+6-3)
- (modified) clang/test/Sema/LifetimeSafety/invalidations.cpp (+223-65)
- (modified) clang/unittests/Analysis/LifetimeSafetyTest.cpp (+109-67)
``````````diff
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index a749a49e7762a..559cc410abd42 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -220,11 +220,18 @@ class LifetimeChecker {
/// Get loans directly pointing to the invalidated container
LoanSet DirectlyInvalidatedLoans =
LoanPropagation.getLoans(InvalidatedOrigin, IOF);
+ bool AllowEquality =
+ isa_and_nonnull<CXXDeleteExpr>(IOF->getInvalidationExpr());
auto IsInvalidated = [&](const Loan *L) {
for (LoanID InvalidID : DirectlyInvalidatedLoans) {
const Loan *InvalidL = FactMgr.getLoanMgr().getLoan(InvalidID);
- if (InvalidL->getAccessPath().isPrefixOf(L->getAccessPath()))
- return true;
+ if (AllowEquality) {
+ if (InvalidL->getAccessPath().isPrefixOf(L->getAccessPath()))
+ return true;
+ } else {
+ if (InvalidL->getAccessPath().isStrictPrefixOf(L->getAccessPath()))
+ return true;
+ }
}
return false;
};
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 2b80da34f356e..d56d073af2d5f 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -962,11 +962,6 @@ void FactsGenerator::handleInvalidatingCall(const Expr *Call,
if (!isInvalidationMethod(*MD))
return;
- // Heuristics to turn-down false positives. Skip member field expressions for
- // now (NFC).
- if (!isa<DeclRefExpr>(Args[0]->IgnoreImpCasts()))
- return;
-
OriginList *ThisList = getOriginsList(*Args[0]);
if (ThisList)
CurrentBlockFacts.push_back(FactMgr.createFact<InvalidateOriginFact>(
@@ -1079,6 +1074,22 @@ void FactsGenerator::handleLifetimeCaptureBy(const FunctionDecl *FD,
static std::optional<PathElement>
getPathElementForLifetimeBoundArg(const FunctionDecl *FD, unsigned ArgIndex,
const Expr *ArgExpr) {
+ if (!ArgExpr)
+ return std::nullopt;
+ if (ArgIndex == 0) {
+ const auto *Method = dyn_cast<CXXMethodDecl>(FD);
+ bool IsContainerArg =
+ (Method && Method->isInstance()) || shouldTrackFirstArgument(FD);
+ if (IsContainerArg) {
+ QualType ArgType = ArgExpr->getType();
+ if (const Type *ArgTypePtr = ArgType.getTypePtrOrNull()) {
+ if (isGslOwnerType(ArgType) ||
+ (ArgTypePtr->isPointerType() &&
+ isGslOwnerType(ArgTypePtr->getPointeeType())))
+ return PathElement::getInterior();
+ }
+ }
+ }
return std::nullopt;
}
@@ -1161,7 +1172,7 @@ void FactsGenerator::handleFunctionCall(const Expr *Call,
// Only flow the outer origin because inner lengths may mismatch.
CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>(
CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc,
- std::nullopt));
+ PathElement::getInterior()));
KillSrc = false;
} else if (IsArgLifetimeBound(I)) {
// Only flow the outer origin here. For lifetimebound args in
diff --git a/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h b/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h
index 024c3c2bc51b7..815dcf8b6766d 100644
--- a/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/LifetimeSafety/Inputs/lifetime-analysis.h
@@ -28,11 +28,11 @@ template<typename T> struct remove_reference<T &> { typedef T type; };
template<typename T> struct remove_reference<T &&> { typedef T type; };
template< class InputIt, class T >
-InputIt find( InputIt first, InputIt last, const T& value );
+InputIt find(InputIt first, InputIt last, const T& value);
template< class ForwardIt1, class ForwardIt2 >
-ForwardIt1 search( ForwardIt1 first, ForwardIt1 last,
- ForwardIt2 s_first, ForwardIt2 s_last );
+ForwardIt1 search(ForwardIt1 first, ForwardIt1 last,
+ ForwardIt2 s_first, ForwardIt2 s_last);
template<typename T>
typename remove_reference<T>::type &&move(T &&t) noexcept;
@@ -225,6 +225,9 @@ struct basic_string {
~basic_string();
basic_string& operator=(const basic_string&);
basic_string& operator+=(const basic_string&);
+ basic_string& append(const basic_string&);
+ basic_string& replace(unsigned pos, unsigned count,
+ const basic_string& str);
basic_string& operator+=(const T*);
void push_back(T);
diff --git a/clang/test/Sema/LifetimeSafety/invalidations.cpp b/clang/test/Sema/LifetimeSafety/invalidations.cpp
index 1572e78a27e14..46980405e46fd 100644
--- a/clang/test/Sema/LifetimeSafety/invalidations.cpp
+++ b/clang/test/Sema/LifetimeSafety/invalidations.cpp
@@ -335,37 +335,41 @@ void IteratorInvalidatedThroughPointerParameter(std::vector<int> *v) { // expect
(void)it; // expected-note {{later used here}}
}
+void IteratorInvalidatedThroughPointerParameterNotUsed(std::vector<int> *v) {
+ // Ok as 'it' is not used. Only 'v' is used.
+ auto it = v->begin();
+ v->push_back(42);
+ v->push_back(43);
+ v->clear();
+}
+
void ParenthesizedContainerInvalidatesIterator() {
- // FIXME: Support invalidation through non-DRE lvalue expressions.
std::vector<int> v;
- auto it = v.begin();
- (v).push_back(42);
- (void)it;
+ auto it = v.begin(); // expected-warning {{local variable 'v' is later invalidated}}
+ (v).push_back(42); // expected-note {{local variable 'v' is 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 'v' is later invalidated}}
- v->push_back(42); // expected-note {{parameter 'v' is invalidated here}}
- (void)v; // expected-note {{later used here}}
+void PointerParameterObjectUseIsOk(std::vector<int> *v) {
+ v->push_back(42);
+ (void)v;
}
-// FIXME: Distinguish owner-borrow from content-borrow.
void LocalPointerAliasObjectUseIsOk() {
std::vector<int> vv;
- std::vector<int> *v = &vv; // expected-warning {{local variable 'vv' is later invalidated}}
- v->push_back(42); // expected-note {{local variable 'vv' is invalidated here}}
- (void)*v; // expected-note {{later used here}}
+ std::vector<int> *v = &vv;
+ v->push_back(42);
+ (void)*v;
}
-// FIXME: Distinguish owner-borrow from content-borrow.
void LocalReferenceAliasObjectUseIsOk() {
std::vector<int> vv;
- std::vector<int> &v = vv; // expected-warning {{local variable 'vv' is later invalidated}}
- v.push_back(42); // expected-note {{local variable 'vv' is invalidated here}}
- (void)v; // expected-note {{later used here}}
+ std::vector<int> &v = vv;
+ v.push_back(42);
+ (void)v;
}
} // namespace ContainerObjectAliases
@@ -402,18 +406,8 @@ void SelfInvalidatingMap() {
// Therefore the following is safe in practice.
// On the other hand, std::flat_map (since C++23) does not provide pointer stability on
// insertion and following is unsafe for this container.
- // FIXME: The warnings below are false positives (self-invalidation of the Owner).
- // Modifying a container should not invalidate the container object itself.
- // To resolve this, we need to:
- // 1. Distinguish owner-borrow (borrowing the container object) from content-borrow (borrowing elements inside the container).
- // 2. Make AccessPaths more precise to reason at element/field granularity rather than treating the whole container as a single storage location.
- mp[1] = "42"; // expected-warning {{local variable 'mp' is later invalidated}} \
- // expected-note {{local variable 'mp' is invalidated here}} \
- // expected-note {{later used here}}
+ mp[1] = "42";
mp[2] = mp[1]; // expected-warning {{local variable 'mp' is later invalidated}} \
- // expected-warning {{local variable 'mp' is later invalidated}} \
- // expected-note {{local variable 'mp' is invalidated here}} \
- // expected-note {{later used here}} \
// expected-note {{local variable 'mp' is invalidated here}} \
// expected-note {{later used here}}
}
@@ -440,6 +434,18 @@ void reassign(std::string str, std::string str2) {
str = str2; // expected-note {{parameter 'str' is invalidated here}}
(void)view; // expected-note {{later used here}}
}
+
+void append_call(std::string str) {
+ std::string_view view = str; // expected-warning {{parameter 'str' is later invalidated}}
+ str.append("456"); // expected-note {{parameter 'str' is invalidated here}}
+ (void)view; // expected-note {{later used here}}
+}
+
+void replace_call(std::string str) {
+ std::string_view view = str; // expected-warning {{parameter 'str' is later invalidated}}
+ str.replace(0, 1, "456"); // expected-note {{parameter 'str' is invalidated here}}
+ (void)view; // expected-note {{later used here}}
+}
} // namespace Strings
// FIXME: This should be diagnosed as use-after-invalidation but with potential move.
@@ -456,14 +462,11 @@ struct S {
std::vector<std::string> strings1;
std::vector<std::string> strings2;
};
-// FIXME: Make Paths more precise to reason at field granularity.
-// Currently we only detect invalidations to direct declarations and not members.
void Invalidate1Use1IsInvalid() {
- // FIXME: Detect this.
S s;
- auto it = s.strings1.begin();
- s.strings1.push_back("1");
- *it;
+ auto it = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ s.strings1.push_back("1"); // expected-note {{local variable 's' is invalidated here}}
+ *it; // expected-note {{later used here}}
}
void Invalidate2Use1IsOk() {
S s;
@@ -472,24 +475,26 @@ void Invalidate2Use1IsOk() {
*it;
}
void ConditionalContainerInvalidatesIterator(bool flag) {
- // FIXME: Support invalidation through conditional lvalue expressions.
std::vector<int> v1, v2;
- auto it = v1.begin();
- (flag ? v1 : v2).push_back(42);
- (void)it;
+ auto it = v1.begin(); // expected-warning {{local variable 'v1' is later invalidated}}
+ (flag ? v1 : v2).push_back(42); // expected-note {{local variable 'v1' is invalidated here}}
+ (void)it; // expected-note {{later used here}}
}
void ConditionalFieldInvalidatesIterator(bool flag) {
- // FIXME: Support conditional invalidation through field expressions.
S s;
- auto it = s.strings1.begin();
- (flag ? s.strings1 : s.strings2).push_back("1");
- *it;
+ auto it1 = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ auto it2 = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ // FIXME: This note is inaccurate.
+ // It should say 's.strings1' is invalidated. Same for 's.strings2'.
+ (flag ? s.strings1 : s.strings2).push_back("1"); // expected-note 2 {{local variable 's' is invalidated here}}
+ *it1; // expected-note {{later used here}}
+ *it2; // expected-note {{later used here}}
}
void Invalidate1Use2ViaRefIsOk() {
S s;
auto it = s.strings2.begin();
auto& strings1 = s.strings1;
- strings1.push_back("1"); // OK
+ strings1.push_back("1");
*it;
}
void Invalidate1UseSIsOk() {
@@ -498,12 +503,11 @@ 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; // expected-warning {{local variable 's' is later invalidated}}
- p->push_back("1"); // expected-note {{local variable 's' is invalidated here}}
- (void)*p; // expected-note {{later used here}}
+ std::vector<std::string>* p = &s;
+ p->push_back("1");
+ (void)*p;
}
void IteratorFromPointerToContainerIsInvalidated() {
std::vector<std::string> s;
@@ -512,26 +516,50 @@ void IteratorFromPointerToContainerIsInvalidated() {
p->push_back("1"); // expected-note {{local variable 's' is 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) // expected-warning {{local variable 'subdirs' is later invalidated}} expected-note {{later used here}}
- path = std::string(); // expected-note {{local variable 'subdirs' is invalidated here}}
+ for (std::string& path : subdirs)
+ path = std::string();
+}
+
+struct SField { int a; int b;};
+void PointerToVectorElementField() {
+ std::vector<SField> v = {{1, 2}, {3, 4}};
+ int* ptr = &v[0].a; // expected-warning {{local variable 'v' is later invalidated}}
+ v.resize(100); // expected-note {{local variable 'v' is invalidated here}}
+ *ptr = 10; // expected-note {{later used here}}
+}
+
+void Invalidate1Use2ViaRefIsInvalid() {
+ S s;
+ auto it1 = s.strings1.begin();
+ auto it2 = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ auto& strings2 = s.strings2;
+ strings2.push_back("1"); // expected-note {{local variable 's' is invalidated here}}
+ *it1;
+ *it2; // expected-note {{later used here}}
}
+void InvalidateBothInASingleExpression(bool cond) {
+ S s;
+ auto it1 = s.strings1.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ auto it2 = s.strings2.begin(); // expected-warning {{local variable 's' is later invalidated}}
+ auto& both = cond ? s.strings1 : s.strings2;
+ both.push_back("1"); // expected-note 2 {{local variable 's' is invalidated here}}
+ *it1; // expected-note {{later used here}}
+ *it2; // expected-note {{later used here}}
+}
} // namespace ContainersAsFields
namespace InvalidatedField {
std::string StableString;
-// FIXME: Distinguish owner-borrow from interior-borrow.
struct SinkOwnerBorrow {
- std::string *dest_; // expected-note {{this field dangles}}
+ std::string *dest_;
- SinkOwnerBorrow(std::string *dest, int n) : dest_(dest) { // expected-warning {{parameter 'dest' escapes to the field 'dest_' and is later invalidated}}
+ SinkOwnerBorrow(std::string *dest, int n) : dest_(dest) {
if (n > 0)
- dest->clear(); // expected-note {{parameter 'dest' is invalidated here}}
+ dest->clear();
}
};
@@ -599,6 +627,25 @@ struct S {
strings.push_back("1");
}
};
+
+// FIXME: Detect invalidation of fields.
+// https://github.com/llvm/llvm-project/issues/180992
+struct InvalidateMemberFields {
+ InvalidateMemberFields();
+ void invalidateField() {
+ auto it = container.begin();
+ container.push_back("1");
+ *it;
+ }
+ void invalidateFieldRef() {
+ auto it = contiainerRef.begin();
+ contiainerRef.push_back("1");
+ *it;
+ }
+private:
+ std::vector<std::string> container;
+ std::vector<std::string>& contiainerRef;
+};
} // namespace InvalidatedField
namespace InvalidatedGlobal {
@@ -742,6 +789,13 @@ void MapSubscriptDoesNotInvalidate() {
*it;
}
+void MapOperatorBracket() {
+ std::unordered_map<int, int> m;
+ auto it = m.begin();
+ m[1];
+ *it;
+}
+
void PrintMax(const int& a, const int& b);
void MapSubscriptMultipleCallsDoesNotInvalidate(std::map<int, int> mp, int a, int b) {
@@ -749,14 +803,7 @@ void MapSubscriptMultipleCallsDoesNotInvalidate(std::map<int, int> mp, int a, in
}
void FlatMapSubscriptMultipleCallsInvalidate(std::flat_map<int, int> mp, int a, int b) {
- // FIXME: The duplicate warning below is a false positive caused by self-invalidation of the Owner 'mp'.
- // While the warning on the temporary reference returned by mp[a] is a true positive (it dangles),
- // the second warning on 'mp' itself is redundant and incorrect.
- // Resolving this requires distinguishing owner-borrow from content-borrow.
PrintMax(mp[a], mp[b]); // expected-warning {{parameter 'mp' is later invalidated}} \
- // expected-warning {{parameter 'mp' is later invalidated}} \
- // expected-note {{parameter 'mp' is invalidated here}} \
- // expected-note {{later used here}} \
// expected-note {{parameter 'mp' is invalidated here}} \
// expected-note {{later used here}}
}
@@ -896,7 +943,7 @@ struct StringOwner {
void member_destructor_invalidates_pointer() {
StringOwner owner = {"42", "43"};
const char *p = owner.s.data();
- owner.t.~basic_string(); // OK
+ owner.t.~basic_string();
(void)*p;
}
@@ -936,6 +983,38 @@ void invalid_after_ternary_reset(bool flag) {
} // namespace unique_ptr_invalidation
+
+
+namespace NestedContainers {
+// FIXME: Maybe come up with a access path representation to detect this.
+void InnerIteratorInvalidated() {
+ std::vector<std::vector<int>> v;
+ v.resize(1);
+ // FIXME: Detect this.
+ // We cannot differentiate between v.* and v.*.*.
+ // An annotation system along with lifetimebound could be helpful to describe the paths returned.
+ auto it = v[0].begin();
+ v[0].push_back(1);
+ *it;
+}
+void InnerIteratorInvalidatedOneUseOtherIsStillBad() {
+ // FIXME: Detect this.
+ std::vector<std::vector<int>> v;
+ v.resize(100);
+ auto it = v[0].begin();
+ v[1].push_back(1);
+ *it;
+}
+
+void OuterIteratorNotInvalidated() {
+ std::vector<std::vector<int>> v;
+ v.resize(1);
+ auto it = v.begin();
+ v[0].push_back(1);
+ it->clear(); // OK
+}
+} // namespace NestedContainers
+
namespace DeepFieldNesting {
struct Level3 {
std::vector<std::string> vec;
@@ -966,6 +1045,14 @@ void SiblingLevel2Ok() {
*it;
}
+// Modifying same vector: Invalid
+void SameVectorInvalid() {
+ Level1 obj;
+ auto it = obj.inner2_1.inner3_1.vec.begin(); // expected-warning {{local variable 'obj' is later invalidated}}
+ obj.inner2_1.inner3_1.vec.push_back("1"); // expected-note {{local variable 'obj' is invalidated here}}
+ *it; // expected-note {{later used here}}
+}
+
// Modifying sibling non-container field at Level 3: OK
void SiblingFieldLevel3Ok() {
Level1 obj;
@@ -974,7 +1061,31 @@ void SiblingFieldLevel3Ok() {
*it;
}
-// Modifying parent structure after use: OK
+// Modifying parent at Level 2 (reassigning struct): Invalid
+// FIXME: We don't currently detect invalidation of member containers when the parent struct is reassigned.
+void ParentLevel2Invalid() {
+ Level1 obj;
+ auto it = obj.inner2_1.inner3_1.vec.begin();
+ Level3 new_val;
+ obj.inner2_1.inner3_1 = new_val;
+ *it;
+}
+
+// Deep nesting with pointers: Invalid
+void PointerNestedInvalid(Level1* ptr) { // expected-warning {{parameter 'ptr' is later invalidated}}
+ auto it = ptr->inner2_1.inner3_1.vec.begin();
+ ptr->inner2_1.inner3_1.vec.push_back("1"); // expected-note {{parameter 'ptr' is invalidated here}}
+ *it; // expected-note {{later used here}}
+}
+
+// 7. Deep nesting with references: Invalid
+void ReferenceNestedInvalid(Level1& ref) { // expected-warning {{parameter 'ref' is later invalidated}}
+ auto it = ref.inner2_1.inner3_1.vec.begin();
+ ref.inner2_1.inner3_1.vec.push_back("1"); // expected-note {{parameter 'ref' is invalidated here}}
+ *it; // expected-note {{later used here}}
+}
+
+// 8. Modifying parent structure after use: OK
void ParentModifiedAfterUseOk() {
Level1 obj;
auto it = obj.inner2_1.inner3_1.vec.begin();
@@ -983,14 +1094,14 @@ void ParentModifiedAfterUseOk() {
obj.inner2_1.inner3_1 = new_val; // OK, because ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/207523
More information about the llvm-branch-commits
mailing list