https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/204794
From 2bf90eb776b9c808f3435779fc1aee6467a96299 Mon Sep 17 00:00:00 2001 From: Gabor Horvath <[email protected]> Date: Fri, 19 Jun 2026 11:40:40 +0100 Subject: [PATCH] [LifetimeSafety] Recognize *this and base-casts of this as this-member access A member access on `this` was recognized only when the base was a direct CXXThisExpr after IgnoreParenImpCasts. But `this->m`, `(*this).m`, and `static_cast<Base*>(this)->m` / `((Base*)this)->m` all name the same field of the same object; the non-`this->` spellings routed the access to a fresh, disconnected origin, so a borrow stored through them was dropped: a dangling field bound through `(*this).p = &local` was silent where `this->p = &local` is caught. Add an isThisExpr helper recognizing `this`, `*this`, and derived-to-base / value-preserving pointer casts of `this` (which IgnoreParenImpCasts does not strip), and use it at both the field-origin mapping and the implicit-object field-use recognition. Assisted-by: Claude Opus 4.8 --- .../LifetimeSafety/LifetimeAnnotations.h | 7 ++ .../LifetimeSafety/FactsGenerator.cpp | 3 +- .../LifetimeSafety/LifetimeAnnotations.cpp | 27 +++++++ clang/lib/Analysis/LifetimeSafety/Origins.cpp | 2 +- .../Sema/LifetimeSafety/dangling-field.cpp | 76 +++++++++++++++++++ 5 files changed, 112 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index 47fcd5dbfd569..44df6b93afd7e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -22,6 +22,13 @@ bool isInStlNamespace(const Decl *D); bool isPointerLikeType(QualType QT); +/// Whether `E` denotes the enclosing object `this`: the `this` expression +/// itself, `*this`, or a derived-to-base / value-preserving pointer cast of +/// `this`. A member access through any of these names a field of the same +/// object, so they must be modeled identically. Parens and implicit casts are +/// looked through. +bool isThisExpr(const Expr *E); + /// Returns the most recent declaration of the method to ensure all /// lifetime-bound attributes from redeclarations are considered. const FunctionDecl *getDeclWithMergedLifetimeBoundAttrs(const FunctionDecl *FD); diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 8358c69a5165a..d652e00bda639 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -972,8 +972,7 @@ void FactsGenerator::handleImplicitObjectFieldUses(const Expr *Call, if (!MemberCall) return; - if (!isa_and_present<CXXThisExpr>( - MemberCall->getImplicitObjectArgument()->IgnoreImpCasts())) + if (!isThisExpr(MemberCall->getImplicitObjectArgument())) return; const auto *MD = dyn_cast<CXXMethodDecl>(FD); diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 6a52616c5d590..510a6639ec180 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -11,6 +11,8 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/OperatorKinds.h" @@ -124,6 +126,31 @@ bool isPointerLikeType(QualType QT) { return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); } +bool isThisExpr(const Expr *E) { + E = E->IgnoreParenImpCasts(); + if (isa<CXXThisExpr>(E)) + return true; + // A derived-to-base / value-preserving pointer cast of `this`, e.g. + // `static_cast<Base*>(this)` or `(Base*)this`: an explicit cast that + // IgnoreParenImpCasts does not strip. A member access through such a base + // view of `this` still names a field of the enclosing object. + if (const auto *CE = dyn_cast<CastExpr>(E)) + switch (CE->getCastKind()) { + case CK_DerivedToBase: + case CK_UncheckedDerivedToBase: + case CK_BaseToDerived: + case CK_NoOp: + return isThisExpr(CE->getSubExpr()); + default: + break; + } + // `*this`: a dereference of `this`. + if (const auto *UO = dyn_cast<UnaryOperator>(E); + UO && UO->getOpcode() == UO_Deref) + return isThisExpr(UO->getSubExpr()); + return false; +} + static bool isReferenceOrPointerLikeType(QualType QT) { return QT->isReferenceType() || isPointerLikeType(QT); } diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index c837f246fa17b..9af94b0485b1e 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -256,7 +256,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 && isThisExpr(ME->getBase())) ReferencedDecl = Field; if (ReferencedDecl) { OriginList *Head = nullptr; diff --git a/clang/test/Sema/LifetimeSafety/dangling-field.cpp b/clang/test/Sema/LifetimeSafety/dangling-field.cpp index b1eb31c4ee486..599be35144fa6 100644 --- a/clang/test/Sema/LifetimeSafety/dangling-field.cpp +++ b/clang/test/Sema/LifetimeSafety/dangling-field.cpp @@ -245,3 +245,79 @@ struct HasUniquePtrField { } }; } // namespace MakeUnique + +namespace ThisMemberAccessSpellings { +// `this->m`, `(*this).m`, and base-casts of `this` +// (`static_cast<Base*>(this)->m`, `((Base*)this)->m`) all name the same field of +// the same object, so a borrow stored through any spelling is routed to the +// field's origin and the dangling field is caught. +struct Holder { + std::string_view view; // expected-note 2 {{this field dangles}} + void via_arrow() { + std::string local; + this->view = local; // expected-warning {{stack memory associated with local variable 'local' escapes to the field 'view' which will dangle}} + } + void via_deref() { + std::string local; + (*this).view = local; // expected-warning {{stack memory associated with local variable 'local' escapes to the field 'view' which will dangle}} + } +}; + +struct Base { std::string_view q; }; // expected-note 2 {{this field dangles}} +struct Derived : Base { + void via_static_cast() { + std::string local; + static_cast<Base *>(this)->q = local; // expected-warning {{stack memory associated with local variable 'local' escapes to the field 'q' which will dangle}} + } + void via_cstyle_cast() { + std::string local; + ((Base *)this)->q = local; // expected-warning {{stack memory associated with local variable 'local' escapes to the field 'q' which will dangle}} + } +}; + +// A field use via a member call through any spelling of `this` keeps the borrow +// live (use-after-scope). +struct UseViaMethod { + std::string_view view; + void touch() const; + void via_arrow() { + { + std::string local; + view = local; // expected-warning {{local variable 'local' does not live long enough}} + } // expected-note {{destroyed here}} + this->touch(); // expected-note {{later used here}} + } + void via_deref() { + { + std::string local; + view = local; // expected-warning {{local variable 'local' does not live long enough}} + } // expected-note {{destroyed here}} + (*this).touch(); // expected-note {{later used here}} + } +}; + +// FIXME: These dangling stores are missed. this-member recognition is a +// structural match on the base spelling, so a base that only sometimes denotes +// `this` (a ternary) is not recognized; and a non-`this` object's field is not +// tracked at all, because the field-origin shortcut is keyed on the FieldDecl +// alone, which uniquely identifies an object only for `this`. Once fields carry +// their own origins/loans, this-member recognition should be loan-based (does +// the base carry the `this` loan?), which would also handle +// `(cond ? this : obj)->m` and stores into any object's field. +struct Ternary { + std::string_view view; + void store(Ternary *obj, bool cond) { + std::string local; + (cond ? this : obj)->view = local; // no-warning (FIXME: should warn for the `this` case) + } +}; +struct Plain { std::string_view view; }; +void local_object_field() { + Plain h; + { + std::string local; + h.view = local; // no-warning (FIXME: should warn, dangling field of a local object) + } + (void)h.view; +} +} // namespace ThisMemberAccessSpellings _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
