[clang] [clang] Respect the lifetimebound in assignment operator. (PR #106997)
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 3 02:30:48 PDT 2024
https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/106997
>From 88600fb5e7395d4eefb6be24689248b7be2d4115 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Wed, 28 Aug 2024 12:11:28 +0200
Subject: [PATCH 1/2] [clang] Respect the lifetimebound in user-defined
operator.
---
clang/docs/ReleaseNotes.rst | 2 +
clang/lib/Sema/CheckExprLifetime.cpp | 65 +++++++++++++++--------
clang/lib/Sema/CheckExprLifetime.h | 1 +
clang/lib/Sema/SemaOverload.cpp | 4 +-
clang/test/SemaCXX/attr-lifetimebound.cpp | 15 ++++++
5 files changed, 63 insertions(+), 24 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fc940db4813948..f87080955b3302 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -266,6 +266,8 @@ Improvements to Clang's diagnostics
compilation speed with modules. This warning is disabled by default and it needs
to be explicitly enabled or by ``-Weverything``.
+- Clang now respects lifetimebound attribute for the assignment operator parameter. (#GH106372).
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index f28789dba34e10..776d0e8b07a30b 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -326,24 +326,11 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
return false;
}
-static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
- const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
- if (!TSI)
- return false;
- // Don't declare this variable in the second operand of the for-statement;
- // GCC miscompiles that by ending its lifetime before evaluating the
- // third operand. See gcc.gnu.org/PR86769.
- AttributedTypeLoc ATL;
- for (TypeLoc TL = TSI->getTypeLoc();
- (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
- TL = ATL.getModifiedLoc()) {
- if (ATL.getAttrAs<LifetimeBoundAttr>())
- return true;
- }
-
- // Assume that all assignment operators with a "normal" return type return
- // *this, that is, an lvalue reference that is the same type as the implicit
- // object parameter (or the LHS for a non-member operator$=).
+// Return true if this is an "normal" assignment operator.
+// We assuments that a normal assingment operator always returns *this, that is,
+// an lvalue reference that is the same type as the implicit object parameter
+// (or the LHS for a non-member operator$=).
+static bool isNormalAsisgnmentOperator(const FunctionDecl *FD) {
OverloadedOperatorKind OO = FD->getDeclName().getCXXOverloadedOperator();
if (OO == OO_Equal || isCompoundAssignmentOperator(OO)) {
QualType RetT = FD->getReturnType();
@@ -359,10 +346,27 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
return true;
}
}
-
return false;
}
+static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
+ const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+ if (!TSI)
+ return false;
+ // Don't declare this variable in the second operand of the for-statement;
+ // GCC miscompiles that by ending its lifetime before evaluating the
+ // third operand. See gcc.gnu.org/PR86769.
+ AttributedTypeLoc ATL;
+ for (TypeLoc TL = TSI->getTypeLoc();
+ (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
+ TL = ATL.getModifiedLoc()) {
+ if (ATL.getAttrAs<LifetimeBoundAttr>())
+ return true;
+ }
+
+ return isNormalAsisgnmentOperator(FD);
+}
+
// Visit lifetimebound or gsl-pointer arguments.
static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
LocalVisitor Visit,
@@ -968,6 +972,23 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) {
return false;
}
+static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
+ if (!CMD)
+ return false;
+ assert(CMD->getOverloadedOperator() == OverloadedOperatorKind::OO_Equal);
+ return isNormalAsisgnmentOperator(CMD) && CMD->param_size() == 1 &&
+ CMD->getParamDecl(0)->hasAttr<LifetimeBoundAttr>();
+}
+
+static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
+ const AssignedEntity &Entity) {
+ bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored(
+ diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
+ return (EnableGSLAssignmentWarnings &&
+ (isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
+ isAssginmentOperatorLifetimeBound(Entity.AssignmentOperator)));
+}
+
static void checkExprLifetimeImpl(Sema &SemaRef,
const InitializedEntity *InitEntity,
const InitializedEntity *ExtendingEntity,
@@ -1267,8 +1288,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
};
llvm::SmallVector<IndirectLocalPathEntry, 8> Path;
- if (EnableLifetimeWarnings && LK == LK_Assignment &&
- isRecordWithAttr<PointerAttr>(AEntity->LHS->getType()))
+ if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity))
Path.push_back({IndirectLocalPathEntry::GslPointerAssignment, Init});
if (Init->isGLValue())
@@ -1301,8 +1321,7 @@ void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
diag::warn_dangling_pointer_assignment, SourceLocation());
bool RunAnalysis = (EnableDanglingPointerAssignment &&
Entity.LHS->getType()->isPointerType()) ||
- (EnableLifetimeWarnings &&
- isRecordWithAttr<PointerAttr>(Entity.LHS->getType()));
+ shouldRunGSLAssignmentAnalysis(SemaRef, Entity);
if (!RunAnalysis)
return;
diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h
index af381fb96c4d68..8c8d0806dee0a3 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -22,6 +22,7 @@ namespace clang::sema {
struct AssignedEntity {
// The left-hand side expression of the assignment.
Expr *LHS = nullptr;
+ CXXMethodDecl *AssignmentOperator = nullptr;
};
/// Check that the lifetime of the given expr (and its subobjects) is
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 95551173df91a5..861b0a91240b3b 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14768,7 +14768,9 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
// Check for a self move.
DiagnoseSelfMove(Args[0], Args[1], OpLoc);
// lifetime check.
- checkExprLifetime(*this, AssignedEntity{Args[0]}, Args[1]);
+ checkExprLifetime(
+ *this, AssignedEntity{Args[0], dyn_cast<CXXMethodDecl>(FnDecl)},
+ Args[1]);
}
if (ImplicitThis) {
QualType ThisType = Context.getPointerType(ImplicitThis->getType());
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 6566ed6270cd14..0bc06ef9cab3ab 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -287,3 +287,18 @@ std::span<int> test2() {
return abc; // expected-warning {{address of stack memory associated with local variable}}
}
} // namespace ctor_cases
+
+namespace GH106372 {
+class [[gsl::Owner]] Foo {};
+class [[gsl::Pointer]] FooView {};
+
+template <typename T>
+struct StatusOr {
+ template <typename U = T>
+ StatusOr& operator=(U&& v [[clang::lifetimebound]]);
+};
+
+void test(StatusOr<FooView> foo) {
+ foo = Foo(); // expected-warning {{object backing the pointer foo will be destroyed at the end}}
+}
+} // namespace GH106372
>From 577e81d2e068d45079e95850e589581475985e86 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 3 Sep 2024 11:30:16 +0200
Subject: [PATCH 2/2] Add a non-warning test case.
---
clang/test/SemaCXX/attr-lifetimebound.cpp | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 0bc06ef9cab3ab..0fb997a5671085 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -292,13 +292,18 @@ namespace GH106372 {
class [[gsl::Owner]] Foo {};
class [[gsl::Pointer]] FooView {};
+class NonAnnotatedFoo {};
+class NonAnnotatedFooView {};
+
template <typename T>
struct StatusOr {
template <typename U = T>
StatusOr& operator=(U&& v [[clang::lifetimebound]]);
};
-void test(StatusOr<FooView> foo) {
- foo = Foo(); // expected-warning {{object backing the pointer foo will be destroyed at the end}}
+void test(StatusOr<FooView> foo1, StatusOr<NonAnnotatedFooView> foo2) {
+ foo1 = Foo(); // expected-warning {{object backing the pointer foo1 will be destroyed at the end}}
+ // No warning on non-gsl annotated types.
+ foo2 = NonAnnotatedFoo();
}
} // namespace GH106372
More information about the cfe-commits
mailing list