https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/207523
>From 04d5591d430df9b57b807df7be059acaa3c33742 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Sat, 4 Jul 2026 16:28:24 +0000 Subject: [PATCH] [LifetimeSafety] Support container interior paths and invalidations 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 --- clang/lib/Analysis/LifetimeSafety/Checker.cpp | 11 +- .../LifetimeSafety/FactsGenerator.cpp | 34 ++- .../LifetimeSafety/Inputs/lifetime-analysis.h | 9 +- .../Sema/LifetimeSafety/invalidations.cpp | 288 ++++++++++++++---- .../unittests/Analysis/LifetimeSafetyTest.cpp | 176 +++++++---- 5 files changed, 372 insertions(+), 146 deletions(-) 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 aae30bb220c27..01fae20422441 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -956,11 +956,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>( @@ -1070,7 +1065,27 @@ 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; +} void FactsGenerator::handleFunctionCall(const Expr *Call, const FunctionDecl *FD, @@ -1150,8 +1165,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // GSL construction creates a view that borrows from arguments. // Only flow the outer origin because inner lengths may mismatch. CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( - CallList->getOuterOriginID(), ArgList->getOuterOriginID(), - KillSrc)); + CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc, + PathElement::getInterior())); KillSrc = false; } else if (IsArgLifetimeBound(I)) { // Only flow the outer origin here. For lifetimebound args in @@ -1178,7 +1193,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // pointer/reference itself must not outlive the arguments. This // only constrains the top-level origin. CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( - CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc)); + CallList->getOuterOriginID(), ArgList->getOuterOriginID(), KillSrc, + getPathElementForLifetimeBoundArg(FD, I, Args[I]))); KillSrc = false; } } 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 'it' is no longer used! } -// Pointers with sibling modification: OK +// 9. Pointers with sibling modification: OK void PointerSiblingLevel3Ok(Level1* ptr) { auto it = ptr->inner2_1.inner3_1.vec.begin(); ptr->inner2_1.inner3_2.vec.push_back("1"); // OK *it; } -// References with sibling modification: OK +// 10. References with sibling modification: OK void ReferenceSiblingLevel3Ok(Level1& ref) { auto it = ref.inner2_1.inner3_1.vec.begin(); ref.inner2_1.inner3_2.vec.push_back("1"); // OK @@ -1011,3 +1122,50 @@ void TestStructVsField(S& s) { } } // namespace StructFieldDisambiguation +namespace GlobalFieldEscapes { +std::string StableString; +struct GlobalStruct { + std::string_view sv; +}; +GlobalStruct g_struct; + +// FIXME: Fields of global variables are treated as FieldDecl origins, so they escape +// as FieldEscapeFact instead of GlobalEscapeFact, causing us to miss the escape warning. +void TestGlobalFieldInvalidated() { + std::string s; + g_struct.sv = s; + s.clear(); +} + +// Even after fixing the global field escape tracking, this should be OK because g_struct is reassigned. +void GlobalReassignedBeforeInvalidationOk() { + std::string s; + g_struct.sv = s; + g_struct.sv = StableString; // Reassigned! + s.clear(); // OK +} +} // namespace GlobalFieldEscapes + +namespace PointerToStructField { +struct S { + std::vector<int> v; +}; + +void TestPointerToField(S* s) { // expected-warning {{parameter 's' is later invalidated}} + auto it = s->v.begin(); + s->v.push_back(1); // expected-note {{parameter 's' is invalidated here}} + *it; // expected-note {{later used here}} +} + +struct S2 { + std::vector<int> v1; + std::vector<int> v2; +}; + +void TestPointerToSiblingFieldOk(S2* s) { + auto it = s->v1.begin(); + s->v2.push_back(1); // OK: different field + *it; +} +} // namespace PointerToStructField + diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index e13b8f88cb834..a5f9004dc2810 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -289,8 +289,8 @@ class OriginsInfo { /// /// This matcher is intended to be used with an \c OriginInfo object. /// -/// \param LoanVars A vector of strings, where each string is the name of a -/// variable expected to be the source of a loan. +/// \param LoanPathStrs A vector of strings, where each string is the +/// string representation of an access path of a loan. /// \param Annotation A string identifying the program point (created with /// POINT()) where the check should be performed. MATCHER_P2(HasLoansToImpl, LoanPathStrs, Annotation, "") { @@ -779,7 +779,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerSimpleLoan) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerConstructFromOwner) { @@ -795,12 +795,12 @@ TEST_F(LifetimeAnalysisTest, GslPointerConstructFromOwner) { POINT(p1); } )"); - EXPECT_THAT(Origin("a"), HasLoansTo({"al"}, "p1")); - EXPECT_THAT(Origin("b"), HasLoansTo({"bl"}, "p1")); - EXPECT_THAT(Origin("c"), HasLoansTo({"cl"}, "p1")); - EXPECT_THAT(Origin("d"), HasLoansTo({"dl"}, "p1")); - EXPECT_THAT(Origin("e"), HasLoansTo({"el"}, "p1")); - EXPECT_THAT(Origin("f"), HasLoansTo({"fl"}, "p1")); + EXPECT_THAT(Origin("a"), HasLoansTo({"al.*"}, "p1")); + EXPECT_THAT(Origin("b"), HasLoansTo({"bl.*"}, "p1")); + EXPECT_THAT(Origin("c"), HasLoansTo({"cl.*"}, "p1")); + EXPECT_THAT(Origin("d"), HasLoansTo({"dl.*"}, "p1")); + EXPECT_THAT(Origin("e"), HasLoansTo({"el.*"}, "p1")); + EXPECT_THAT(Origin("f"), HasLoansTo({"fl.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) { @@ -815,11 +815,11 @@ TEST_F(LifetimeAnalysisTest, GslPointerConstructFromView) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("q"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("q"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) { @@ -830,7 +830,7 @@ TEST_F(LifetimeAnalysisTest, GslPointerInConditionalOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("v"), HasLoansTo({"a", "b"}, "p1")); + EXPECT_THAT(Origin("v"), HasLoansTo({"a.*", "b.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { @@ -844,10 +844,10 @@ TEST_F(LifetimeAnalysisTest, ExtraParenthesis) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("p"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, ViewFromTemporary) { @@ -874,8 +874,8 @@ TEST_F(LifetimeAnalysisTest, GslPointerWithConstAndAuto) { POINT(p1); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*"}, "p1")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p1")); } @@ -895,9 +895,9 @@ TEST_F(LifetimeAnalysisTest, GslPointerPropagation) { } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"a"}, "p2")); - EXPECT_THAT(Origin("z"), HasLoansTo({"a"}, "p3")); + EXPECT_THAT(Origin("x"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("z"), HasLoansTo({"a.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, GslPointerReassignment) { @@ -916,9 +916,9 @@ TEST_F(LifetimeAnalysisTest, GslPointerReassignment) { } )"); - EXPECT_THAT(Origin("v"), HasLoansTo({"safe"}, "p1")); - EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p2")); - EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe"}, "p3")); + EXPECT_THAT(Origin("v"), HasLoansTo({"safe.*"}, "p1")); + EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe.*"}, "p2")); + EXPECT_THAT(Origin("v"), HasLoansTo({"unsafe.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) { @@ -942,8 +942,8 @@ TEST_F(LifetimeAnalysisTest, GslPointerConversionOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("x"), HasLoansTo({"xl"}, "p1")); - EXPECT_THAT(Origin("y"), HasLoansTo({"yl"}, "p1")); + EXPECT_THAT(Origin("x"), HasLoansTo({"xl.*"}, "p1")); + EXPECT_THAT(Origin("y"), HasLoansTo({"yl.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundSimple) { @@ -959,10 +959,10 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundSimple) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); // The origin of v2 should now contain the loan to 'o' from v1. - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - EXPECT_THAT(Origin("v3"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"b.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunctionOfAView) { @@ -979,7 +979,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMemberFunctionOfAView) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o.*"}, "p1")); // The call v1.pass() is bound to 'v1'. EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p2")); } @@ -1011,11 +1011,11 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMultipleArgs) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o1"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o2"}, "p2")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o1.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"o2.*"}, "p2")); // v3 should have loans from both v1 and v2, demonstrating the union of // loans. - EXPECT_THAT(Origin("v3"), HasLoansTo({"o1", "o2"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"o1.*", "o2.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundMixedArgs) { @@ -1031,10 +1031,10 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundMixedArgs) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o1"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"o2"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o1.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"o2.*"}, "p1")); // v3 should only have loans from v1, as v2 is not lifetimebound. - EXPECT_THAT(Origin("v3"), HasLoansTo({"o1"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"o1.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundChainOfViews) { @@ -1050,9 +1050,9 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundChainOfViews) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"obj"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"obj.*"}, "p1")); // v2 should inherit the loan from v1 through the chain of calls. - EXPECT_THAT(Origin("v2"), HasLoansTo({"obj"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"obj.*"}, "p2")); } TEST_F(LifetimeAnalysisTest, LifetimeboundRawPointerParameter) { @@ -1079,7 +1079,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundRawPointerParameter) { EXPECT_THAT(Origin("v"), HasLoansTo({"a"}, "p1")); EXPECT_THAT(Origin("ptr1"), HasLoansTo({"b"}, "p2")); EXPECT_THAT(Origin("ptr2"), HasLoansTo({"b"}, "p2")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"c"}, "p3")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"c.*"}, "p3")); } TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { @@ -1092,7 +1092,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundConstRefViewParameter) { POINT(p1); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"o"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"o.*"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({"v1"}, "p1")); } @@ -1127,12 +1127,31 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundReturnReference) { POINT(p3); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"a"}, "p2")); - - EXPECT_THAT(Origin("v3"), HasLoansTo({"a"}, "p2")); + // All views have a single interior path (`.*`) because the implementation + // prevents accumulation of multiple `.*` suffixes (see + // DoNotAddMultipleInteriors test). When `Identity(v1)` returns a `MyObj&` + // with loan `a.*`, constructing `View v2` from it would normally add another + // `.*`, but the implementation actively prevents duplication of '.*'. + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("v3"), HasLoansTo({"a.*"}, "p2")); + EXPECT_THAT(Origin("v4"), HasLoansTo({"c.*"}, "p3")); +} - EXPECT_THAT(Origin("v4"), HasLoansTo({"c"}, "p3")); +TEST_F(LifetimeAnalysisTest, DoNotAddMultipleInteriors) { + SetupTest(R"( + const MyObj& Identity(View v [[clang::lifetimebound]]); + void target() { + MyObj a; + View v = a; + for (int i = 0; i < 10; ++i) { + const MyObj& b = Identity(v); + v = Identity(b); + POINT(p1); + } + } + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { @@ -1149,7 +1168,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnRef) { POINT(p2); } )"); - EXPECT_THAT(Origin("v1"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("v1"), HasLoansTo({"a.*"}, "p1")); EXPECT_THAT(Origin("v2"), HasLoansTo({}, "p2")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); } @@ -1173,7 +1192,7 @@ TEST_F(LifetimeAnalysisTest, LifetimeboundTemplateFunctionReturnVal) { )"); EXPECT_THAT(Origin("v1"), HasLoanToATemporary("p1")); - EXPECT_THAT(Origin("v2"), HasLoansTo({"b"}, "p2")); + EXPECT_THAT(Origin("v2"), HasLoansTo({"b.*"}, "p2")); EXPECT_THAT(Origin("v3"), HasLoansTo({"v2"}, "p2")); // View temporary on RHS is lifetime-extended. EXPECT_THAT(Origin("v4"), HasLoansTo({}, "p2")); @@ -1212,6 +1231,16 @@ TEST_F(LifetimeAnalysisTest, NestedFieldAccess) { EXPECT_THAT(Origin("p2"), HasLoansTo({"o.f.val"}, "b")); } +TEST_F(LifetimeAnalysisTest, PlaceholderInterior) { + SetupTest(R"( + void target(const MyObj& p) { + View v = p; + POINT(a); + } + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"$p.*"}, "a")); +} + TEST_F(LifetimeAnalysisTest, PlaceholderParamField) { SetupTest(R"( struct S { int val; }; @@ -1236,6 +1265,19 @@ TEST_F(LifetimeAnalysisTest, PlaceholderThisField) { EXPECT_THAT(Origin("p1"), HasLoansTo({"$this.f"}, "a")); } +TEST_F(LifetimeAnalysisTest, PlaceholderThisInterior) { + SetupTest(R"( + struct S { + MyObj o; + void target() { + View v = o; + POINT(a); + } + }; + )"); + EXPECT_THAT(Origin("v"), HasLoansTo({"$this.o.*"}, "a")); +} + TEST_F(LifetimeAnalysisTest, PlaceholderThisNestedField) { SetupTest(R"( struct S1 { @@ -1717,7 +1759,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_STLBegin) { POINT(p1); } )"); - EXPECT_THAT(Origin("it"), HasLoansTo({"vec"}, "p1")); + EXPECT_THAT(Origin("it"), HasLoansTo({"vec.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_OwnerDeref) { @@ -1735,7 +1777,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_OwnerDeref) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_Value) { @@ -1753,7 +1795,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_Value) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"opt"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"opt.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_UniquePtr_Get) { @@ -1771,7 +1813,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_UniquePtr_Get) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"up"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"up.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_ConversionOperator) { @@ -1790,7 +1832,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_ConversionOperator) { POINT(p1); } )"); - EXPECT_THAT(Origin("ptr"), HasLoansTo({"owner"}, "p1")); + EXPECT_THAT(Origin("ptr"), HasLoansTo({"owner.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_MapFind) { @@ -1809,7 +1851,7 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_MapFind) { POINT(p1); } )"); - EXPECT_THAT(Origin("it"), HasLoansTo({"m"}, "p1")); + EXPECT_THAT(Origin("it"), HasLoansTo({"m.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_GSLPointerArg) { @@ -1855,11 +1897,11 @@ TEST_F(LifetimeAnalysisTest, TrackImplicitObjectArg_GSLPointerArg) { POINT(end); } )"); - EXPECT_THAT(Origin("sv1"), HasLoansTo({"s1"}, "end")); - EXPECT_THAT(Origin("sv2"), HasLoansTo({"s2"}, "end")); - EXPECT_THAT(Origin("sv3"), HasLoansTo({"s3"}, "end")); - EXPECT_THAT(Origin("sv4"), HasLoansTo({"s4"}, "end")); - EXPECT_THAT(Origin("sv5"), HasLoansTo({"s5"}, "end")); + EXPECT_THAT(Origin("sv1"), HasLoansTo({"s1.*"}, "end")); + EXPECT_THAT(Origin("sv2"), HasLoansTo({"s2.*"}, "end")); + EXPECT_THAT(Origin("sv3"), HasLoansTo({"s3.*"}, "end")); + EXPECT_THAT(Origin("sv4"), HasLoansTo({"s4.*"}, "end")); + EXPECT_THAT(Origin("sv5"), HasLoansTo({"s5.*"}, "end")); } // ========================================================================= // @@ -1885,7 +1927,7 @@ TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdBegin) { POINT(p1); } )"); - EXPECT_THAT(Origin("it"), HasLoansTo({"vec"}, "p1")); + EXPECT_THAT(Origin("it"), HasLoansTo({"vec.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdData) { @@ -1906,7 +1948,7 @@ TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdData) { POINT(p1); } )"); - EXPECT_THAT(Origin("p"), HasLoansTo({"vec"}, "p1")); + EXPECT_THAT(Origin("p"), HasLoansTo({"vec.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdAnyCast) { @@ -1924,7 +1966,7 @@ TEST_F(LifetimeAnalysisTest, TrackFirstArgument_StdAnyCast) { POINT(p1); } )"); - EXPECT_THAT(Origin("r"), HasLoansTo({"a"}, "p1")); + EXPECT_THAT(Origin("r"), HasLoansTo({"a.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, DerivedToBaseThisArg) { @@ -1948,7 +1990,7 @@ TEST_F(LifetimeAnalysisTest, DerivedToBaseThisArg) { POINT(p1); } )"); - EXPECT_THAT(Origin("view"), HasLoansTo({"my_obj_or"}, "p1")); + EXPECT_THAT(Origin("view"), HasLoansTo({"my_obj_or.*"}, "p1")); } TEST_F(LifetimeAnalysisTest, DerivedViewWithNoAnnotation) { @@ -1983,7 +2025,7 @@ TEST_F(LifetimeAnalysisTest, LambdaCaptureViewByValue) { POINT(after_lambda); } )"); - EXPECT_THAT(Origin("lambda"), HasLoansTo({"obj"}, "after_lambda")); + EXPECT_THAT(Origin("lambda"), HasLoansTo({"obj.*"}, "after_lambda")); } TEST_F(LifetimeAnalysisTest, LambdaInitCaptureRawPointerByValue) { @@ -2007,7 +2049,7 @@ TEST_F(LifetimeAnalysisTest, LambdaInitCaptureViewByValue) { POINT(after_lambda); } )"); - EXPECT_THAT(Origin("lambda"), HasLoansTo({"obj"}, "after_lambda")); + EXPECT_THAT(Origin("lambda"), HasLoansTo({"obj.*"}, "after_lambda")); } // ========================================================================= // _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
