Author: bgra8 Date: 2024-06-06T11:46:33+02:00 New Revision: 86295dc197db2f08f4eb582ed1026a8f74ac3338
URL: https://github.com/llvm/llvm-project/commit/86295dc197db2f08f4eb582ed1026a8f74ac3338 DIFF: https://github.com/llvm/llvm-project/commit/86295dc197db2f08f4eb582ed1026a8f74ac3338.diff LOG: Revert "[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression (#91879)" (#94597) This depends on https://github.com/llvm/llvm-project/pull/92527 which needs to be reverted due to https://github.com/llvm/llvm-project/pull/92527#issuecomment-2149120420. This reverts commit 905b402a5d8f1490d668f40942390ebd6e87aa8f. Co-authored-by: Bogdan Graur <bgr...@google.com> Added: Modified: clang/lib/AST/ParentMap.cpp clang/lib/Analysis/CFG.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/StaticAnalyzer/Core/ExprEngine.cpp clang/test/Analysis/cxx-uninitialized-object.cpp clang/test/Analysis/lifetime-extended-regions.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp index 534793b837bbb..3d6a1cc84c7b1 100644 --- a/clang/lib/AST/ParentMap.cpp +++ b/clang/lib/AST/ParentMap.cpp @@ -97,22 +97,6 @@ static void BuildParentMap(MapTy& M, Stmt* S, BuildParentMap(M, SubStmt, OVMode); } break; - case Stmt::CXXDefaultArgExprClass: - if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) { - if (Arg->hasRewrittenInit()) { - M[Arg->getExpr()] = S; - BuildParentMap(M, Arg->getExpr(), OVMode); - } - } - break; - case Stmt::CXXDefaultInitExprClass: - if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) { - if (Init->hasRewrittenInit()) { - 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 02317257c2740..64e6155de090c 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -556,10 +556,6 @@ 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); @@ -2258,10 +2254,16 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc, asc, ExternallyDestructed); case Stmt::CXXDefaultArgExprClass: - return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc); - case Stmt::CXXDefaultInitExprClass: - return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc); + // 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); case Stmt::CXXBindTemporaryExprClass: return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc); @@ -2431,40 +2433,6 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) { return B; } -CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg, - AddStmtChoice asc) { - if (Arg->hasRewrittenInit()) { - 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->hasRewrittenInit()) { - 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/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index fb5ca199b3fc6..d8c77e3e0a5cd 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5608,7 +5608,6 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { InitializationContext.emplace(Loc, Field, CurContext); Expr *Init = nullptr; - bool HasRewrittenInit = false; bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); bool InLifetimeExtendingContext = isInLifetimeExtendingContext(); @@ -5658,7 +5657,6 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { isa_and_present<ExprWithCleanups>(Field->getInClassInitializer()); if (V.HasImmediateCalls || InLifetimeExtendingContext || ContainsAnyTemporaries) { - HasRewrittenInit = true; ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field, CurContext}; ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer = @@ -5702,7 +5700,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc, Field, InitializationContext->Context, - HasRewrittenInit ? Init : nullptr); + Init); } // DR1351: diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 290d96611d466..197d673107285 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1971,45 +1971,33 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet Tmp; StmtNodeBuilder Bldr2(PreVisit, Tmp, *currBldrCtx); - bool HasRewrittenInit = false; - const Expr *ArgE = nullptr; - if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) { + const Expr *ArgE; + if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) ArgE = DefE->getExpr(); - HasRewrittenInit = DefE->hasRewrittenInit(); - } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) { + else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) ArgE = DefE->getExpr(); - HasRewrittenInit = DefE->hasRewrittenInit(); - } else + else llvm_unreachable("unknown constant wrapper kind"); - if (HasRewrittenInit) { - 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)); + bool IsTemporary = false; + if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) { + ArgE = MTE->getSubExpr(); + IsTemporary = true; + } - Bldr2.generateNode(S, I, State); - } + 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); } getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); diff --git a/clang/test/Analysis/cxx-uninitialized-object.cpp b/clang/test/Analysis/cxx-uninitialized-object.cpp index aee0dae15fbfa..e3fa8ae8d7f29 100644 --- a/clang/test/Analysis/cxx-uninitialized-object.cpp +++ b/clang/test/Analysis/cxx-uninitialized-object.cpp @@ -1114,27 +1114,27 @@ void fCXX11MemberInitTest1() { CXX11MemberInitTest1(); } -#ifdef PEDANTIC struct CXX11MemberInitTest2 { struct RecordType { - int a; // expected-note {{uninitialized field 'this->a'}} - int b; // expected-note {{uninitialized field 'this->b'}} + // TODO: we'd expect the note: {{uninitialized field 'this->rec.a'}} + int a; // no-note + // TODO: we'd expect the note: {{uninitialized field 'this->rec.b'}} + int b; // no-note RecordType(int) {} }; - RecordType rec = RecordType(int()); // expected-warning {{2 uninitialized fields}} + RecordType rec = RecordType(int()); int dontGetFilteredByNonPedanticMode = 0; CXX11MemberInitTest2() {} }; void fCXX11MemberInitTest2() { + // TODO: we'd expect the warning: {{2 uninitializeds field}} CXX11MemberInitTest2(); // no-warning } -#endif // PEDANTIC - //===----------------------------------------------------------------------===// // "Esoteric" primitive type tests. //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp index 524f4e0c400d1..4458ad294af7c 100644 --- a/clang/test/Analysis/lifetime-extended-regions.cpp +++ b/clang/test/Analysis/lifetime-extended-regions.cpp @@ -121,10 +121,11 @@ 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]+}}} }} - // The lifetime lifetime of object bound to reference members of aggregates, - // that are created from default member initializer was extended. + // 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]+}}} }} RefAggregate defaultInitExtended{i}; - clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }} + clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }} } void lambda() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
