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 <yronglin...@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 <yronglin...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
