[clang] [Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (PR #117437)

via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 23 07:24:50 PST 2024


https://github.com/yronglin created https://github.com/llvm/llvm-project/pull/117437

Clang now support the following:

- Extending lifetime of object bound to reference members of aggregates, that are created from default member initializer.
- Rebuild all sub-expressions in CXXDefaultArgExpr and CXXDefaultInitExpr as needed.

But CFG and ExprEngine need to be updated to address this change.

This PR reapply https://github.com/llvm/llvm-project/pull/91879.

- Introduce a new flag `HasRebuiltInit` flag in `CXXDefaultInitExpr` and `CXXDefaultArgExpr`.
- Fixes https://github.com/llvm/llvm-project/issues/93725.

>From 75b4765da0b4ebb008781b393181c5b080be2578 Mon Sep 17 00:00:00 2001
From: yronglin <yronglin777 at gmail.com>
Date: Fri, 22 Nov 2024 21:06:58 +0800
Subject: [PATCH] [Analyzer][CFG] Correctly handle rebuilt default arg and
 default init expression

Signed-off-by: yronglin <yronglin777 at gmail.com>
---
 clang/include/clang/AST/ExprCXX.h             | 30 +++++++---
 clang/include/clang/AST/Stmt.h                | 13 ++++-
 clang/lib/AST/ASTImporter.cpp                 |  7 ++-
 clang/lib/AST/ExprCXX.cpp                     | 43 +++++++-------
 clang/lib/AST/ParentMap.cpp                   | 17 ++++++
 clang/lib/Analysis/CFG.cpp                    | 50 ++++++++++++++---
 clang/lib/Analysis/ReachableCode.cpp          | 31 +++++-----
 clang/lib/Sema/SemaExpr.cpp                   |  9 ++-
 clang/lib/Sema/TreeTransform.h                |  7 ++-
 clang/lib/Serialization/ASTReaderStmt.cpp     |  8 ++-
 clang/lib/Serialization/ASTWriterStmt.cpp     |  2 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  | 56 +++++++++++--------
 .../Analysis/lifetime-extended-regions.cpp    |  7 +--
 clang/test/SemaCXX/warn-unreachable.cpp       | 39 +++++++++++++
 14 files changed, 229 insertions(+), 90 deletions(-)

diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 696a574833dad2..99680537a3f777 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1277,7 +1277,8 @@ class CXXDefaultArgExpr final
   DeclContext *UsedContext;
 
   CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *Param,
-                    Expr *RewrittenExpr, DeclContext *UsedContext)
+                    DeclContext *UsedContext, Expr *RewrittenExpr,
+                    bool HasRebuiltInit)
       : Expr(SC,
              Param->hasUnparsedDefaultArg()
                  ? Param->getType().getNonReferenceType()
@@ -1287,25 +1288,27 @@ class CXXDefaultArgExpr final
         Param(Param), UsedContext(UsedContext) {
     CXXDefaultArgExprBits.Loc = Loc;
     CXXDefaultArgExprBits.HasRewrittenInit = RewrittenExpr != nullptr;
+    CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
     if (RewrittenExpr)
       *getTrailingObjects<Expr *>() = RewrittenExpr;
     setDependence(computeDependence(this));
   }
 
-  CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit)
+  CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
       : Expr(CXXDefaultArgExprClass, Empty) {
     CXXDefaultArgExprBits.HasRewrittenInit = HasRewrittenInit;
+    CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
   }
 
 public:
   static CXXDefaultArgExpr *CreateEmpty(const ASTContext &C,
-                                        bool HasRewrittenInit);
+                                        bool HasRewrittenInit, bool HasRebuiltInit);
 
   // \p Param is the parameter whose default argument is used by this
   // expression.
   static CXXDefaultArgExpr *Create(const ASTContext &C, SourceLocation Loc,
-                                   ParmVarDecl *Param, Expr *RewrittenExpr,
-                                   DeclContext *UsedContext);
+                                   ParmVarDecl *Param, DeclContext *UsedContext,
+                                   Expr *RewrittenExpr, bool HasRebuiltInit);
   // Retrieve the parameter that the argument was created from.
   const ParmVarDecl *getParam() const { return Param; }
   ParmVarDecl *getParam() { return Param; }
@@ -1314,6 +1317,10 @@ class CXXDefaultArgExpr final
     return CXXDefaultArgExprBits.HasRewrittenInit;
   }
 
+  bool hasRebuiltInit() const {
+    return CXXDefaultArgExprBits.HasRebuiltInit;
+  }
+
   // Retrieve the argument to the function call.
   Expr *getExpr();
   const Expr *getExpr() const {
@@ -1385,26 +1392,31 @@ class CXXDefaultInitExpr final
 
   CXXDefaultInitExpr(const ASTContext &Ctx, SourceLocation Loc,
                      FieldDecl *Field, QualType Ty, DeclContext *UsedContext,
-                     Expr *RewrittenInitExpr);
+                     Expr *RewrittenInitExpr, bool HasRebuiltInit);
 
-  CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit)
+  CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
       : Expr(CXXDefaultInitExprClass, Empty) {
     CXXDefaultInitExprBits.HasRewrittenInit = HasRewrittenInit;
+    CXXDefaultInitExprBits.HasRebuiltInit = HasRebuiltInit;
   }
 
 public:
   static CXXDefaultInitExpr *CreateEmpty(const ASTContext &C,
-                                         bool HasRewrittenInit);
+                                         bool HasRewrittenInit, bool HasRebuiltInit);
   /// \p Field is the non-static data member whose default initializer is used
   /// by this expression.
   static CXXDefaultInitExpr *Create(const ASTContext &Ctx, SourceLocation Loc,
                                     FieldDecl *Field, DeclContext *UsedContext,
-                                    Expr *RewrittenInitExpr);
+                                    Expr *RewrittenInitExpr, bool HasRebuiltInit);
 
   bool hasRewrittenInit() const {
     return CXXDefaultInitExprBits.HasRewrittenInit;
   }
 
+  bool hasRebuiltInit() const {
+    return CXXDefaultInitExprBits.HasRebuiltInit;
+  }
+
   /// Get the field whose initializer will be used.
   FieldDecl *getField() { return Field; }
   const FieldDecl *getField() const { return Field; }
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 83fafbabb1d460..1b2dbcbaa09219 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -839,6 +839,10 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(bool)
     unsigned HasRewrittenInit : 1;
 
+    /// Whether this CXXDefaultArgExpr rebuild its argument and stores a copy.
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasRebuiltInit : 1;
+
     /// The location where the default argument expression was used.
     SourceLocation Loc;
   };
@@ -850,11 +854,16 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(ExprBitfields)
     unsigned : NumExprBits;
 
-    /// Whether this CXXDefaultInitExprBitfields rewrote its argument and stores
-    /// a copy.
+    /// Whether this CXXDefaultInitExpr rewrote its argument and stores
+    /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may
+    /// be partial rebuilt.
     LLVM_PREFERRED_TYPE(bool)
     unsigned HasRewrittenInit : 1;
 
+    /// Whether this CXXDefaultInitExpr fully rebuild its argument and stores a copy.
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasRebuiltInit : 1;
+
     /// The location where the default initializer expression was used.
     SourceLocation Loc;
   };
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index baed1416635432..e2126f5fd29a47 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -8154,8 +8154,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
     RewrittenInit = ExprOrErr.get();
   }
   return CXXDefaultArgExpr::Create(Importer.getToContext(), *ToUsedLocOrErr,
-                                   *ToParamOrErr, RewrittenInit,
-                                   *UsedContextOrErr);
+                                   *ToParamOrErr, *UsedContextOrErr,
+                                   RewrittenInit, E->hasRebuiltInit());
 }
 
 ExpectedStmt
@@ -8863,7 +8863,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
   }
 
   return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
-                                    ToField, *UsedContextOrErr, RewrittenInit);
+                                    ToField, *UsedContextOrErr, RewrittenInit,
+                                    E->hasRebuiltInit());
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 0ce129de85f03f..6b6507d4b5ebeb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1009,21 +1009,23 @@ const IdentifierInfo *UserDefinedLiteral::getUDSuffix() const {
 }
 
 CXXDefaultArgExpr *CXXDefaultArgExpr::CreateEmpty(const ASTContext &C,
-                                                  bool HasRewrittenInit) {
+                                                  bool HasRewrittenInit,
+                                                  bool HasRebuiltInit) {
   size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
   auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
-  return new (Mem) CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit);
+  return new (Mem)
+      CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
 }
 
-CXXDefaultArgExpr *CXXDefaultArgExpr::Create(const ASTContext &C,
-                                             SourceLocation Loc,
-                                             ParmVarDecl *Param,
-                                             Expr *RewrittenExpr,
-                                             DeclContext *UsedContext) {
+CXXDefaultArgExpr *
+CXXDefaultArgExpr::Create(const ASTContext &C, SourceLocation Loc,
+                          ParmVarDecl *Param, DeclContext *UsedContext,
+                          Expr *RewrittenExpr, bool HasRebuiltInit) {
   size_t Size = totalSizeToAlloc<Expr *>(RewrittenExpr != nullptr);
   auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
-  return new (Mem) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param,
-                                     RewrittenExpr, UsedContext);
+  return new (Mem)
+      CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param, UsedContext,
+                        RewrittenExpr, HasRebuiltInit);
 }
 
 Expr *CXXDefaultArgExpr::getExpr() {
@@ -1044,7 +1046,7 @@ Expr *CXXDefaultArgExpr::getAdjustedRewrittenExpr() {
 CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
                                        SourceLocation Loc, FieldDecl *Field,
                                        QualType Ty, DeclContext *UsedContext,
-                                       Expr *RewrittenInitExpr)
+                                       Expr *RewrittenInitExpr, bool hasRebuiltInit)
     : Expr(CXXDefaultInitExprClass, Ty.getNonLValueExprType(Ctx),
            Ty->isLValueReferenceType()   ? VK_LValue
            : Ty->isRValueReferenceType() ? VK_XValue
@@ -1053,6 +1055,7 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
       Field(Field), UsedContext(UsedContext) {
   CXXDefaultInitExprBits.Loc = Loc;
   CXXDefaultInitExprBits.HasRewrittenInit = RewrittenInitExpr != nullptr;
+  CXXDefaultInitExprBits.HasRebuiltInit = hasRebuiltInit;
 
   if (CXXDefaultInitExprBits.HasRewrittenInit)
     *getTrailingObjects<Expr *>() = RewrittenInitExpr;
@@ -1063,22 +1066,24 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
 }
 
 CXXDefaultInitExpr *CXXDefaultInitExpr::CreateEmpty(const ASTContext &C,
-                                                    bool HasRewrittenInit) {
+                                                    bool HasRewrittenInit,
+                                                    bool HasRebuiltInit) {
   size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
   auto *Mem = C.Allocate(Size, alignof(CXXDefaultInitExpr));
-  return new (Mem) CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit);
+  return new (Mem)
+      CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
 }
 
-CXXDefaultInitExpr *CXXDefaultInitExpr::Create(const ASTContext &Ctx,
-                                               SourceLocation Loc,
-                                               FieldDecl *Field,
-                                               DeclContext *UsedContext,
-                                               Expr *RewrittenInitExpr) {
+CXXDefaultInitExpr *
+CXXDefaultInitExpr::Create(const ASTContext &Ctx, SourceLocation Loc,
+                           FieldDecl *Field, DeclContext *UsedContext,
+                           Expr *RewrittenInitExpr, bool HasRebuiltInit) {
 
   size_t Size = totalSizeToAlloc<Expr *>(RewrittenInitExpr != nullptr);
   auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultInitExpr));
-  return new (Mem) CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(),
-                                      UsedContext, RewrittenInitExpr);
+  return new (Mem)
+      CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(), UsedContext,
+                         RewrittenInitExpr, HasRebuiltInit);
 }
 
 Expr *CXXDefaultInitExpr::getExpr() {
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index fd749b02b758c9..61eb2ec0e1cb94 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "llvm/ADT/DenseMap.h"
 
@@ -96,6 +97,22 @@ static void BuildParentMap(MapTy& M, Stmt* S,
       BuildParentMap(M, SubStmt, OVMode);
     }
     break;
+  case Stmt::CXXDefaultArgExprClass:
+    if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
+      if (Arg->hasRebuiltInit()) {
+        M[Arg->getExpr()] = S;
+        BuildParentMap(M, Arg->getExpr(), OVMode);
+      }
+    }
+    break;
+  case Stmt::CXXDefaultInitExprClass:
+    if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
+      if (Init->hasRebuiltInit()) {
+        M[Init->getExpr()] = S;
+        BuildParentMap(M, Init->getExpr(), OVMode);
+      }
+    }
+    break;
   default:
     for (Stmt *SubStmt : S->children()) {
       if (SubStmt) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61..82c283335b0ff4 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -556,6 +556,10 @@ class CFGBuilder {
 
 private:
   // Visitors to walk an AST and construct the CFG.
+  CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
+                                   AddStmtChoice asc);
+  CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
+                                    AddStmtChoice asc);
   CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
   CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
   CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2261,16 +2265,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
                                    asc, ExternallyDestructed);
 
     case Stmt::CXXDefaultArgExprClass:
+      return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
     case Stmt::CXXDefaultInitExprClass:
-      // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
-      // called function's declaration, not by the caller. If we simply add
-      // this expression to the CFG, we could end up with the same Expr
-      // appearing multiple times (PR13385).
-      //
-      // It's likewise possible for multiple CXXDefaultInitExprs for the same
-      // expression to be used in the same function (through aggregate
-      // initialization).
-      return VisitStmt(S, asc);
+      return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
 
     case Stmt::CXXBindTemporaryExprClass:
       return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2440,6 +2438,40 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
   return B;
 }
 
+CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
+                                             AddStmtChoice asc) {
+  if (Arg->hasRebuiltInit()) {
+    if (asc.alwaysAdd(*this, Arg)) {
+      autoCreateBlock();
+      appendStmt(Block, Arg);
+    }
+    return VisitStmt(Arg->getExpr(), asc);
+  }
+
+  // We can't add the default argument if it's not rewritten because the
+  // expression inside a CXXDefaultArgExpr is owned by the called function's
+  // declaration, not by the caller, we could end up with the same expression
+  // appearing multiple times.
+  return VisitStmt(Arg, asc);
+}
+
+CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
+                                              AddStmtChoice asc) {
+  if (Init->hasRebuiltInit()) {
+    if (asc.alwaysAdd(*this, Init)) {
+      autoCreateBlock();
+      appendStmt(Block, Init);
+    }
+    return VisitStmt(Init->getExpr(), asc);
+  }
+
+  // We can't add the default initializer if it's not rewritten because multiple
+  // CXXDefaultInitExprs for the same sub-expression to be used in the same
+  // function (through aggregate initialization). we could end up with the same
+  // expression appearing multiple times.
+  return VisitStmt(Init, asc);
+}
+
 CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
   if (asc.alwaysAdd(*this, ILE)) {
     autoCreateBlock();
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index dd81c8e0a3d543..9f0527317eca95 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -454,11 +454,10 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
   return isDeadRoot;
 }
 
-// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
-// the coroutine statement. `Block` is the CFGBlock containing the `DeadStmt`.
-static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
+template <class... Ts>
+static bool isDeadStmtIn(const Stmt *DeadStmt, const CFGBlock *Block) {
   // The coroutine statement, co_return, co_await, or co_yield.
-  const Stmt *CoroStmt = nullptr;
+  const Stmt *TargetStmt = nullptr;
   // Find the first coroutine statement after the DeadStmt in the block.
   bool AfterDeadStmt = false;
   for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
@@ -467,32 +466,29 @@ static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
       const Stmt *S = CS->getStmt();
       if (S == DeadStmt)
         AfterDeadStmt = true;
-      if (AfterDeadStmt &&
-          // For simplicity, we only check simple coroutine statements.
-          (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
-        CoroStmt = S;
+      if (AfterDeadStmt && llvm::isa<Ts...>(S)) {
+        TargetStmt = S;
         break;
       }
     }
-  if (!CoroStmt)
+  if (!TargetStmt)
     return false;
   struct Checker : DynamicRecursiveASTVisitor {
     const Stmt *DeadStmt;
-    bool CoroutineSubStmt = false;
+    bool IsSubStmtOfTargetStmt = false;
     Checker(const Stmt *S) : DeadStmt(S) {
-      // Statements captured in the CFG can be implicit.
       ShouldVisitImplicitCode = true;
     }
 
     bool VisitStmt(Stmt *S) override {
       if (S == DeadStmt)
-        CoroutineSubStmt = true;
+        IsSubStmtOfTargetStmt = true;
       return true;
     }
   };
   Checker checker(DeadStmt);
-  checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
-  return checker.CoroutineSubStmt;
+  checker.TraverseStmt(const_cast<Stmt *>(TargetStmt));
+  return checker.IsSubStmtOfTargetStmt;
 }
 
 static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
@@ -503,7 +499,12 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
   // Coroutine statements are never considered dead statements, because removing
   // them may change the function semantic if it is the only coroutine statement
   // of the coroutine.
-  return !isInCoroutineStmt(S, Block);
+  //
+  // If the dead stmt is a sub-stmt of CXXDefaultInitExpr and CXXDefaultArgExpr,
+  // we would rather expect to find CXXDefaultInitExpr and CXXDefaultArgExpr as
+  // a valid dead stmt.
+  return !isDeadStmtIn<CoreturnStmt, CoroutineSuspendExpr, CXXDefaultArgExpr,
+                       CXXDefaultInitExpr>(S, Block);
 }
 
 const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c7472ce92703b..baea4ffef1799e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5489,6 +5489,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
 
   bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
   bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+  bool HasRebuiltInit = false;
   std::optional<ExpressionEvaluationContextRecord::InitializationContext>
       InitializationContext =
           OutermostDeclarationWithDelayedImmediateInvocations();
@@ -5546,6 +5547,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
       if (Res.isInvalid())
         return ExprError();
       Init = Res.get();
+      HasRebuiltInit = true;
     }
   }
 
@@ -5555,7 +5557,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
     return ExprError();
 
   return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
-                                   Init, InitializationContext->Context);
+                                   InitializationContext->Context, Init,
+                                   HasRebuiltInit);
 }
 
 static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@@ -5597,6 +5600,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
 
   bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
   bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+  bool HasRebuiltInit = false;
   EnterExpressionEvaluationContext EvalContext(
       *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
 
@@ -5658,6 +5662,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
       return ExprError();
     }
     Init = Res.get();
+    HasRebuiltInit = true;
   }
 
   if (Field->getInClassInitializer()) {
@@ -5680,7 +5685,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
 
     return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc,
                                       Field, InitializationContext->Context,
-                                      Init);
+                                      Init, HasRebuiltInit);
   }
 
   // DR1351:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1465bba87724b9..100a8ba03c9c1d 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3403,9 +3403,10 @@ class TreeTransform {
   /// require any semantic analysis. Subclasses may override this routine to
   /// provide different behavior.
   ExprResult RebuildCXXDefaultArgExpr(SourceLocation Loc, ParmVarDecl *Param,
-                                      Expr *RewrittenExpr) {
+                                      Expr *RewrittenExpr, bool HasRebuiltInit) {
     return CXXDefaultArgExpr::Create(getSema().Context, Loc, Param,
-                                     RewrittenExpr, getSema().CurContext);
+                                     getSema().CurContext, RewrittenExpr,
+                                     HasRebuiltInit);
   }
 
   /// Build a new C++11 default-initialization expression.
@@ -13765,7 +13766,7 @@ TreeTransform<Derived>::TransformCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
     return E;
 
   return getDerived().RebuildCXXDefaultArgExpr(E->getUsedLocation(), Param,
-                                               InitRes.get());
+                                               InitRes.get(), E->hasRebuiltInit());
 }
 
 template<typename Derived>
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index c39a1950a6cf24..43c2e1df948138 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1879,6 +1879,7 @@ void ASTStmtReader::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
   E->UsedContext = readDeclAs<DeclContext>();
   E->CXXDefaultArgExprBits.Loc = readSourceLocation();
   E->CXXDefaultArgExprBits.HasRewrittenInit = Record.readInt();
+  E->CXXDefaultArgExprBits.HasRebuiltInit = Record.readInt();
   if (E->CXXDefaultArgExprBits.HasRewrittenInit)
     *E->getTrailingObjects<Expr *>() = Record.readSubExpr();
 }
@@ -1886,6 +1887,7 @@ void ASTStmtReader::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
 void ASTStmtReader::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
   VisitExpr(E);
   E->CXXDefaultInitExprBits.HasRewrittenInit = Record.readInt();
+  E->CXXDefaultInitExprBits.HasRebuiltInit = Record.readInt();
   E->Field = readDeclAs<FieldDecl>();
   E->UsedContext = readDeclAs<DeclContext>();
   E->CXXDefaultInitExprBits.Loc = readSourceLocation();
@@ -4095,12 +4097,14 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) {
 
     case EXPR_CXX_DEFAULT_ARG:
       S = CXXDefaultArgExpr::CreateEmpty(
-          Context, /*HasRewrittenInit=*/Record[ASTStmtReader::NumExprFields]);
+          Context, /*HasRewrittenInit=*/Record[ASTStmtReader::NumExprFields],
+          /*HasRebuiltInit=*/Record[ASTStmtReader::NumExprFields + 1]);
       break;
 
     case EXPR_CXX_DEFAULT_INIT:
       S = CXXDefaultInitExpr::CreateEmpty(
-          Context, /*HasRewrittenInit=*/Record[ASTStmtReader::NumExprFields]);
+          Context, /*HasRewrittenInit=*/Record[ASTStmtReader::NumExprFields],
+          /*HasRebuiltInit=*/Record[ASTStmtReader::NumExprFields + 1]);
       break;
 
     case EXPR_CXX_BIND_TEMPORARY:
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index e7f567ff59a8ad..39a5234e34bd21 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1880,6 +1880,7 @@ void ASTStmtWriter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
   Record.AddDeclRef(cast_or_null<Decl>(E->getUsedContext()));
   Record.AddSourceLocation(E->getUsedLocation());
   Record.push_back(E->hasRewrittenInit());
+  Record.push_back(E->hasRebuiltInit());
   if (E->hasRewrittenInit())
     Record.AddStmt(E->getRewrittenExpr());
   Code = serialization::EXPR_CXX_DEFAULT_ARG;
@@ -1888,6 +1889,7 @@ void ASTStmtWriter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
 void ASTStmtWriter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
   VisitExpr(E);
   Record.push_back(E->hasRewrittenInit());
+  Record.push_back(E->hasRebuiltInit());
   Record.AddDeclRef(E->getField());
   Record.AddDeclRef(cast_or_null<Decl>(E->getUsedContext()));
   Record.AddSourceLocation(E->getExprLoc());
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 22eab9f66418d4..dd1532e1e5819a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1986,33 +1986,45 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       ExplodedNodeSet Tmp;
       StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx);
 
-      const Expr *ArgE;
-      if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S))
+      bool HasRebuiltInit = false;
+      const Expr *ArgE = nullptr;
+      if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) {
         ArgE = DefE->getExpr();
-      else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S))
+        HasRebuiltInit = DefE->hasRebuiltInit();
+      } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) {
         ArgE = DefE->getExpr();
-      else
+        HasRebuiltInit = DefE->hasRebuiltInit();
+      } else
         llvm_unreachable("unknown constant wrapper kind");
 
-      bool IsTemporary = false;
-      if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
-        ArgE = MTE->getSubExpr();
-        IsTemporary = true;
-      }
+      if (HasRebuiltInit) {
+        for (auto *N : PreVisit) {
+          ProgramStateRef state = N->getState();
+          const LocationContext *LCtx = N->getLocationContext();
+          state = state->BindExpr(S, LCtx, state->getSVal(ArgE, LCtx));
+          Bldr2.generateNode(S, N, state);
+        }
+      } else {
+        // If it's not rewritten, the contents of these expressions are not
+        // actually part of the current function, so we fall back to constant
+        // evaluation.
+        bool IsTemporary = false;
+        if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) {
+          ArgE = MTE->getSubExpr();
+          IsTemporary = true;
+        }
+
+        std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
+        const LocationContext *LCtx = Pred->getLocationContext();
+        for (auto *I : PreVisit) {
+          ProgramStateRef State = I->getState();
+          State = State->BindExpr(S, LCtx, ConstantVal.value_or(UnknownVal()));
+          if (IsTemporary)
+            State = createTemporaryRegionIfNeeded(State, LCtx, cast<Expr>(S),
+                                                  cast<Expr>(S));
 
-      std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE);
-      if (!ConstantVal)
-        ConstantVal = UnknownVal();
-
-      const LocationContext *LCtx = Pred->getLocationContext();
-      for (const auto I : PreVisit) {
-        ProgramStateRef State = I->getState();
-        State = State->BindExpr(S, LCtx, *ConstantVal);
-        if (IsTemporary)
-          State = createTemporaryRegionIfNeeded(State, LCtx,
-                                                cast<Expr>(S),
-                                                cast<Expr>(S));
-        Bldr2.generateNode(S, I, State);
+          Bldr2.generateNode(S, I, State);
+        }
       }
 
       getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this);
diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp
index 4458ad294af7cb..524f4e0c400d17 100644
--- a/clang/test/Analysis/lifetime-extended-regions.cpp
+++ b/clang/test/Analysis/lifetime-extended-regions.cpp
@@ -121,11 +121,10 @@ void aggregateWithReferences() {
   clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
   clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
   
-  // FIXME: clang currently support extending lifetime of object bound to reference members of aggregates,
-  // that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change.
-  // The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
+  // The lifetime lifetime of object bound to reference members of aggregates,
+  // that are created from default member initializer was extended.
   RefAggregate defaultInitExtended{i};
-  clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
+  clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }}
 }
 
 void lambda() {
diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index e6f5bc5ef8e127..8c23822dc16e0a 100644
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -414,3 +414,42 @@ void tautological_compare(bool x, int y) {
     calledFun();
 
 }
+
+namespace test_rebuilt_default_arg {
+struct A {
+  explicit A(int = __builtin_LINE());
+};
+
+int h(int a) {
+  return 3;
+  A();  // expected-warning {{will never be executed}}
+}
+
+struct Temp {
+  Temp();
+  ~Temp();
+};
+
+struct B {
+  explicit B(const Temp &t = Temp());
+};
+int f(int a) {
+  return 3;
+  B();  // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_arg
+namespace test_rebuilt_default_init {
+
+struct A {
+  A();
+  ~A();
+};
+
+struct B {
+  const A &t = A();
+};
+int f(int a) {
+  return 3;
+  A{};  // expected-warning {{will never be executed}}
+}
+} // namespace test_rebuilt_default_init



More information about the cfe-commits mailing list