[clang] 01e5684 - [clang] Respect the lifetimebound in assignment operator. (#106997)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 4 01:09:07 PDT 2024
Author: Haojian Wu
Date: 2024-09-04T10:09:04+02:00
New Revision: 01e56849001b4ace984e9557abc82bc051e03677
URL: https://github.com/llvm/llvm-project/commit/01e56849001b4ace984e9557abc82bc051e03677
DIFF: https://github.com/llvm/llvm-project/commit/01e56849001b4ace984e9557abc82bc051e03677.diff
LOG: [clang] Respect the lifetimebound in assignment operator. (#106997)
Fixes #106372
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/CheckExprLifetime.cpp
clang/lib/Sema/CheckExprLifetime.h
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/attr-lifetimebound.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 45b08128600efb..511724c73015ea 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@ Improvements to Clang's diagnostics
- Improved diagnostic when trying to overload a function in an ``extern "C"`` context. (#GH80235)
+- 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..f7540a6e3a8979 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,22 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) {
return false;
}
+static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) {
+ if (!CMD)
+ return false;
+ 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 +1287,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 +1320,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..0fb997a5671085 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -287,3 +287,23 @@ 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 {};
+
+class NonAnnotatedFoo {};
+class NonAnnotatedFooView {};
+
+template <typename T>
+struct StatusOr {
+ template <typename U = T>
+ StatusOr& operator=(U&& v [[clang::lifetimebound]]);
+};
+
+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