https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/232867
From 128e10d3cf9fabd71ae0e067ae6c08b06b174815 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Wed, 14 Oct 2026 01:40:48 +0100 Subject: [PATCH 1/5] [LifetimeSafety] Track origins through nested subobject fields The field-origin model only recognized a pointer stored as a direct member of `*this` (`this->field`). A pointer nested inside a member subobject was never modeled -- most importantly libc++'s `unique_ptr`, whose pointer lives in a `_LIBCPP_COMPRESSED_PAIR` that expands (on clang) to an anonymous struct. Its loan was lost when the field was read, so e.g. `unique_ptr::reset()` could not see that the deleter consumes the owned pointer. Generalize both halves of the model to `this`-rooted subobject access paths, traversing only *anonymous* struct/union subobjects. Their members are uniquely named within the class, so a FieldDecl-keyed origin cannot conflate two distinct subobjects: - OriginManager::getOrCreateList(Expr) shares origins with the terminal FieldDecl of a `this`-rooted chain of anonymous subobject accesses. Arrow accesses (`this->p->f`) are rejected, since `f` belongs to another object. - FactsGenerator::issueFieldPlaceholderLoans recurses into anonymous subobject fields, issuing a `$this.<field>` placeholder loan for each pointer leaf (e.g. unique_ptr's `__ptr_` inside the compressed-pair anonymous struct). Escape to a field of an anonymous subobject is now detected (dangling-field.cpp). Escape through a *named* subobject remains a documented FIXME: a FieldDecl-keyed origin cannot distinguish two members of the same type. Assisted-By: claude --- .../Analyses/LifetimeSafety/FactsGenerator.h | 2 +- .../LifetimeSafety/FactsGenerator.cpp | 37 ++++++++++----- clang/lib/Analysis/LifetimeSafety/Origins.cpp | 26 ++++++++++- .../LifetimeSafety/annotation-suggestions.cpp | 12 +++++ .../Sema/LifetimeSafety/dangling-field.cpp | 45 ++++++++++++++++--- 5 files changed, 104 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index f5f84f27b5035f..009f050710c35f 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -165,7 +165,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { bool escapesViaReturn(OriginID OID) const; llvm::SmallVector<Fact *> issuePlaceholderLoans(); - void issueFieldPlaceholderLoans(const CXXMethodDecl *MD, + void issueFieldPlaceholderLoans(const CXXRecordDecl *ClassDef, llvm::SmallVectorImpl<Fact *> &Facts); FactManager &FactMgr; AnalysisDeclContext &AC; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 6a805bb7be2d86..ea3582185063f6 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -1211,15 +1211,30 @@ void FactsGenerator::handleUse(const Expr *E) { // class and its bases. It represents the storage the field refers to, as // opposed to the field itself (covered by $this). void FactsGenerator::issueFieldPlaceholderLoans( - const CXXMethodDecl *MD, llvm::SmallVectorImpl<Fact *> &Facts) { - const CXXRecordDecl *ClassDecl = MD->getParent()->getDefinition(); - if (!ClassDecl) - return; + const CXXRecordDecl *ClassDef, llvm::SmallVectorImpl<Fact *> &Facts) { + assert(ClassDef); + assert(ClassDef->isThisDeclarationADefinition()); auto IssueForRecord = [&](const CXXRecordDecl *RD) { for (const FieldDecl *Field : RD->fields()) { - // Skip private fields of bases as they cannot be named in MD. - if (RD != ClassDecl && Field->getAccess() == AS_private) + // Skip private fields of bases as they cannot be named in ClassDef. + if (RD != ClassDef && Field->getAccess() == AS_private) continue; + + if (Field->isAnonymousStructOrUnion()) { + if (const CXXRecordDecl *Sub = Field->getType()->getAsCXXRecordDecl()) { + if (const CXXRecordDecl *SubDef = Sub->getDefinition()) { + // Anonymous records cannot have base classes and they can only have + // public fields. Sema rejects them with an error otherwise. + [[maybe_unused]] auto IsFieldPublic = [](const FieldDecl *F) { + return F->getAccess() == AS_public; + }; + assert(SubDef->getNumBases() == 0); + assert(llvm::all_of(SubDef->fields(), IsFieldPublic)); + issueFieldPlaceholderLoans(SubDef, Facts); + continue; + } + } + } OriginList *FieldList = getOriginsList(*Field); if (!FieldList) continue; @@ -1229,8 +1244,8 @@ void FactsGenerator::issueFieldPlaceholderLoans( } return true; }; - IssueForRecord(ClassDecl); - ClassDecl->forallBases(IssueForRecord); + IssueForRecord(ClassDef); + ClassDef->forallBases(IssueForRecord); } // Creates an IssueFact for a new placeholder loan for each pointer or reference @@ -1248,8 +1263,10 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { const Loan *L = FactMgr.getLoanMgr().createPlaceholderLoan(MD); PlaceholderLoanFacts.push_back( FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); - if (LSOpts.CheckFieldOriginEscape && !isa<CXXConstructorDecl>(MD)) - issueFieldPlaceholderLoans(MD, PlaceholderLoanFacts); + if (LSOpts.CheckFieldOriginEscape && !isa<CXXConstructorDecl>(MD)) { + if (const auto *ClassDef = MD->getParent()->getDefinition()) + issueFieldPlaceholderLoans(ClassDef, PlaceholderLoanFacts); + } } for (const ParmVarDecl *PVD : FD->parameters()) { OriginList *List = getOriginsList(*PVD); diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index 385b203d728455..4aaf2e2d0b7309 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -256,6 +256,30 @@ OriginList *OriginManager::getOrCreateList(const ValueDecl *D) { return DeclToList[D] = buildListForType(D->getType(), D); } +/// Returns true if the field named by \p ME denotes a subobject of `*this`, +/// reached as `this->f` or through anonymous-struct/union subobjects. +static bool isSubobjectOfThis(const MemberExpr *ME) { + const Expr *Base = ME->getBase()->IgnoreParenImpCasts(); + if (isa<CXXThisExpr>(Base)) + return true; + + // An arrow access dereferences a pointer, so the field belongs to another + // object, not a subobject of `*this`. + if (ME->isArrow()) + return false; + + // Traverse only *anonymous* subobjects: their members are uniquely named + // within the class, so the FieldDecl-keyed origin can't conflate two + // subobjects. A named subobject (`this->a.f`) could repeat the same inner + // type, so it is rejected. + if (const auto *BaseAsME = dyn_cast<MemberExpr>(Base)) { + const auto *BaseField = dyn_cast<FieldDecl>(BaseAsME->getMemberDecl()); + if (BaseField && BaseField->isAnonymousStructOrUnion()) + return isSubobjectOfThis(BaseAsME); + } + return false; +} + OriginList *OriginManager::getOrCreateList(const Expr *E) { if (auto *ParenIgnored = E->IgnoreParens(); ParenIgnored != E) return getOrCreateList(ParenIgnored); @@ -290,7 +314,7 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { ReferencedDecl = DRE->getDecl(); else if (auto *ME = dyn_cast<MemberExpr>(E)) if (auto *Field = dyn_cast<FieldDecl>(ME->getMemberDecl()); - Field && isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts())) + Field && isSubobjectOfThis(ME)) ReferencedDecl = Field; if (ReferencedDecl) { OriginList *Head = nullptr; diff --git a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp index 34f3ad4f45de1a..12ed6452ba845f 100644 --- a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp +++ b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp @@ -818,6 +818,18 @@ struct FieldTypes { } }; +namespace FieldOriginEscapeThroughAnonStruct { +struct FieldInAnonStruct { + struct { + int *ptr_; // expected-note {{if field 'ptr_' is owned by the class, mark the method [[clang::lifetimebound]]}} + }; + + int *get_ptr() const { // expected-warning {{return value depends on the object referenced by field 'ptr_', not on '*this'}} + return ptr_; // expected-note {{returned here}} + } +}; +} // namespace FieldOriginEscapeThroughAnonStruct + struct Base { int *pub_ptr_; // expected-note {{if field 'pub_ptr_' is owned by the class, mark the method [[clang::lifetimebound]]}} int *get_priv() const { // expected-warning {{return value depends on the object referenced by field 'priv_ptr_', not on '*this'}} diff --git a/clang/test/Sema/LifetimeSafety/dangling-field.cpp b/clang/test/Sema/LifetimeSafety/dangling-field.cpp index 68ed47687b29ae..c2611a99384c8f 100644 --- a/clang/test/Sema/LifetimeSafety/dangling-field.cpp +++ b/clang/test/Sema/LifetimeSafety/dangling-field.cpp @@ -196,27 +196,60 @@ struct MemberSetters { } }; -// FIXME: Detect escape to field of field. -struct IndirectEscape{ +// FIXME: Escape to a field of a *named* subobject is not detected; only +// anonymous struct/union subobjects are traversed. +struct IndirectEscape { struct { const char *p; } b; - + void foo() { std::string s; b.p = s.data(); } }; -// FIXME: Detect escape to field of field. +// FIXME: Not detected either, and this shows *why* only anonymous subobjects are +// traversed: the named members `a` and `b` share the terminal field `Inner::p`, +// so a FieldDecl-keyed origin could not tell `a.p` apart from `b.p` if it +// traversed named subobjects. +struct IndirectEscapeSameType { + struct Inner { + const char *p; + }; + Inner a; + Inner b; + + void foo() { + std::string s; + a.p = s.data(); + } +}; + +// Escape to a field of an anonymous subobject is detected (members of an +// anonymous struct/union are uniquely named within the enclosing class). struct IndirectEscape2 { struct { - const char *p; + const char *p; // expected-note {{this field dangles}} }; void foo() { std::string s; - p = s.data(); + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + } +}; + +// A field reached through a *reference* member is NOT a subobject of `*this` +// (the reference binds to external storage), so it is not attributed to a +// `$this` field: no false diagnostic. +struct IndirectEscapeThroughReference { + struct Inner { const char *p; }; + Inner &r; + IndirectEscapeThroughReference(Inner &i) : r(i) {} + + void foo() { + std::string s; + r.p = s.data(); // no diagnostic: r.p is not a subobject of *this } }; From e402c38cf34ee6859bc2fc92e4bff361fd592571 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Fri, 16 Oct 2026 21:25:26 +0100 Subject: [PATCH 2/5] NFC Simplify by stripping dead condition --- clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index ea3582185063f6..4296de878db706 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -1264,8 +1264,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { PlaceholderLoanFacts.push_back( FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); if (LSOpts.CheckFieldOriginEscape && !isa<CXXConstructorDecl>(MD)) { - if (const auto *ClassDef = MD->getParent()->getDefinition()) - issueFieldPlaceholderLoans(ClassDef, PlaceholderLoanFacts); + const CXXRecordDecl *RecDef = MD->getParent()->getDefinition(); + assert(RecDef && "class methods have a defined parent"); + issueFieldPlaceholderLoans(RecDef, PlaceholderLoanFacts); } } for (const ParmVarDecl *PVD : FD->parameters()) { From c0b0286f1e9a391d155af5deae8fb99e2b01afff Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Fri, 16 Oct 2026 21:42:02 +0100 Subject: [PATCH 3/5] Add extra tests --- .../LifetimeSafety/annotation-suggestions.cpp | 34 +++++++++++++++++ .../Sema/LifetimeSafety/dangling-field.cpp | 37 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp index 12ed6452ba845f..11c6400ca81da4 100644 --- a/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp +++ b/clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp @@ -828,6 +828,40 @@ struct FieldInAnonStruct { return ptr_; // expected-note {{returned here}} } }; + +struct FieldInNestedAnonStruct { + struct { + struct { + int *ptr_; // expected-note {{if field 'ptr_' is owned by the class, mark the method [[clang::lifetimebound]]}} + }; + }; + + int *get_ptr() const { // expected-warning {{return value depends on the object referenced by field 'ptr_', not on '*this'}} + return ptr_; // expected-note {{returned here}} + } +}; + +struct MultiFieldAnonStructSuggestsOnlyForReturnedField { + struct { + int *a_; + int *b_; // expected-note {{if field 'b_' is owned by the class, mark the method [[clang::lifetimebound]]}} + }; + + int *get_b() const { // expected-warning {{return value depends on the object referenced by field 'b_', not on '*this'}} + return b_; // expected-note {{returned here}} + } +}; + +struct MultiFieldAnonUnionSuggestsOnlyForReturnedField { + union { + int *a_; + int *b_; // expected-note {{if field 'b_' is owned by the class, mark the method [[clang::lifetimebound]]}} + }; + + int *get_b() const { // expected-warning {{return value depends on the object referenced by field 'b_', not on '*this'}} + return b_; // expected-note {{returned here}} + } +}; } // namespace FieldOriginEscapeThroughAnonStruct struct Base { diff --git a/clang/test/Sema/LifetimeSafety/dangling-field.cpp b/clang/test/Sema/LifetimeSafety/dangling-field.cpp index c2611a99384c8f..c557f176260b2e 100644 --- a/clang/test/Sema/LifetimeSafety/dangling-field.cpp +++ b/clang/test/Sema/LifetimeSafety/dangling-field.cpp @@ -253,6 +253,43 @@ struct IndirectEscapeThroughReference { } }; +struct NestedAnonStructEscape { + struct { + struct { + const char *p; // expected-note {{this field dangles}} + }; + }; + + void foo() { + std::string s; + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + } +}; + +struct MultiFieldAnonStructEscapesOnlyWrittenField { + struct { + const char *p; // expected-note {{this field dangles}} + const char *q; + }; + + void foo() { + std::string s; + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + } +}; + +struct AnonUnionEscapesOnlyWrittenField { + union { + const char *p; // expected-note {{this field dangles}} + const char *q; + }; + + void foo() { + std::string s; + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + } +}; + namespace DanglingPointerFieldInBaseClass { struct BaseWithPointer { std::string_view view; // expected-note {{this field dangles}} From 46def12159b5e954ac42636b23aa9c1c35e2313b Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Fri, 16 Oct 2026 22:51:46 +0100 Subject: [PATCH 4/5] Nest the assert --- clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 4296de878db706..47836fef6536a6 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -1212,7 +1212,7 @@ void FactsGenerator::handleUse(const Expr *E) { // opposed to the field itself (covered by $this). void FactsGenerator::issueFieldPlaceholderLoans( const CXXRecordDecl *ClassDef, llvm::SmallVectorImpl<Fact *> &Facts) { - assert(ClassDef); + assert(ClassDef && "class methods have a defined parent"); assert(ClassDef->isThisDeclarationADefinition()); auto IssueForRecord = [&](const CXXRecordDecl *RD) { for (const FieldDecl *Field : RD->fields()) { @@ -1263,11 +1263,9 @@ llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { const Loan *L = FactMgr.getLoanMgr().createPlaceholderLoan(MD); PlaceholderLoanFacts.push_back( FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); - if (LSOpts.CheckFieldOriginEscape && !isa<CXXConstructorDecl>(MD)) { - const CXXRecordDecl *RecDef = MD->getParent()->getDefinition(); - assert(RecDef && "class methods have a defined parent"); - issueFieldPlaceholderLoans(RecDef, PlaceholderLoanFacts); - } + if (LSOpts.CheckFieldOriginEscape && !isa<CXXConstructorDecl>(MD)) + issueFieldPlaceholderLoans(MD->getParent()->getDefinition(), + PlaceholderLoanFacts); } for (const ParmVarDecl *PVD : FD->parameters()) { OriginList *List = getOriginsList(*PVD); From 45b964487e14a8548c3f0eb6e2af26d03582343f Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Fri, 16 Oct 2026 22:52:16 +0100 Subject: [PATCH 5/5] Add tests for anonymous-union field aliasing --- .../Sema/LifetimeSafety/dangling-field.cpp | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/clang/test/Sema/LifetimeSafety/dangling-field.cpp b/clang/test/Sema/LifetimeSafety/dangling-field.cpp index c557f176260b2e..3a4ee83046f5ed 100644 --- a/clang/test/Sema/LifetimeSafety/dangling-field.cpp +++ b/clang/test/Sema/LifetimeSafety/dangling-field.cpp @@ -290,6 +290,37 @@ struct AnonUnionEscapesOnlyWrittenField { } }; +// FIXME: Union members alias one storage, but each keeps an independent +// FieldDecl-keyed origin, so writing both members reports both. +struct AnonUnionAliasingWarnsPerMember { + union { + const char *p; // expected-note {{this field dangles}} + const char *q; // expected-note {{this field dangles}} + }; + + void foo() { + std::string s; + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + q = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'q' which will dangle}} + } +}; + +// FIXME: false positive: writing a non-dangling pointer through `q` overwrites +// the shared union storage, but `p`'s origin is tracked independently so `p` is +// still reported. Union aliasing is not modeled. +struct AnonUnionSafeOverwriteStillWarns { + union { + const char *p; // expected-note {{this field dangles}} + const char *q; + }; + + void foo() { + std::string s; + p = s.data(); // expected-warning {{stack memory associated with local variable 's' escapes to the field 'p' which will dangle}} + q = kGlobal.data(); // safe overwrite of the shared storage; `p` is still (wrongly) reported + } +}; + namespace DanglingPointerFieldInBaseClass { struct BaseWithPointer { std::string_view view; // expected-note {{this field dangles}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
