Skip to content

Commit 01e5684

Browse files
authored
[clang] Respect the lifetimebound in assignment operator. (#106997)
Fixes #106372
1 parent de37da8 commit 01e5684

File tree

5 files changed

+67
-24
lines changed

5 files changed

+67
-24
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,8 @@ Improvements to Clang's diagnostics
268268

269269
- Improved diagnostic when trying to overload a function in an ``extern "C"`` context. (#GH80235)
270270

271+
- Clang now respects lifetimebound attribute for the assignment operator parameter. (#GH106372).
272+
271273
Improvements to Clang's time-trace
272274
----------------------------------
273275

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -326,24 +326,11 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
326326
return false;
327327
}
328328

329-
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
330-
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
331-
if (!TSI)
332-
return false;
333-
// Don't declare this variable in the second operand of the for-statement;
334-
// GCC miscompiles that by ending its lifetime before evaluating the
335-
// third operand. See gcc.gnu.org/PR86769.
336-
AttributedTypeLoc ATL;
337-
for (TypeLoc TL = TSI->getTypeLoc();
338-
(ATL = TL.getAsAdjusted<AttributedTypeLoc>());
339-
TL = ATL.getModifiedLoc()) {
340-
if (ATL.getAttrAs<LifetimeBoundAttr>())
341-
return true;
342-
}
343-
344-
// Assume that all assignment operators with a "normal" return type return
345-
// *this, that is, an lvalue reference that is the same type as the implicit
346-
// object parameter (or the LHS for a non-member operator$=).
329+
// Return true if this is an "normal" assignment operator.
330+
// We assuments that a normal assingment operator always returns *this, that is,
331+
// an lvalue reference that is the same type as the implicit object parameter
332+
// (or the LHS for a non-member operator$=).
333+
static bool isNormalAsisgnmentOperator(const FunctionDecl *FD) {
347334
OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
348335
if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) {
349336
QualType RetT = FD->getReturnType();
@@ -359,10 +346,27 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
359346
return true;
360347
}
361348
}
362-
363349
return false;
364350
}
365351

352+
static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
353+
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
354+
if (!TSI)
355+
return false;
356+
// Don't declare this variable in the second operand of the for-statement;
357+
// GCC miscompiles that by ending its lifetime before evaluating the
358+
// third operand. See gcc.gnu.org/PR86769.
359+
AttributedTypeLoc ATL;
360+
for (TypeLoc TL = TSI->getTypeLoc();
361+
(ATL = TL.getAsAdjusted<AttributedTypeLoc>());
362+
TL = ATL.getModifiedLoc()) {
363+
if (ATL.getAttrAs<LifetimeBoundAttr>())
364+
return true;
365+
}
366+
367+
return isNormalAsisgnmentOperator(FD);
368+
}
369+
366370
// Visit lifetimebound or gsl-pointer arguments.
367371
static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
368372
LocalVisitor Visit,
@@ -968,6 +972,22 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) {
968972
return false;
969973
}
970974

975+
static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
976+
if (!CMD)
977+
return false;
978+
return isNormalAsisgnmentOperator(CMD) && CMD->param_size() == 1 &&
979+
CMD->getParamDecl(0)->hasAttr<LifetimeBoundAttr>();
980+
}
981+
982+
static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
983+
const AssignedEntity &Entity) {
984+
bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored(
985+
diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
986+
return (EnableGSLAssignmentWarnings &&
987+
(isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
988+
isAssginmentOperatorLifetimeBound(Entity.AssignmentOperator)));
989+
}
990+
971991
static void checkExprLifetimeImpl(Sema &SemaRef,
972992
const InitializedEntity *InitEntity,
973993
const InitializedEntity *ExtendingEntity,
@@ -1267,8 +1287,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
12671287
};
12681288

12691289
llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
1270-
if (EnableLifetimeWarnings && LK == LK_Assignment &&
1271-
isRecordWithAttr<PointerAttr>(AEntity->LHS->getType()))
1290+
if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
12721291
Path.push_back({IndirectLocalPathEntry::GslPointerAssignment, Init});
12731292

12741293
if (Init->isGLValue())
@@ -1301,8 +1320,7 @@ void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
13011320
diag::warn_dangling_pointer_assignment, SourceLocation());
13021321
bool RunAnalysis = (EnableDanglingPointerAssignment &&
13031322
Entity.LHS->getType()->isPointerType()) ||
1304-
(EnableLifetimeWarnings &&
1305-
isRecordWithAttr<PointerAttr>(Entity.LHS->getType()));
1323+
shouldRunGSLAssignmentAnalysis(SemaRef, Entity);
13061324

13071325
if (!RunAnalysis)
13081326
return;

clang/lib/Sema/CheckExprLifetime.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace clang::sema {
2222
struct AssignedEntity {
2323
// The left-hand side expression of the assignment.
2424
Expr *LHS = nullptr;
25+
CXXMethodDecl *AssignmentOperator = nullptr;
2526
};
2627

2728
/// Check that the lifetime of the given expr (and its subobjects) is

clang/lib/Sema/SemaOverload.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14768,7 +14768,9 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
1476814768
// Check for a self move.
1476914769
DiagnoseSelfMove(Args[0], Args[1], OpLoc);
1477014770
// lifetime check.
14771-
checkExprLifetime(*this, AssignedEntity{Args[0]}, Args[1]);
14771+
checkExprLifetime(
14772+
*this, AssignedEntity{Args[0], dyn_cast<CXXMethodDecl>(FnDecl)},
14773+
Args[1]);
1477214774
}
1477314775
if (ImplicitThis) {
1477414776
QualType ThisType = Context.getPointerType(ImplicitThis->getType());

clang/test/SemaCXX/attr-lifetimebound.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,23 @@ std::span<int> test2() {
287287
return abc; // expected-warning {{address of stack memory associated with local variable}}
288288
}
289289
} // namespace ctor_cases
290+
291+
namespace GH106372 {
292+
class [[gsl::Owner]] Foo {};
293+
class [[gsl::Pointer]] FooView {};
294+
295+
class NonAnnotatedFoo {};
296+
class NonAnnotatedFooView {};
297+
298+
template <typename T>
299+
struct StatusOr {
300+
template <typename U = T>
301+
StatusOr& operator=(U&& v [[clang::lifetimebound]]);
302+
};
303+
304+
void test(StatusOr<FooView> foo1, StatusOr<NonAnnotatedFooView> foo2) {
305+
foo1 = Foo(); // expected-warning {{object backing the pointer foo1 will be destroyed at the end}}
306+
// No warning on non-gsl annotated types.
307+
foo2 = NonAnnotatedFoo();
308+
}
309+
} // namespace GH106372

0 commit comments

Comments
 (0)