https://github.com/Xazax-hun created https://github.com/llvm/llvm-project/pull/207346
A parameter annotated [[clang::lifetime_capture_by(X)]] promises its borrow is captured by X. When X names only concrete parameters (not `this`) yet the body captures the borrow into the enclosing object -- a store into a field of `this` -- the annotation does not describe the capture, so callers cannot tell the object now aliases the argument. Report it as a new validation, -Wlifetime-safety-capture-by-violation. Assisted-by: Claude Opus 4.8 From b9d63d03fb332f243b12497e9008fbf0cf7fc014 Mon Sep 17 00:00:00 2001 From: Gabor Horvath <[email protected]> Date: Fri, 3 Jul 2026 09:20:16 +0100 Subject: [PATCH] [LifetimeSafety] Verify lifetime_capture_by(X) against the body A parameter annotated [[clang::lifetime_capture_by(X)]] promises its borrow is captured by X. When X names only concrete parameters (not `this`) yet the body captures the borrow into the enclosing object -- a store into a field of `this` -- the annotation does not describe the capture, so callers cannot tell the object now aliases the argument. Report it as a new validation, -Wlifetime-safety-capture-by-violation. Assisted-by: Claude Opus 4.8 --- clang/docs/ReleaseNotes.md | 15 +++++ .../Analyses/LifetimeSafety/LifetimeSafety.h | 7 +++ clang/include/clang/Basic/DiagnosticGroups.td | 15 ++++- .../clang/Basic/DiagnosticSemaKinds.td | 7 +++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 50 ++++++++++++++- clang/lib/Sema/SemaLifetimeSafety.h | 11 ++++ .../LifetimeSafety/capture-by-violation.cpp | 62 +++++++++++++++++++ 7 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 clang/test/Sema/LifetimeSafety/capture-by-violation.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 6788763d0687e..57612ccbff8f4 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -601,6 +601,21 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the }; ``` +- Added `-Wlifetime-safety-capture-by-violation` to detect a parameter annotated + `[[clang::lifetime_capture_by(X)]]` (with `X` not naming `this`) whose borrow + is instead captured into the enclosing object -- stored into a field of + `this`. The annotation names a different capturer than the body uses. For + example: + + ```c++ + struct S { + const int *field; + void store(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { + field = &x; // warning: the borrow is captured into this object, but the annotation does not name 'this' + } + }; + ``` + - Improved `-Wassign-enum` performance by caching enum enumerator values. (#GH176454) - Fixed a false negative in `-Warray-bounds` where the warning was suppressed diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 80a23f18baebd..0d699110a5978 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -133,6 +133,13 @@ class LifetimeSafetySemaHelper { virtual void reportLifetimeboundViolation(const CXXMethodDecl *MDWithLifetimebound) {} + // Reports a parameter annotated [[clang::lifetime_capture_by(X)]] (with X not + // naming `this`) whose borrow is instead captured into the enclosing object + // -- stored into a field of `this`. The annotation names a different capturer + // than the body uses, so callers are told the wrong entity aliases the + // argument. + virtual void reportCaptureByViolation(const ParmVarDecl *PVD) {} + // Reports a member function definition that has [[clang::lifetimebound]] on // the implicit this parameter when the canonical declaration does not. virtual void reportMisplacedLifetimebound(WarningScope Scope, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 1c9d021317289..0abb622d5e269 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -700,6 +700,18 @@ Detects misuse of [[clang::noescape]] annotation where the parameter escapes (fo }]; } +def LifetimeSafetyCaptureByViolation + : DiagGroup<"lifetime-safety-capture-by-violation"> { + code Documentation = [{ +Detects a parameter annotated '[[clang::lifetime_capture_by(X)]]' (with X not +naming 'this') whose borrow is instead captured into the enclosing object -- +stored into a field of 'this'. The annotation names a different capturer than the +body actually uses, so callers are told the wrong entity aliases the argument. +Annotate the parameter '[[clang::lifetime_capture_by(this)]]' (or +'[[clang::lifetimebound]]') to match the body. + }]; +} + def LifetimeSafetyAnnotationPlacement : DiagGroup<"lifetime-safety-annotation-placement", [LifetimeSafetyInapplicableLifetimebound, @@ -712,7 +724,8 @@ Validates that lifetime annotations are placed on appropriate types and in appro def LifetimeSafetyAnnotationVerification : DiagGroup<"lifetime-safety-annotation-verification", [LifetimeSafetyNoescape, - LifetimeSafetyLifetimeboundViolation]> { + LifetimeSafetyLifetimeboundViolation, + LifetimeSafetyCaptureByViolation]> { code Documentation = [{ Verifies that function implementations adhere to their lifetime annotation contracts through dataflow analysis. }]; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 86b765fdf1fab..f2fcab8fffeb9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11049,6 +11049,13 @@ def warn_lifetime_safety_lifetimebound_violation InGroup<LifetimeSafetyLifetimeboundViolation>, DefaultIgnore; +def warn_lifetime_safety_capture_by_violation + : Warning<"the borrow from %select{an unnamed parameter|'%1'}0 is captured " + "into this object, but '[[clang::lifetime_capture_by]]' does not " + "name 'this'">, + InGroup<LifetimeSafetyCaptureByViolation>, + DefaultIgnore; + def warn_lifetime_safety_intra_tu_misplaced_lifetimebound : Warning<"'lifetimebound' attribute on this definition is not visible to callers before the definition; add it to the declaration instead">, InGroup<LifetimeSafetyIntraTUMisplacedLifetimebound>, diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index 746ebbfb15c39..3f3c575f17fef 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -60,6 +60,12 @@ class LifetimeChecker { llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap; llvm::DenseMap<AnnotationTarget, EscapingTarget> AnnotationWarningsMap; llvm::DenseMap<const ParmVarDecl *, EscapingTarget> NoescapeWarningsMap; + /// Parameters annotated [[clang::lifetime_capture_by(X)]] with X *not* naming + /// `this`, whose borrow nonetheless escapes into the enclosing object (a + /// store into a field of `this`) -- the body captures into `this`, which the + /// annotation's named capturer does not describe. Keyed by parameter to + /// de-duplicate. + llvm::DenseSet<const ParmVarDecl *> CaptureByFieldViolations; llvm::DenseSet<const Decl *> VerifiedLiftimeboundEscapes; const LoanPropagationAnalysis &LoanPropagation; const MovedLoansAnalysis &MovedLoans; @@ -104,6 +110,7 @@ class LifetimeChecker { issuePendingWarnings(); suggestAnnotations(); reportNoescapeViolations(); + reportCaptureByViolations(); reportLifetimeboundViolations(); reportMisplacedLifetimebound(); reportInapplicableLifetimebound(); @@ -114,6 +121,29 @@ class LifetimeChecker { inferAnnotations(); } + /// Returns true if \p PVD is annotated [[clang::lifetime_capture_by(X...)]] + /// whose capturer list names only concrete parameters -- not `this`, and not + /// the `unknown`/`global` sentinels. Only then does a store into a field of + /// `this` contradict the stated capturer (`this` already covers the object, + /// `unknown` may stand in for it, and `global` is a separate concern). + static bool capturesNamedNonThis(const ParmVarDecl *PVD) { + const auto *A = PVD->getAttr<LifetimeCaptureByAttr>(); + if (!A) + return false; + bool NamesConcrete = false; + for (int Idx : A->params()) + switch (Idx) { + case LifetimeCaptureByAttr::This: + case LifetimeCaptureByAttr::Unknown: + case LifetimeCaptureByAttr::Global: + case LifetimeCaptureByAttr::Invalid: + return false; + default: + NamesConcrete = true; // a concrete parameter capturer + } + return NamesConcrete; + } + /// Checks if an escaping origin holds a placeholder loan, indicating a /// missing [[clang::lifetimebound]] annotation or a violation of /// [[clang::noescape]]. @@ -165,9 +195,18 @@ class LifetimeChecker { for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); const AccessPath &AP = L->getAccessPath(); - if (const auto *PVD = AP.getAsPlaceholderParam()) + if (const auto *PVD = AP.getAsPlaceholderParam()) { + // A [[clang::lifetime_capture_by(X)]] parameter promises its borrow is + // captured by X. If the borrow instead escapes into the enclosing + // object -- a store into a field of `this` (FieldEscapeFact) -- while X + // names only concrete parameters (not `this`), the annotation does not + // describe this capture. (A capture into a genuine parameter capturer + // produces no field escape; `this`/`unknown` capturers are excluded.) + if (isa<FieldEscapeFact>(OEF) && !MovedAtEscape.lookup(LID) && + capturesNamedNonThis(PVD)) + CaptureByFieldViolations.insert(PVD); CheckParam(PVD, /*IsMoved=*/MovedAtEscape.lookup(LID)); - else if (const auto *MD = AP.getAsPlaceholderThis()) + } else if (const auto *MD = AP.getAsPlaceholderThis()) CheckImplicitThis(MD); } } @@ -428,6 +467,13 @@ class LifetimeChecker { } } + void reportCaptureByViolations() { + if (!SemaHelper) + return; + for (const ParmVarDecl *PVD : CaptureByFieldViolations) + SemaHelper->reportCaptureByViolation(PVD); + } + void reportLifetimeboundViolations() { if (!isa<FunctionDecl>(FD)) return; diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 3b9ea9eafbec5..bfc7283243b0c 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -61,6 +61,7 @@ inline bool IsLifetimeSafetyEnabled(Sema &S, const Decl *D) { diag::warn_lifetime_safety_dangling_global_moved, diag::warn_lifetime_safety_noescape_escapes, diag::warn_lifetime_safety_lifetimebound_violation, + diag::warn_lifetime_safety_capture_by_violation, diag::warn_lifetime_safety_cross_tu_misplaced_lifetimebound, diag::warn_lifetime_safety_intra_tu_misplaced_lifetimebound, diag::warn_lifetime_safety_invalidated_field, @@ -330,6 +331,16 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { << 2 << "" << Attr->getRange(); } + void reportCaptureByViolation(const ParmVarDecl *PVD) override { + const auto *Attr = PVD->getAttr<LifetimeCaptureByAttr>(); + SourceLocation Loc = Attr ? Attr->getLocation() : PVD->getLocation(); + SourceRange Range = Attr ? Attr->getRange() : PVD->getSourceRange(); + StringRef ParamName = PVD->getName(); + bool HasName = ParamName.size() > 0; + S.Diag(Loc, diag::warn_lifetime_safety_capture_by_violation) + << HasName << ParamName << Range; + } + void reportMisplacedLifetimebound(WarningScope Scope, const CXXMethodDecl *FDef, const CXXMethodDecl *FDecl) override { diff --git a/clang/test/Sema/LifetimeSafety/capture-by-violation.cpp b/clang/test/Sema/LifetimeSafety/capture-by-violation.cpp new file mode 100644 index 0000000000000..74edc273a4b16 --- /dev/null +++ b/clang/test/Sema/LifetimeSafety/capture-by-violation.cpp @@ -0,0 +1,62 @@ +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety-capture-by-violation -verify %s + +struct Cap { const int *p; }; + +struct S { + const int *field; + + // The annotation names parameter 'c', but the body captures the borrow into + // 'this' (a store into a field) -- the annotation does not name 'this'. + void lies(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { // expected-warning {{the borrow from 'x' is captured into this object, but '[[clang::lifetime_capture_by]]' does not name 'this'}} + field = &x; + } + + // Also captured into 'c', but still stored into a field of 'this' while the + // annotation omits 'this' -- flagged (the fix is to add 'this' to the list). + void incomplete(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { // expected-warning {{the borrow from 'x' is captured into this object, but '[[clang::lifetime_capture_by]]' does not name 'this'}} + c.p = &x; + field = &x; + } + + // Truthful: captured only into the named parameter 'c', no field-of-'this' + // store, nothing flagged. + void truthful(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { + c.p = &x; + } + + // capture_by(this) matches a store into a field of 'this'. + void captures_this(const int &x [[clang::lifetime_capture_by(this)]]) { + field = &x; + } + + // capture_by naming 'this' among other capturers still matches. + void captures_this_and_param(Cap &c, + const int &x [[clang::lifetime_capture_by(this, c)]]) { + field = &x; + } + + // capture_by(unknown) already covers an unspecified capturer, including + // 'this' -- not flagged. + void captures_unknown(const int &x [[clang::lifetime_capture_by(unknown)]]) { + field = &x; + } + + // capture_by(global) is a separate concern -- not flagged here. + void captures_global(const int &x [[clang::lifetime_capture_by(global)]]) { + field = &x; + } + + // A store into a field of another object (not 'this') is not a capture into + // 'this' and is not flagged. + void store_into_other(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { + c.p = &x; + } +}; + +// A constructor that stores a capture_by(c) parameter into a field of 'this'. +struct T { + const int *field; + T(Cap &c, const int &x [[clang::lifetime_capture_by(c)]]) { // expected-warning {{the borrow from 'x' is captured into this object, but '[[clang::lifetime_capture_by]]' does not name 'this'}} + field = &x; + } +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
