Skip to content

Commit c22bb6f

Browse files
usx95bricknerb
andauthored
[clang] Implement lifetime analysis for lifetime_capture_by(X) (#115921)
This PR uses the existing lifetime analysis for the `capture_by` attribute. The analysis is behind `-Wdangling-capture` warning and is disabled by default for now. Once it is found to be stable, it will be default enabled. Planned followup: - add implicit inference of this attribute on STL container methods like `std::vector::push_back`. - (consider) warning if capturing `X` cannot capture anything. It should be a reference, pointer or a view type. - refactoring temporary visitors and other related handlers. - start discussing `__global` vs `global` in the annotation in a separate PR. --------- Co-authored-by: Boaz Brickner <[email protected]>
1 parent 3e15bce commit c22bb6f

14 files changed

+701
-167
lines changed

clang/include/clang/Basic/AttrDocs.td

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3921,17 +3921,42 @@ have their lifetimes extended.
39213921
def LifetimeCaptureByDocs : Documentation {
39223922
let Category = DocCatFunction;
39233923
let Content = [{
3924-
Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a function
3925-
parameter or implicit object parameter indicates that that objects that are referred to
3926-
by that parameter may also be referred to by the capturing entity ``X``.
3924+
Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a
3925+
function parameter or implicit object parameter indicates that the capturing
3926+
entity ``X`` may refer to the object referred by that parameter.
3927+
3928+
Below is a list of types of the parameters and what they're considered to refer to:
3929+
3930+
- A reference param (of non-view type) is considered to refer to its referenced object.
3931+
- A pointer param (of non-view type) is considered to refer to its pointee.
3932+
- View type param (type annotated with ``[[gsl::Pointer()]]``) is considered to refer
3933+
to its pointee (gsl owner). This holds true even if the view type appears as a reference
3934+
in the parameter. For example, both ``std::string_view`` and
3935+
``const std::string_view &`` are considered to refer to a ``std::string``.
3936+
- A ``std::initializer_list<T>`` is considered to refer to its underlying array.
3937+
- Aggregates (arrays and simple ``struct``\s) are considered to refer to all
3938+
objects that their transitive subobjects refer to.
3939+
3940+
Clang would diagnose when a temporary object is used as an argument to such an
3941+
annotated parameter.
3942+
In this case, the capturing entity ``X`` could capture a dangling reference to this
3943+
temporary object.
39273944

3928-
By default, a reference is considered to refer to its referenced object, a
3929-
pointer is considered to refer to its pointee, a ``std::initializer_list<T>``
3930-
is considered to refer to its underlying array, and aggregates (arrays and
3931-
simple ``struct``\s) are considered to refer to all objects that their
3932-
transitive subobjects refer to.
3945+
.. code-block:: c++
3946+
3947+
void addToSet(std::string_view a [[clang::lifetime_capture_by(s)]], std::set<std::string_view>& s) {
3948+
s.insert(a);
3949+
}
3950+
void use() {
3951+
std::set<std::string_view> s;
3952+
addToSet(std::string(), s); // Warning: object whose reference is captured by 's' will be destroyed at the end of the full-expression.
3953+
// ^^^^^^^^^^^^^
3954+
std::string local;
3955+
addToSet(local, s); // Ok.
3956+
}
39333957

39343958
The capturing entity ``X`` can be one of the following:
3959+
39353960
- Another (named) function parameter.
39363961

39373962
.. code-block:: c++
@@ -3951,7 +3976,7 @@ The capturing entity ``X`` can be one of the following:
39513976
std::set<std::string_view> s;
39523977
};
39533978

3954-
- 'global', 'unknown' (without quotes).
3979+
- `global`, `unknown`.
39553980

39563981
.. code-block:: c++
39573982

@@ -3983,6 +4008,22 @@ The attribute supports specifying more than one capturing entities:
39834008
s2.insert(a);
39844009
}
39854010

4011+
Limitation: The capturing entity ``X`` is not used by the analysis and is
4012+
used for documentation purposes only. This is because the analysis is
4013+
statement-local and only detects use of a temporary as an argument to the
4014+
annotated parameter.
4015+
4016+
.. code-block:: c++
4017+
4018+
void addToSet(std::string_view a [[clang::lifetime_capture_by(s)]], std::set<std::string_view>& s);
4019+
void use() {
4020+
std::set<std::string_view> s;
4021+
if (foo()) {
4022+
std::string str;
4023+
addToSet(str, s); // Not detected.
4024+
}
4025+
}
4026+
39864027
.. _`lifetimebound`: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
39874028
}];
39884029
}

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
453453
def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
454454
def DanglingAssignment: DiagGroup<"dangling-assignment">;
455455
def DanglingAssignmentGsl : DiagGroup<"dangling-assignment-gsl">;
456+
def DanglingCapture : DiagGroup<"dangling-capture">;
456457
def DanglingElse: DiagGroup<"dangling-else">;
457458
def DanglingField : DiagGroup<"dangling-field">;
458459
def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
@@ -462,6 +463,7 @@ def ReturnStackAddress : DiagGroup<"return-stack-address">;
462463
def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
463464
def Dangling : DiagGroup<"dangling", [DanglingAssignment,
464465
DanglingAssignmentGsl,
466+
DanglingCapture,
465467
DanglingField,
466468
DanglingInitializerList,
467469
DanglingGsl,

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10129,10 +10129,11 @@ def err_lifetimebound_ctor_dtor : Error<
1012910129
"%select{constructor|destructor}0">;
1013010130
def err_lifetimebound_parameter_void_return_type : Error<
1013110131
"'lifetimebound' attribute cannot be applied to a parameter of a function "
10132-
"that returns void">;
10132+
"that returns void; did you mean 'lifetime_capture_by(X)'">;
1013310133
def err_lifetimebound_implicit_object_parameter_void_return_type : Error<
1013410134
"'lifetimebound' attribute cannot be applied to an implicit object "
10135-
"parameter of a function that returns void">;
10135+
"parameter of a function that returns void; "
10136+
"did you mean 'lifetime_capture_by(X)'">;
1013610137

1013710138
// CHECK: returning address/reference of stack memory
1013810139
def warn_ret_stack_addr_ref : Warning<
@@ -10227,6 +10228,12 @@ def warn_dangling_pointer_assignment : Warning<
1022710228
"object backing %select{|the pointer }0%1 "
1022810229
"will be destroyed at the end of the full-expression">,
1022910230
InGroup<DanglingAssignment>;
10231+
def warn_dangling_reference_captured : Warning<
10232+
"object whose reference is captured by '%0' will be destroyed at the end of "
10233+
"the full-expression">, InGroup<DanglingCapture>, DefaultIgnore;
10234+
def warn_dangling_reference_captured_by_unknown : Warning<
10235+
"object whose reference is captured will be destroyed at the end of "
10236+
"the full-expression">, InGroup<DanglingCapture>, DefaultIgnore;
1023010237

1023110238
// For non-floating point, expressions of the form x == x or x != x
1023210239
// should result in a warning, since these always evaluate to a constant.

clang/include/clang/Sema/Sema.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,6 +2323,9 @@ class Sema final : public SemaBase {
23232323
bool BuiltinVectorMath(CallExpr *TheCall, QualType &Res, bool FPOnly = false);
23242324
bool BuiltinVectorToScalarMath(CallExpr *TheCall);
23252325

2326+
void checkLifetimeCaptureBy(FunctionDecl *FDecl, bool IsMemberFunction,
2327+
const Expr *ThisArg, ArrayRef<const Expr *> Args);
2328+
23262329
/// Handles the checks for format strings, non-POD arguments to vararg
23272330
/// functions, NULL arguments passed to non-NULL parameters, diagnose_if
23282331
/// attributes and AArch64 SME attributes.

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 63 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,14 @@ enum LifetimeKind {
4545
/// a default member initializer), the program is ill-formed.
4646
LK_MemInitializer,
4747

48-
/// The lifetime of a temporary bound to this entity probably ends too soon,
48+
/// The lifetime of a temporary bound to this entity may end too soon,
4949
/// because the entity is a pointer and we assign the address of a temporary
5050
/// object to it.
5151
LK_Assignment,
52+
53+
/// The lifetime of a temporary bound to this entity may end too soon,
54+
/// because the entity may capture the reference to a temporary object.
55+
LK_LifetimeCapture,
5256
};
5357
using LifetimeResult =
5458
llvm::PointerIntPair<const InitializedEntity *, 3, LifetimeKind>;
@@ -1108,13 +1112,14 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
11081112
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
11091113
}
11101114

1111-
static void checkExprLifetimeImpl(Sema &SemaRef,
1112-
const InitializedEntity *InitEntity,
1113-
const InitializedEntity *ExtendingEntity,
1114-
LifetimeKind LK,
1115-
const AssignedEntity *AEntity, Expr *Init) {
1116-
assert((AEntity && LK == LK_Assignment) ||
1117-
(InitEntity && LK != LK_Assignment));
1115+
static void
1116+
checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
1117+
const InitializedEntity *ExtendingEntity, LifetimeKind LK,
1118+
const AssignedEntity *AEntity,
1119+
const CapturingEntity *CapEntity, Expr *Init) {
1120+
assert(!AEntity || LK == LK_Assignment);
1121+
assert(!CapEntity || LK == LK_LifetimeCapture);
1122+
assert(!InitEntity || (LK != LK_Assignment && LK != LK_LifetimeCapture));
11181123
// If this entity doesn't have an interesting lifetime, don't bother looking
11191124
// for temporaries within its initializer.
11201125
if (LK == LK_FullExpression)
@@ -1197,12 +1202,23 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
11971202
break;
11981203
}
11991204

1205+
case LK_LifetimeCapture: {
1206+
// The captured entity has lifetime beyond the full-expression,
1207+
// and the capturing entity does too, so don't warn.
1208+
if (!MTE)
1209+
return false;
1210+
if (CapEntity->Entity)
1211+
SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured)
1212+
<< CapEntity->Entity << DiagRange;
1213+
else
1214+
SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured_by_unknown)
1215+
<< DiagRange;
1216+
return false;
1217+
}
1218+
12001219
case LK_Assignment: {
12011220
if (!MTE || pathContainsInit(Path))
12021221
return false;
1203-
assert(shouldLifetimeExtendThroughPath(Path) ==
1204-
PathLifetimeKind::NoExtend &&
1205-
"No lifetime extension for assignments");
12061222
if (IsGslPtrValueFromGslTempOwner)
12071223
SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer_assignment)
12081224
<< AEntity->LHS << DiagRange;
@@ -1411,13 +1427,23 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
14111427
};
14121428

14131429
llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
1414-
if (LK == LK_Assignment &&
1415-
shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) {
1416-
Path.push_back(
1417-
{isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator)
1418-
? IndirectLocalPathEntry::LifetimeBoundCall
1419-
: IndirectLocalPathEntry::GslPointerAssignment,
1420-
Init});
1430+
switch (LK) {
1431+
case LK_Assignment: {
1432+
if (shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
1433+
Path.push_back(
1434+
{isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator)
1435+
? IndirectLocalPathEntry::LifetimeBoundCall
1436+
: IndirectLocalPathEntry::GslPointerAssignment,
1437+
Init});
1438+
break;
1439+
}
1440+
case LK_LifetimeCapture: {
1441+
if (isPointerLikeType(Init->getType()))
1442+
Path.push_back({IndirectLocalPathEntry::GslPointerInit, Init});
1443+
break;
1444+
}
1445+
default:
1446+
break;
14211447
}
14221448

14231449
if (Init->isGLValue())
@@ -1430,23 +1456,23 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
14301456
/*RevisitSubinits=*/!InitEntity);
14311457
}
14321458

1433-
void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
1459+
void checkInitLifetime(Sema &SemaRef, const InitializedEntity &Entity,
14341460
Expr *Init) {
14351461
auto LTResult = getEntityLifetime(&Entity);
14361462
LifetimeKind LK = LTResult.getInt();
14371463
const InitializedEntity *ExtendingEntity = LTResult.getPointer();
14381464
checkExprLifetimeImpl(SemaRef, &Entity, ExtendingEntity, LK,
1439-
/*AEntity*/ nullptr, Init);
1465+
/*AEntity=*/nullptr, /*CapEntity=*/nullptr, Init);
14401466
}
14411467

14421468
void checkExprLifetimeMustTailArg(Sema &SemaRef,
14431469
const InitializedEntity &Entity, Expr *Init) {
14441470
checkExprLifetimeImpl(SemaRef, &Entity, nullptr, LK_MustTail,
1445-
/*AEntity*/ nullptr, Init);
1471+
/*AEntity=*/nullptr, /*CapEntity=*/nullptr, Init);
14461472
}
14471473

1448-
void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
1449-
Expr *Init) {
1474+
void checkAssignmentLifetime(Sema &SemaRef, const AssignedEntity &Entity,
1475+
Expr *Init) {
14501476
bool EnableDanglingPointerAssignment = !SemaRef.getDiagnostics().isIgnored(
14511477
diag::warn_dangling_pointer_assignment, SourceLocation());
14521478
bool RunAnalysis = (EnableDanglingPointerAssignment &&
@@ -1458,7 +1484,20 @@ void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
14581484

14591485
checkExprLifetimeImpl(SemaRef, /*InitEntity=*/nullptr,
14601486
/*ExtendingEntity=*/nullptr, LK_Assignment, &Entity,
1461-
Init);
1487+
/*CapEntity=*/nullptr, Init);
1488+
}
1489+
1490+
void checkCaptureByLifetime(Sema &SemaRef, const CapturingEntity &Entity,
1491+
Expr *Init) {
1492+
if (SemaRef.getDiagnostics().isIgnored(diag::warn_dangling_reference_captured,
1493+
SourceLocation()) &&
1494+
SemaRef.getDiagnostics().isIgnored(
1495+
diag::warn_dangling_reference_captured_by_unknown, SourceLocation()))
1496+
return;
1497+
return checkExprLifetimeImpl(SemaRef, /*InitEntity=*/nullptr,
1498+
/*ExtendingEntity=*/nullptr, LK_LifetimeCapture,
1499+
/*AEntity=*/nullptr,
1500+
/*CapEntity=*/&Entity, Init);
14621501
}
14631502

14641503
} // namespace clang::sema

clang/lib/Sema/CheckExprLifetime.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,31 @@ struct AssignedEntity {
2525
CXXMethodDecl *AssignmentOperator = nullptr;
2626
};
2727

28+
struct CapturingEntity {
29+
// In an function call involving a lifetime capture, this would be the
30+
// argument capturing the lifetime of another argument.
31+
// void addToSet(std::string_view sv [[clang::lifetime_capture_by(setsv)]],
32+
// set<std::string_view>& setsv);
33+
// set<std::string_view> setsv;
34+
// addToSet(std::string(), setsv); // Here 'setsv' is the 'Entity'.
35+
//
36+
// This is 'nullptr' when the capturing entity is 'global' or 'unknown'.
37+
Expr *Entity = nullptr;
38+
};
39+
2840
/// Check that the lifetime of the given expr (and its subobjects) is
2941
/// sufficient for initializing the entity, and perform lifetime extension
3042
/// (when permitted) if not.
31-
void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
43+
void checkInitLifetime(Sema &SemaRef, const InitializedEntity &Entity,
3244
Expr *Init);
3345

3446
/// Check that the lifetime of the given expr (and its subobjects) is
3547
/// sufficient for assigning to the entity.
36-
void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init);
48+
void checkAssignmentLifetime(Sema &SemaRef, const AssignedEntity &Entity,
49+
Expr *Init);
50+
51+
void checkCaptureByLifetime(Sema &SemaRef, const CapturingEntity &Entity,
52+
Expr *Init);
3753

3854
/// Check that the lifetime of the given expr (and its subobjects) is
3955
/// sufficient, assuming that it is passed as an argument to a musttail

clang/lib/Sema/SemaChecking.cpp

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313

14+
#include "CheckExprLifetime.h"
1415
#include "clang/AST/APValue.h"
1516
#include "clang/AST/ASTContext.h"
1617
#include "clang/AST/Attr.h"
@@ -3222,6 +3223,47 @@ void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
32223223
<< ParamName << (FDecl != nullptr) << FDecl;
32233224
}
32243225

3226+
void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction,
3227+
const Expr *ThisArg,
3228+
ArrayRef<const Expr *> Args) {
3229+
if (!FD || Args.empty())
3230+
return;
3231+
auto GetArgAt = [&](int Idx) -> const Expr * {
3232+
if (Idx == LifetimeCaptureByAttr::GLOBAL ||
3233+
Idx == LifetimeCaptureByAttr::UNKNOWN)
3234+
return nullptr;
3235+
if (IsMemberFunction && Idx == 0)
3236+
return ThisArg;
3237+
return Args[Idx - IsMemberFunction];
3238+
};
3239+
auto HandleCaptureByAttr = [&](const LifetimeCaptureByAttr *Attr,
3240+
unsigned ArgIdx) {
3241+
if (!Attr)
3242+
return;
3243+
Expr *Captured = const_cast<Expr *>(GetArgAt(ArgIdx));
3244+
for (int CapturingParamIdx : Attr->params()) {
3245+
Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx));
3246+
CapturingEntity CE{Capturing};
3247+
// Ensure that 'Captured' outlives the 'Capturing' entity.
3248+
checkCaptureByLifetime(*this, CE, Captured);
3249+
}
3250+
};
3251+
for (unsigned I = 0; I < FD->getNumParams(); ++I)
3252+
HandleCaptureByAttr(FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(),
3253+
I + IsMemberFunction);
3254+
// Check when the implicit object param is captured.
3255+
if (IsMemberFunction) {
3256+
TypeSourceInfo *TSI = FD->getTypeSourceInfo();
3257+
if (!TSI)
3258+
return;
3259+
AttributedTypeLoc ATL;
3260+
for (TypeLoc TL = TSI->getTypeLoc();
3261+
(ATL = TL.getAsAdjusted<AttributedTypeLoc>());
3262+
TL = ATL.getModifiedLoc())
3263+
HandleCaptureByAttr(ATL.getAttrAs<LifetimeCaptureByAttr>(), 0);
3264+
}
3265+
}
3266+
32253267
void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
32263268
const Expr *ThisArg, ArrayRef<const Expr *> Args,
32273269
bool IsMemberFunction, SourceLocation Loc,
@@ -3262,7 +3304,8 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
32623304
}
32633305
}
32643306
}
3265-
3307+
if (FD)
3308+
checkLifetimeCaptureBy(FD, IsMemberFunction, ThisArg, Args);
32663309
if (FDecl || Proto) {
32673310
CheckNonNullArguments(*this, FDecl, Proto, Args, Loc);
32683311

clang/lib/Sema/SemaExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13821,7 +13821,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
1382113821
CheckForNullPointerDereference(*this, LHSExpr);
1382213822

1382313823
AssignedEntity AE{LHSExpr};
13824-
checkExprLifetime(*this, AE, RHS.get());
13824+
checkAssignmentLifetime(*this, AE, RHS.get());
1382513825

1382613826
if (getLangOpts().CPlusPlus20 && LHSType.isVolatileQualified()) {
1382713827
if (CompoundType.isNull()) {

0 commit comments

Comments
 (0)