[clang] [LifetimeSafety] Track origins through nested subobject fields (PR #232867)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 16 11:52:57 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis
Author: Balázs Benics (steakhal)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/232867.diff
5 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+1-1)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+27-10)
- (modified) clang/lib/Analysis/LifetimeSafety/Origins.cpp (+25-1)
- (modified) clang/test/Sema/LifetimeSafety/annotation-suggestions.cpp (+12)
- (modified) clang/test/Sema/LifetimeSafety/dangling-field.cpp (+39-6)
``````````diff
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index f5f84f27b5035..009f050710c35 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 6a805bb7be2d8..ea3582185063f 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 385b203d72845..4aaf2e2d0b730 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 34f3ad4f45de1..12ed6452ba845 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 68ed47687b29a..c2611a99384c8 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
}
};
``````````
</details>
https://github.com/llvm/llvm-project/pull/232867
More information about the cfe-commits
mailing list