[clang] 5c287ef - [Clang] Extend lifetime bound analysis to support assignments for the built-in pointer type (#96475)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 1 08:43:13 PDT 2024
Author: Haojian Wu
Date: 2024-07-01T17:43:07+02:00
New Revision: 5c287efee77930ab15fe5579bed00b2750bbb254
URL: https://github.com/llvm/llvm-project/commit/5c287efee77930ab15fe5579bed00b2750bbb254
DIFF: https://github.com/llvm/llvm-project/commit/5c287efee77930ab15fe5579bed00b2750bbb254.diff
LOG: [Clang] Extend lifetime bound analysis to support assignments for the built-in pointer type (#96475)
The lifetime bound warning in Clang currently only considers
initializations. This patch extends the warning to include assignments.
- **Support for assignments of built-in pointer types**: this is done is
by reusing the existing statement-local implementation. Clang now warns
if the pointer is assigned to a temporary object that being destoryed at
the end of the full assignment expression.
With this patch, we will detect more cases under the on-by-default
diagnostic `-Wdangling`. I have added a new category for this specific
diagnostic so that people can temporarily disable it if their codebase
is not yet clean.
This is the first step to address #63310, focusing only on pointer
types. Support for C++ assignment operators will come in a follow-up
patch.
Fixes #54492
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/CheckExprLifetime.cpp
clang/lib/Sema/CheckExprLifetime.h
clang/lib/Sema/SemaExpr.cpp
clang/test/Parser/compound_literal.c
clang/test/SemaCXX/attr-lifetimebound.cpp
clang/test/SemaCXX/warn-dangling-local.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f893d8a8729a2..90c2469c1c89a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -641,6 +641,10 @@ Improvements to Clang's diagnostics
used rather than when they are needed for constant evaluation or when code is generated for them.
The check is now stricter to prevent crashes for some unsupported declarations (Fixes #GH95495).
+- Clang now diagnoses dangling cases where a pointer is assigned to a temporary
+ that will be destroyed at the end of the full expression.
+ Fixes #GH54492.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index de99cb46d3145..74deb92c94c7a 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 cbf4745bad723..b8eafffd4650d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10109,6 +10109,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..be77949e8b547 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,17 +966,34 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
return false;
}
-void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
- Expr *Init) {
- LifetimeResult LR = getEntityLifetime(&Entity);
- LifetimeKind LK = LR.getInt();
- const InitializedEntity *ExtendingEntity = LR.getPointer();
-
+static void checkExprLifetimeImpl(
+ Sema &SemaRef,
+ llvm::PointerUnion<const InitializedEntity *, const AssignedEntity *>
+ CEntity,
+ Expr *Init) {
+ LifetimeKind LK = LK_FullExpression;
+
+ const AssignedEntity *AEntity = nullptr;
+ // Local variables for initialized entity.
+ const InitializedEntity *InitEntity = nullptr;
+ const InitializedEntity *ExtendingEntity = nullptr;
+ if (CEntity.is<const InitializedEntity *>()) {
+ InitEntity = CEntity.get<const InitializedEntity *>();
+ auto LTResult = getEntityLifetime(InitEntity);
+ LK = LTResult.getInt();
+ ExtendingEntity = LTResult.getPointer();
+ } else {
+ AEntity = CEntity.get<const AssignedEntity *>();
+ if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type
+ LK = LK_Extended;
+ }
// If this entity doesn't have an interesting lifetime, don't bother looking
// for temporaries within its initializer.
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);
@@ -1028,6 +1047,8 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
switch (shouldLifetimeExtendThroughPath(Path)) {
case PathLifetimeKind::Extend:
+ assert(InitEntity && "Lifetime extension should happen only for "
+ "initialization and not assignment");
// Update the storage duration of the materialized temporary.
// FIXME: Rebuild the expression instead of mutating it.
MTE->setExtendingDecl(ExtendingEntity->getDecl(),
@@ -1036,6 +1057,8 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
return true;
case PathLifetimeKind::ShouldExtend:
+ assert(InitEntity && "Lifetime extension should happen only for "
+ "initialization and not assignment");
// We're supposed to lifetime-extend the temporary along this path (per
// the resolution of DR1815), but we don't support that yet.
//
@@ -1053,17 +1076,24 @@ 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: {
- if (isa<MaterializeTemporaryExpr>(L)) {
+ assert(InitEntity && "Expect only on initializing the entity");
+
+ 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.
@@ -1077,7 +1107,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 +1167,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 +1176,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 +1185,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 +1204,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 +1213,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;
}
@@ -1252,8 +1284,20 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
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);
+}
+
+void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
+ Expr *Init) {
+ checkExprLifetimeImpl(SemaRef, &Entity, Init);
+}
+
+void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity,
+ Expr *Init) {
+ checkExprLifetimeImpl(SemaRef, &Entity, Init);
}
} // namespace clang::sema
diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h
index f9cd3f55d3dbf..af381fb96c4d6 100644
--- a/clang/lib/Sema/CheckExprLifetime.h
+++ b/clang/lib/Sema/CheckExprLifetime.h
@@ -18,12 +18,22 @@
namespace clang::sema {
+/// Describes an entity that is being assigned.
+struct AssignedEntity {
+ // The left-hand side expression of the assignment.
+ Expr *LHS = nullptr;
+};
+
/// 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,
Expr *Init);
+/// Check that the lifetime of the given expr (and its subobjects) is
+/// sufficient for assigning to the entity.
+void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init);
+
} // namespace clang::sema
#endif // LLVM_CLANG_SEMA_CHECK_EXPR_LIFETIME_H
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d1ddd6f9d1072..66a8de6454c57 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/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..70bc545c07bd9 100644
--- a/clang/test/SemaCXX/attr-lifetimebound.cpp
+++ b/clang/test/SemaCXX/attr-lifetimebound.cpp
@@ -40,6 +40,13 @@ 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}}
+ 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
More information about the cfe-commits
mailing list