llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Kashika Akhouri (kashika0112)
<details>
<summary>Changes</summary>
This PR adds support for `[[clang::lifetime_capture_by(...)]]` attribute on
method declarations (which denote that the implicit this object is being
captured).
- The PR checks the Type attribute (attached to the method's `TypeSourceInfo`)
to check for lifetime_capture_by attribute on method declaration using a new
helper method: `getCaptureByAttrFromFunctionType`.
- Adds the support for capture_by on method declarations in
`handleLifetimeCaptureBy`.
Example:
```cpp
struct X {
} x;
struct S {
void capture(X &x) [[clang::lifetime_capture_by(x)]];
};
void use() {
{
S s;
s.capture(x);
}
(void)x;
}
```
Warnings Generated
```cpp
c4.cpp:16:9: warning: local variable 's' does not live long enough
[-Wlifetime-safety-use-after-scope]
16 | s.capture(x);
| ^
c4.cpp:17:5: note: local variable 's' is destroyed here
17 | }
| ^
c4.cpp:18:11: note: later used here
18 | (void)x;
| ^
```
- The PR extends on [Support [[clang::lifetime_capture_by(X)]] in Plain
Containers](https://github.com/llvm/llvm-project/pull/204361) to support
method-level attributes in `collectCaptureBy` ensuring that parameters
capturing this are properly registered as lifetime-tracked types.
- This PR removes `CallArgs` shifting logic from `handleLifetimeCaptureBy` in
`FactsGenerator.cpp` to fix crashes. The `CapturingArgIdx` generated by the
attribute parser already accounts for the offset of the implicit this, which
removed the need for CallArgs.
---
Full diff: https://github.com/llvm/llvm-project/pull/219423.diff
5 Files Affected:
- (modified)
clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h (+6)
- (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+22-18)
- (modified) clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp (+18)
- (modified) clang/lib/Analysis/LifetimeSafety/Origins.cpp (+21-15)
- (modified) clang/test/Sema/LifetimeSafety/capture-by.cpp (+48-5)
``````````diff
diff --git
a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
index 6f1259fa867dc..dd60ea2ee0660 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
@@ -60,6 +60,12 @@ getImplicitObjectParamLifetimeBoundAttr(const FunctionDecl
*FD);
/// method or because it's a normal assignment operator.
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD);
+/// Check if a function has a lifetime_capture_by attribute on its declaration
+/// or its function type (which represents the implicit 'this' parameter for
+/// methods). Returns the attribute if found, nullptr otherwise.
+const LifetimeCaptureByAttr *
+getCaptureByAttrFromFunctionType(const FunctionDecl *FD);
+
using LifetimeBoundParamInfo =
llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *>;
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index ce3039528b283..0c2f414409be4 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -1033,22 +1033,31 @@ void FactsGenerator::handleLifetimeCaptureBy(const
FunctionDecl *FD,
const auto *Method = dyn_cast<CXXMethodDecl>(FD);
bool IsInstance =
Method && Method->isInstance() && !isa<CXXConstructorDecl>(FD);
- auto getParamDeclAt = [FD, IsInstance](unsigned I) -> const ParmVarDecl * {
+ auto getArgCaptureByAttr =
+ [FD, IsInstance](
+ unsigned I) -> std::pair<const LifetimeCaptureByAttr *, QualType> {
if (IsInstance) {
- // FIXME: Add support for I == 0 i.e. capture_by on function declarations
- if (I > 0 && I - 1 < FD->getNumParams())
- return FD->getParamDecl(I - 1);
+ if (I == 0) {
+ auto *MethodDecl = cast<CXXMethodDecl>(FD);
+ QualType ParamType = MethodDecl->getFunctionObjectParameterType();
+ return {getCaptureByAttrFromFunctionType(FD), ParamType};
+ }
+ if (I > 0 && I - 1 < FD->getNumParams()) {
+ const ParmVarDecl *PVD = FD->getParamDecl(I - 1);
+ return {PVD ? PVD->getAttr<LifetimeCaptureByAttr>() : nullptr,
+ PVD->getType()};
+ }
} else {
- if (I < FD->getNumParams())
- return FD->getParamDecl(I);
+ if (I < FD->getNumParams()) {
+ const ParmVarDecl *PVD = FD->getParamDecl(I);
+ return {PVD ? PVD->getAttr<LifetimeCaptureByAttr>() : nullptr,
+ PVD->getType()};
+ }
}
- return nullptr;
+ return {nullptr, QualType()};
};
for (unsigned I = 0; I < Args.size(); ++I) {
- const ParmVarDecl *PVD = getParamDeclAt(I);
- if (!PVD)
- continue;
- const auto *Attr = PVD->getAttr<LifetimeCaptureByAttr>();
+ auto [Attr, ParamType] = getArgCaptureByAttr(I);
if (!Attr)
continue;
OriginList *CapturedOriginList = getOriginsList(*Args[I]);
@@ -1056,7 +1065,7 @@ void FactsGenerator::handleLifetimeCaptureBy(const
FunctionDecl *FD,
continue;
// For references to pointer-like types, peel the outer origin (the pointer
// object itself) so that we capture the underlying data (the inner
origin).
- if (QualType ParamType = PVD->getType();
+ if (!ParamType.isNull() &&
(ParamType->isReferenceType() &&
isPointerLikeType(ParamType->getPointeeType())) &&
CapturedOriginList->getLength() > 1)
@@ -1067,13 +1076,8 @@ void FactsGenerator::handleLifetimeCaptureBy(const
FunctionDecl *FD,
CapturingArgIdx == LifetimeCaptureByAttr::Unknown ||
CapturingArgIdx == LifetimeCaptureByAttr::Invalid)
continue;
- ArrayRef<const Expr *> CallArgs = IsInstance ? Args.drop_front() : Args;
- const Expr *CapturedByArg =
- (CapturingArgIdx == LifetimeCaptureByAttr::This)
- ? Args[0]
- : CallArgs[CapturingArgIdx];
+ const Expr *CapturedByArg = Args[CapturingArgIdx];
assert(CapturedByArg && "Capturer expression must be valid");
-
OriginList *CapturingOriginList = getOriginsList(*CapturedByArg);
OriginList *Dest = getRValueOrigins(CapturedByArg, CapturingOriginList);
if (!Dest)
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
index 154657193cabc..bef5d2ad3ec72 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
@@ -136,6 +136,24 @@ FunctionCallInfo getFunctionCallInfo(const Expr *Call) {
return Info;
}
+const LifetimeCaptureByAttr *
+getCaptureByAttrFromFunctionType(const FunctionDecl *FD) {
+ const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+ if (!TSI)
+ return nullptr;
+ // Walk through the type layers looking for a capture_by attribute.
+ TypeLoc TL = TSI->getTypeLoc();
+ while (true) {
+ auto ATL = TL.getAsAdjusted<AttributedTypeLoc>();
+ if (!ATL)
+ break;
+ if (auto *Attr = ATL.getAttrAs<LifetimeCaptureByAttr>())
+ return Attr;
+ TL = ATL.getModifiedLoc();
+ }
+ return nullptr;
+}
+
std::optional<LifetimeBoundParamInfo>
getTrackedArgInfo(const FunctionDecl *FD, llvm::ArrayRef<const Expr *> Args,
unsigned I) {
diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp
b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
index 0c0c280d73cb6..4b528f463db7e 100644
--- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
@@ -107,23 +107,29 @@ class LifetimeAnnotatedOriginTypeCollector
const auto *MD = dyn_cast<CXXMethodDecl>(FD);
bool IsInstance = MD && MD->isInstance();
int Offset = (MD && MD->isImplicitObjectMemberFunction()) ? 1 : 0;
+ auto ProcessAttr = [&](const LifetimeCaptureByAttr *Attr) {
+ for (int Idx : Attr->params()) {
+ if (Idx == LifetimeCaptureByAttr::Global ||
+ Idx == LifetimeCaptureByAttr::Unknown ||
+ Idx == LifetimeCaptureByAttr::Invalid)
+ continue;
+ if (Idx == LifetimeCaptureByAttr::This) {
+ if (IsInstance)
+ CollectedTypes.push_back(MD->getFunctionObjectParameterType());
+ } else if (int LogicalIdx = Idx - Offset;
+ LogicalIdx >= 0 &&
+ (unsigned)LogicalIdx < FD->getNumParams()) {
+ CollectedTypes.push_back(
+ FD->getParamDecl(LogicalIdx)->getType().getNonReferenceType());
+ }
+ }
+ };
+ if (const auto *MethodAttr =
+ IsInstance ? getCaptureByAttrFromFunctionType(FD) : nullptr)
+ ProcessAttr(MethodAttr);
for (const auto *Param : FD->parameters()) {
if (auto *Attr = Param->getAttr<LifetimeCaptureByAttr>()) {
- for (int Idx : Attr->params()) {
- if (Idx == LifetimeCaptureByAttr::Global ||
- Idx == LifetimeCaptureByAttr::Unknown ||
- Idx == LifetimeCaptureByAttr::Invalid)
- continue;
- if (Idx == LifetimeCaptureByAttr::This) {
- if (IsInstance)
- CollectedTypes.push_back(MD->getFunctionObjectParameterType());
- } else if (int LogicalIdx = Idx - Offset;
- LogicalIdx >= 0 &&
- (unsigned)LogicalIdx < FD->getNumParams()) {
- CollectedTypes.push_back(
- FD->getParamDecl(LogicalIdx)->getType().getNonReferenceType());
- }
- }
+ ProcessAttr(Attr);
}
}
}
diff --git a/clang/test/Sema/LifetimeSafety/capture-by.cpp
b/clang/test/Sema/LifetimeSafety/capture-by.cpp
index e9e745b52ec0b..e6b0cb2e24e36 100644
--- a/clang/test/Sema/LifetimeSafety/capture-by.cpp
+++ b/clang/test/Sema/LifetimeSafety/capture-by.cpp
@@ -252,19 +252,62 @@ void initializer_list_capture() {
// Implicit object param 'this' is captured
// ****************************************************************************
namespace this_is_captured {
-struct X {} x;
+struct X {} x; // cfg-note {{this global dangles}}
struct S {
void capture(X &x) [[clang::lifetime_capture_by(x)]];
};
-// FIXME: Add support for capture of method declarations in -Wlifetime-safety
void use() {
- S{}.capture(x); // expected-warning {{object whose reference is captured by
'x' will be destroyed at the end of the full-expression}}
- S s;
- s.capture(x);
+ S{}.capture(x); // expected-warning {{object whose reference is captured by
'x' will be destroyed at the end of the full-expression}} \
+ // cfg-warning {{temporary object does not live long
enough}} \
+ // cfg-note {{temporary object is destroyed here}}
+ (void)x; // cfg-note {{later used here}}
+ S s;
+ s.capture(x); // cfg-warning {{stack memory associated with local variable
's' escapes to the global variable 'x' which will dangle}}
}
} // namespace this_is_captured
+namespace method_decl_capture {
+struct Container {
+ const void* stored = nullptr;
+ void add(const void* s) { stored = s; }
+};
+struct Obj {
+ void register_into(Container& c) const [[clang::lifetime_capture_by(c)]] {
+ c.add(this);
+ }
+};
+void test() {
+ Container c;
+ {
+ Obj local_obj;
+ local_obj.register_into(c); // cfg-warning {{local variable 'local_obj'
does not live long enough}}
+ } // cfg-note {{local variable 'local_obj' is
destroyed here}}
+ (void)c.stored; // cfg-note {{later used here}}
+}
+} // namespace method_decl_capture
+
+namespace method_capture_chaining {
+struct Container {
+ const void* stored = nullptr;
+ void capture(const void* v [[clang::lifetime_capture_by_this]]);
+};
+struct Obj {
+ const void* data = nullptr;
+ void addTo(Container& c) const [[clang::lifetime_capture_by(c)]] {
+ c.capture(data);
+ }
+};
+void test() {
+ Container c;
+ {
+ Obj local_obj;
+ local_obj.addTo(c); // cfg-warning {{local variable 'local_obj'
does not live long enough}}
+ } // cfg-note {{local variable 'local_obj' is
destroyed here}}
+ (void)c.stored; // cfg-note {{later used here}}
+}
+} // namespace method_capture_chaining
+
namespace temporary_capturing_object {
struct S {
void add(const int& x [[clang::lifetime_capture_by_this]]);
``````````
</details>
https://github.com/llvm/llvm-project/pull/219423
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
