[clang] Extend the existing lifetimebound check for assignments.

Currently we only detect the builtin pointer type.

Author: hokein

clang/include/clang/Basic/DiagnosticGroups.td

@@ -430,14 +430,16 @@ 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">;
 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]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -10092,6 +10092,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 "

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"
 
@@ -210,7 +212,7 @@ struct RevertToOldSizeRAII {
 
 using LocalVisitor = llvm::function_ref<bool(IndirectLocalPath &Path, Local L,
                                              ReferenceKind RK)>;
-}
+} // namespace
 
 static bool isVarOnPath(IndirectLocalPath &Path, VarDecl *VD) {
   for (auto E : Path)
@@ -962,12 +964,27 @@ 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 AEntityPointer =
+                 std::get_if<const AssignedEntity *>(&CEntity)) {
+    AEntity = *AEntityPointer;
+    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.
@@ -1026,6 +1043,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(),
@@ -1034,6 +1052,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.
         //
@@ -1051,16 +1070,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
@@ -1075,7 +1101,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
@@ -1135,15 +1161,16 @@ 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)
               << DiagRange;
         else
           SemaRef.Diag(DiagLoc, RK == RK_ReferenceBinding
-                            ? diag::warn_new_dangling_reference
-                            : diag::warn_new_dangling_initializer_list)
-              << !Entity.getParent() << DiagRange;
+                                    ? diag::warn_new_dangling_reference
+                                    : diag::warn_new_dangling_initializer_list)
+              << !InitEntity->getParent() << DiagRange;
       } else {
         // We can't determine if the allocation outlives the local declaration.
         return false;
@@ -1152,13 +1179,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;
@@ -1170,21 +1198,21 @@ 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,
         // other than an invented function for std::is_convertible ([meta.rel]),
         // 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;
     }
@@ -1217,9 +1245,8 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity,
       case IndirectLocalPathEntry::VarInit: {
         const VarDecl *VD = cast<VarDecl>(Elem.D);
         SemaRef.Diag(VD->getLocation(), diag::note_local_var_initializer)
-            << VD->getType()->isReferenceType()
-            << VD->isImplicit() << VD->getDeclName()
-            << nextPathEntryRange(Path, I + 1, L);
+            << VD->getType()->isReferenceType() << VD->isImplicit()
+            << VD->getDeclName() << nextPathEntryRange(Path, I + 1, L);
         break;
       }
 

clang/lib/Sema/CheckExprLifetime.h

@@ -15,13 +15,23 @@
 #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 the entity, emit diagnostics if
+/// not.
+void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity,
                        Expr *Init);
 
 } // namespace clang::sema

clang/lib/Sema/SemaExpr.cpp

@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "CheckExprLifetime.h"
 #include "TreeTransform.h"
 #include "UsedDeclVisitor.h"
 #include "clang/AST/ASTConsumer.h"
@@ -13777,6 +13778,9 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
     return QualType();
 
   CheckForNullPointerDereference(*this, LHSExpr);
+  
+  AssignedEntity AE{LHSExpr};
+  checkExprLifetime(*this, &AE, RHS.get());
 
   if (getLangOpts().CPlusPlus20 && LHSType.isVolatileQualified()) {
     if (CompoundType.isNull()) {

clang/lib/Sema/SemaInit.cpp

@@ -7288,7 +7288,8 @@ PerformConstructorInitialization(Sema &S,
 
 void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
                                     Expr *Init) {
-  return sema::checkExprLifetime(*this, Entity, Init);
+  sema::CheckingEntity CEntity(&Entity);
+  return sema::checkExprLifetime(*this, CEntity, Init);
 }
 
 static void DiagnoseNarrowingInInitList(Sema &S,

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};
 }

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

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