https://github.com/hokein updated
https://github.com/llvm/llvm-project/pull/96475
>From 97d1b80680112c3fa271501427a18273aed61dbe Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Mon, 24 Jun 2024 10:58:53 +0200
Subject: [PATCH 1/2] [clang] Extend the existing lifetimebound check for
assignments.
Currently we only detect the builtin pointer type.
---
clang/include/clang/Basic/DiagnosticGroups.td | 4 +-
.../clang/Basic/DiagnosticSemaKinds.td | 5 ++
clang/lib/Sema/CheckExprLifetime.cpp | 60 ++++++++++++++-----
clang/lib/Sema/CheckExprLifetime.h | 18 +++++-
clang/lib/Sema/SemaExpr.cpp | 4 ++
clang/lib/Sema/SemaInit.cpp | 2 +-
clang/test/Parser/compound_literal.c | 5 +-
clang/test/SemaCXX/attr-lifetimebound.cpp | 6 ++
clang/test/SemaCXX/warn-dangling-local.cpp | 2 +
9 files changed, 83 insertions(+), 23 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index 1c4f305fb5d00..9ae579c2c2a11 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -430,6 +430,7 @@ def LogicalOpParentheses:
DiagGroup<"logical-op-parentheses">;
def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
+def DanglingAssignment: DiagGroup<"dangling-assignment">;
def DanglingElse: DiagGroup<"dangling-else">;
def DanglingField : DiagGroup<"dangling-field">;
def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
@@ -437,7 +438,8 @@ def DanglingGsl : DiagGroup<"dangling-gsl">;
def ReturnStackAddress : DiagGroup<"return-stack-address">;
// Name of this warning in GCC
def : DiagGroup<"return-local-addr", [ReturnStackAddress]>;
-def Dangling : DiagGroup<"dangling", [DanglingField,
+def Dangling : DiagGroup<"dangling", [DanglingAssignment,
+ DanglingField,
DanglingInitializerList,
DanglingGsl,
ReturnStackAddress]>;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 33412e14897b6..f613243cb8460 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10120,6 +10120,11 @@ def warn_new_dangling_initializer_list : Warning<
"the allocated initializer list}0 "
"will be destroyed at the end of the full-expression">,
InGroup<DanglingInitializerList>;
+def warn_dangling_pointer_assignment : Warning<
+ "object backing the pointer %0 "
+ "will be destroyed at the end of the full-expression">,
+ InGroup<DanglingAssignment>;
+
def warn_unsupported_lifetime_extension : Warning<
"lifetime extension of "
"%select{temporary|backing array of initializer list}0 created "
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index 54e2f1c22536d..67fbd5e449d4a 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -8,6 +8,8 @@
#include "CheckExprLifetime.h"
#include "clang/AST/Expr.h"
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Sema/Initialization.h"
#include "clang/Sema/Sema.h"
#include "llvm/ADT/PointerIntPair.h"
@@ -964,11 +966,26 @@ static bool
pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
return false;
}
-void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
Expr *Init) {
- LifetimeResult LR = getEntityLifetime(&Entity);
- LifetimeKind LK = LR.getInt();
- const InitializedEntity *ExtendingEntity = LR.getPointer();
+ LifetimeKind LK = LK_FullExpression;
+
+ const AssignedEntity *AEntity = nullptr;
+ // Local variables for initialized entity.
+ const InitializedEntity *InitEntity = nullptr;
+ const InitializedEntity *ExtendingEntity = nullptr;
+ if (auto IEntityP = std::get_if<const InitializedEntity *>(&CEntity)) {
+ InitEntity = *IEntityP;
+ auto LTResult = getEntityLifetime(InitEntity);
+ LK = LTResult.getInt();
+ ExtendingEntity = LTResult.getPointer();
+ } else if (auto AEntityP = std::get_if<const AssignedEntity *>(&CEntity)) {
+ AEntity = *AEntityP;
+ if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type
+ LK = LK_Extended;
+ } else {
+ llvm_unreachable("unexpected kind");
+ }
// If this entity doesn't have an interesting lifetime, don't bother looking
// for temporaries within its initializer.
@@ -1028,6 +1045,7 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
switch (shouldLifetimeExtendThroughPath(Path)) {
case PathLifetimeKind::Extend:
+ assert(InitEntity && "Expect only on initializing the entity");
// Update the storage duration of the materialized temporary.
// FIXME: Rebuild the expression instead of mutating it.
MTE->setExtendingDecl(ExtendingEntity->getDecl(),
@@ -1036,6 +1054,7 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
return true;
case PathLifetimeKind::ShouldExtend:
+ assert(InitEntity && "Expect only on initializing the entity");
// We're supposed to lifetime-extend the temporary along this path (per
// the resolution of DR1815), but we don't support that yet.
//
@@ -1053,16 +1072,23 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
if (pathContainsInit(Path))
return false;
- SemaRef.Diag(DiagLoc, diag::warn_dangling_variable)
- << RK << !Entity.getParent()
- << ExtendingEntity->getDecl()->isImplicit()
- << ExtendingEntity->getDecl() << Init->isGLValue() << DiagRange;
+ if (InitEntity) {
+ SemaRef.Diag(DiagLoc, diag::warn_dangling_variable)
+ << RK << !InitEntity->getParent()
+ << ExtendingEntity->getDecl()->isImplicit()
+ << ExtendingEntity->getDecl() << Init->isGLValue() << DiagRange;
+ } else {
+ SemaRef.Diag(DiagLoc, diag::warn_dangling_pointer_assignment)
+ << AEntity->LHS << DiagRange;
+ }
break;
}
break;
}
case LK_MemInitializer: {
+ assert(InitEntity && "Expect only on initializing the entity");
+
if (isa<MaterializeTemporaryExpr>(L)) {
// Under C++ DR1696, if a mem-initializer (or a default member
// initializer used by the absence of one) would lifetime-extend a
@@ -1077,7 +1103,7 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
<< true;
return false;
}
- bool IsSubobjectMember = ExtendingEntity != &Entity;
+ bool IsSubobjectMember = ExtendingEntity != InitEntity;
SemaRef.Diag(DiagLoc, shouldLifetimeExtendThroughPath(Path) !=
PathLifetimeKind::NoExtend
? diag::err_dangling_member
@@ -1137,6 +1163,7 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
}
case LK_New:
+ assert(InitEntity && "Expect only on initializing the entity");
if (isa<MaterializeTemporaryExpr>(L)) {
if (IsGslPtrInitWithGslTempOwner)
SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
@@ -1145,7 +1172,7 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
SemaRef.Diag(DiagLoc, RK == RK_ReferenceBinding
? diag::warn_new_dangling_reference
: diag::warn_new_dangling_initializer_list)
- << !Entity.getParent() << DiagRange;
+ << !InitEntity->getParent() << DiagRange;
} else {
// We can't determine if the allocation outlives the local declaration.
return false;
@@ -1154,13 +1181,14 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
case LK_Return:
case LK_StmtExprResult:
+ assert(InitEntity && "Expect only on initializing the entity");
if (auto *DRE = dyn_cast<DeclRefExpr>(L)) {
// We can't determine if the local variable outlives the statement
// expression.
if (LK == LK_StmtExprResult)
return false;
SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref)
- << Entity.getType()->isReferenceType() << DRE->getDecl()
+ << InitEntity->getType()->isReferenceType() << DRE->getDecl()
<< isa<ParmVarDecl>(DRE->getDecl()) << DiagRange;
} else if (isa<BlockExpr>(L)) {
SemaRef.Diag(DiagLoc, diag::err_ret_local_block) << DiagRange;
@@ -1172,8 +1200,8 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
SemaRef.Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
} else if (auto *CLE = dyn_cast<CompoundLiteralExpr>(L)) {
SemaRef.Diag(DiagLoc, diag::warn_ret_stack_addr_ref)
- << Entity.getType()->isReferenceType() << CLE->getInitializer() <<
2
- << DiagRange;
+ << InitEntity->getType()->isReferenceType() <<
CLE->getInitializer()
+ << 2 << DiagRange;
} else {
// P2748R5: Disallow Binding a Returned Glvalue to a Temporary.
// [stmt.return]/p6: In a function whose return type is a reference,
@@ -1181,12 +1209,12 @@ void checkExprLifetime(Sema &SemaRef, const
InitializedEntity &Entity,
// a return statement that binds the returned reference to a temporary
// expression ([class.temporary]) is ill-formed.
if (SemaRef.getLangOpts().CPlusPlus26 &&
- Entity.getType()->isReferenceType())
+ InitEntity->getType()->isReferenceType())
SemaRef.Diag(DiagLoc, diag::err_ret_local_temp_ref)
- << Entity.getType()->isReferenceType() << DiagRange;
+ << InitEntity->getType()->isReferenceType() << DiagRange;
else
SemaRef.Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
- << Entity.getType()->isReferenceType() << DiagRange;
+ << InitEntity->getType()->isReferenceType() << DiagRange;
}
break;
}
diff --git a/clang/lib/Sema/CheckExprLifetime.h
b/clang/lib/Sema/CheckExprLifetime.h
index f9cd3f55d3dbf..0882d41b35b6c 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -15,13 +15,25 @@
#include "clang/AST/Expr.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Sema.h"
+#include <variant>
namespace clang::sema {
+/// Describes an entity that is being assigned.
+struct AssignedEntity {
+ // The left-hand side expression of the assignment.
+ Expr *LHS = nullptr;
+};
+
+using CheckingEntity =
+ std::variant<const InitializedEntity *, const AssignedEntity *>;
+
/// Check that the lifetime of the given expr (and its subobjects) is
-/// sufficient for initializing the entity, and perform lifetime extension
-/// (when permitted) if not.
-void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+/// sufficient for initializing or assigning to the entity.
+///
+/// If the entity is being initialized and its lifetime is insufficient,
perform
+/// lifetime extension (when permitted).
+void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
Expr *Init);
} // namespace clang::sema
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index db44cfe1288b6..5c8b0682dcacc 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "CheckExprLifetime.h"
#include "TreeTransform.h"
#include "UsedDeclVisitor.h"
#include "clang/AST/ASTConsumer.h"
@@ -13778,6 +13779,9 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr,
ExprResult &RHS,
CheckForNullPointerDereference(*this, LHSExpr);
+ AssignedEntity AE{LHSExpr};
+ checkExprLifetime(*this, &AE, RHS.get());
+
if (getLangOpts().CPlusPlus20 && LHSType.isVolatileQualified()) {
if (CompoundType.isNull()) {
// C++2a [expr.ass]p5:
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 26c65b34f4cc4..db2bacbad8a61 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7287,7 +7287,7 @@ PerformConstructorInitialization(Sema &S,
void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
Expr *Init) {
- return sema::checkExprLifetime(*this, Entity, Init);
+ return sema::checkExprLifetime(*this, &Entity, Init);
}
static void DiagnoseNarrowingInInitList(Sema &S,
diff --git a/clang/test/Parser/compound_literal.c
b/clang/test/Parser/compound_literal.c
index 808ca63f97d9e..281a6d7ada430 100644
--- a/clang/test/Parser/compound_literal.c
+++ b/clang/test/Parser/compound_literal.c
@@ -1,8 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ -Wno-dangling-assignment %s
// expected-no-diagnostics
int main(void) {
char *s;
- s = (char []){"whatever"};
+ // In C++ mode, the cast creates a "char [4]" array temporary here.
+ s = (char []){"whatever"}; // dangling!
s = (char(*)){s};
}
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 5fcda6f23dab7..7c572ecef52ba 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -40,6 +40,12 @@ namespace usage_ok {
int *p = A().class_member(); // expected-warning {{temporary whose address
is used as value of local variable 'p' will be destroyed at the end of the
full-expression}}
int *q = A(); // expected-warning {{temporary whose address is used as value
of local variable 'q' will be destroyed at the end of the full-expression}}
int *r = A(1); // expected-warning {{temporary whose address is used as
value of local variable 'r' will be destroyed at the end of the
full-expression}}
+
+ void test_assignment() {
+ p = A().class_member(); // expected-warning {{object backing the pointer p
will be destroyed at the end of the full-expression}}
+ q = A(); // expected-warning {{object backing the pointer q will be
destroyed at the end of the full-expression}}
+ r = A(1); // expected-warning {{object backing the pointer r will be
destroyed at the end of the full-expression}}
+ }
}
# 1 "<std>" 1 3
diff --git a/clang/test/SemaCXX/warn-dangling-local.cpp
b/clang/test/SemaCXX/warn-dangling-local.cpp
index 5c1d56972945c..a946b8a241a38 100644
--- a/clang/test/SemaCXX/warn-dangling-local.cpp
+++ b/clang/test/SemaCXX/warn-dangling-local.cpp
@@ -4,8 +4,10 @@ using T = int[];
void f() {
int *p = &(int&)(int&&)0; // expected-warning {{temporary whose address is
used as value of local variable 'p' will be destroyed at the end of the
full-expression}}
+ p = &(int&)(int&&)0; // expected-warning {{object backing the pointer p
will be destroyed at the end of the full-expression}}
int *q = (int *const &)T{1, 2, 3}; // expected-warning {{temporary whose
address is used as value of local variable 'q' will be destroyed at the end of
the full-expression}}
+ q = (int *const &)T{1, 2, 3}; // expected-warning {{object backing the
pointer q will be destroyed at the end of the full-expression}}
// FIXME: We don't warn here because the 'int*' temporary is not const, but
// it also can't have actually changed since it was created, so we could
>From 78485d8f62eb36ecf7176f37ad19c43e2eea6a09 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Fri, 28 Jun 2024 16:25:23 +0200
Subject: [PATCH 2/2] Address review comments.
---
clang/lib/Sema/CheckExprLifetime.cpp | 10 +++++++---
clang/test/SemaCXX/attr-lifetimebound.cpp | 1 +
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index 67fbd5e449d4a..e50fa4facdff6 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -992,6 +992,8 @@ void checkExprLifetime(Sema &SemaRef, const CheckingEntity
&CEntity,
if (LK == LK_FullExpression)
return;
+ // FIXME: consider moving the TemporaryVisitor and visitLocalsRetained*
+ // functions to a dedicated class.
auto TemporaryVisitor = [&](IndirectLocalPath &Path, Local L,
ReferenceKind RK) -> bool {
SourceRange DiagRange = nextPathEntryRange(Path, 0, L);
@@ -1089,7 +1091,7 @@ void checkExprLifetime(Sema &SemaRef, const
CheckingEntity &CEntity,
case LK_MemInitializer: {
assert(InitEntity && "Expect only on initializing the entity");
- if (isa<MaterializeTemporaryExpr>(L)) {
+ if (MTE) {
// Under C++ DR1696, if a mem-initializer (or a default member
// initializer used by the absence of one) would lifetime-extend a
// temporary, the program is ill-formed.
@@ -1280,8 +1282,10 @@ void checkExprLifetime(Sema &SemaRef, const
CheckingEntity &CEntity,
TemporaryVisitor,
EnableLifetimeWarnings);
else
- visitLocalsRetainedByInitializer(Path, Init, TemporaryVisitor, false,
- EnableLifetimeWarnings);
+ visitLocalsRetainedByInitializer(
+ Path, Init, TemporaryVisitor,
+ // Don't revisit the sub inits for the intialization case.
+ /*RevisitSubinits=*/!InitEntity, EnableLifetimeWarnings);
}
} // namespace clang::sema
diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp
b/clang/test/SemaCXX/attr-lifetimebound.cpp
index 7c572ecef52ba..70bc545c07bd9 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -43,6 +43,7 @@ namespace usage_ok {
void test_assignment() {
p = A().class_member(); // expected-warning {{object backing the pointer p
will be destroyed at the end of the full-expression}}
+ p = {A().class_member()}; // expected-warning {{object backing the pointer
p will be destroyed at the end of the full-expression}}
q = A(); // expected-warning {{object backing the pointer q will be
destroyed at the end of the full-expression}}
r = A(1); // expected-warning {{object backing the pointer r will be
destroyed at the end of the full-expression}}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
