llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Haojian Wu (hokein) <details> <summary>Changes</summary> Reland the remaining change of https://github.com/llvm/llvm-project/pull/104906, with a proper fix for the regression. --- Patch is 22.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105884.diff 2 Files Affected: - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+61-95) - (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+11) ``````````diff diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index c1362559536962..ef43a0d25e0c9e 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -237,13 +237,11 @@ static bool pathContainsInit(IndirectLocalPath &Path) { static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Expr *Init, LocalVisitor Visit, - bool RevisitSubinits, - bool EnableLifetimeWarnings); + bool RevisitSubinits); static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, Expr *Init, ReferenceKind RK, - LocalVisitor Visit, - bool EnableLifetimeWarnings); + LocalVisitor Visit); template <typename T> static bool isRecordWithAttr(QualType Type) { if (auto *RD = Type->getAsCXXRecordDecl()) @@ -365,8 +363,7 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { // Visit lifetimebound or gsl-pointer arguments. static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, - LocalVisitor Visit, - bool EnableLifetimeWarnings) { + LocalVisitor Visit) { const FunctionDecl *Callee; ArrayRef<Expr *> Args; @@ -381,6 +378,8 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, if (!Callee) return; + bool EnableGSLAnalysis = !Callee->getASTContext().getDiagnostics().isIgnored( + diag::warn_dangling_lifetime_pointer, SourceLocation()); Expr *ObjectArg = nullptr; if (isa<CXXOperatorCallExpr>(Call) && Callee->isCXXInstanceMember()) { ObjectArg = Args[0]; @@ -393,11 +392,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, - Visit, - /*EnableLifetimeWarnings=*/false); + Visit); else - visitLocalsRetainedByInitializer(Path, Arg, Visit, true, - /*EnableLifetimeWarnings=*/false); + visitLocalsRetainedByInitializer(Path, Arg, Visit, true); Path.pop_back(); }; auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) { @@ -408,7 +405,8 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // Once we initialized a value with a reference, it can no longer dangle. if (!Value) { for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) { - if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit) + if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit || + PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall) continue; if (PE.Kind == IndirectLocalPathEntry::GslPointerInit || PE.Kind == IndirectLocalPathEntry::GslPointerAssignment) @@ -421,11 +419,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, Arg, D}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, - Visit, - /*EnableLifetimeWarnings=*/true); + Visit); else - visitLocalsRetainedByInitializer(Path, Arg, Visit, true, - /*EnableLifetimeWarnings=*/true); + visitLocalsRetainedByInitializer(Path, Arg, Visit, true); Path.pop_back(); }; @@ -448,7 +444,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, CheckCoroObjArg = false; if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg) VisitLifetimeBoundArg(Callee, ObjectArg); - else if (EnableLifetimeWarnings) { + else if (EnableGSLAnalysis) { if (auto *CME = dyn_cast<CXXMethodDecl>(Callee); CME && shouldTrackImplicitObjectArg(CME)) VisitGSLPointerArg(Callee, ObjectArg, @@ -461,15 +457,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, I != N; ++I) { if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); - else if (EnableLifetimeWarnings && I == 0) { + else if (EnableGSLAnalysis && I == 0) { // GSL if (shouldTrackFirstArgument(Callee)) { VisitGSLPointerArg(Callee, Args[0], !Callee->getReturnType()->isReferenceType()); - } else { - if (auto *CCE = dyn_cast<CXXConstructExpr>(Call); - CCE && CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) - VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0], - true); + } else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call); + CCE && + CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) { + VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0], + true); } } } @@ -479,8 +475,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, /// glvalue expression \c Init. static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, Expr *Init, ReferenceKind RK, - LocalVisitor Visit, - bool EnableLifetimeWarnings) { + LocalVisitor Visit) { RevertToOldSizeRAII RAII(Path); // Walk past any constructs which we can lifetime-extend across. @@ -517,8 +512,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, else // We can't lifetime extend through this but we might still find some // retained temporaries. - return visitLocalsRetainedByInitializer(Path, Init, Visit, true, - EnableLifetimeWarnings); + return visitLocalsRetainedByInitializer(Path, Init, Visit, true); } // Step into CXXDefaultInitExprs so we can diagnose cases where a @@ -532,21 +526,18 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init)) { if (Visit(Path, Local(MTE), RK)) - visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), Visit, true); } if (auto *M = dyn_cast<MemberExpr>(Init)) { // Lifetime of a non-reference type field is same as base object. if (auto *F = dyn_cast<FieldDecl>(M->getMemberDecl()); F && !F->getType()->isReferenceType()) - visitLocalsRetainedByInitializer(Path, M->getBase(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, M->getBase(), Visit, true); } if (isa<CallExpr>(Init)) - return visitFunctionCallArguments(Path, Init, Visit, - EnableLifetimeWarnings); + return visitFunctionCallArguments(Path, Init, Visit); switch (Init->getStmtClass()) { case Stmt::DeclRefExprClass: { @@ -565,8 +556,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, } else if (VD->getInit() && !isVarOnPath(Path, VD)) { Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD}); visitLocalsRetainedByReferenceBinding(Path, VD->getInit(), - RK_ReferenceBinding, Visit, - EnableLifetimeWarnings); + RK_ReferenceBinding, Visit); } } break; @@ -578,15 +568,13 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, // handling all sorts of rvalues passed to a unary operator. const UnaryOperator *U = cast<UnaryOperator>(Init); if (U->getOpcode() == UO_Deref) - visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, U->getSubExpr(), Visit, true); break; } case Stmt::ArraySectionExprClass: { - visitLocalsRetainedByInitializer(Path, - cast<ArraySectionExpr>(Init)->getBase(), - Visit, true, EnableLifetimeWarnings); + visitLocalsRetainedByInitializer( + Path, cast<ArraySectionExpr>(Init)->getBase(), Visit, true); break; } @@ -594,11 +582,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, case Stmt::BinaryConditionalOperatorClass: { auto *C = cast<AbstractConditionalOperator>(Init); if (!C->getTrueExpr()->getType()->isVoidType()) - visitLocalsRetainedByReferenceBinding(Path, C->getTrueExpr(), RK, Visit, - EnableLifetimeWarnings); + visitLocalsRetainedByReferenceBinding(Path, C->getTrueExpr(), RK, Visit); if (!C->getFalseExpr()->getType()->isVoidType()) - visitLocalsRetainedByReferenceBinding(Path, C->getFalseExpr(), RK, Visit, - EnableLifetimeWarnings); + visitLocalsRetainedByReferenceBinding(Path, C->getFalseExpr(), RK, Visit); break; } @@ -621,8 +607,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, /// the prvalue expression \c Init. static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Expr *Init, LocalVisitor Visit, - bool RevisitSubinits, - bool EnableLifetimeWarnings) { + bool RevisitSubinits) { RevertToOldSizeRAII RAII(Path); Expr *Old; @@ -663,18 +648,16 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, if (VD && VD->getType().isConstQualified() && VD->getInit() && !isVarOnPath(Path, VD)) { Path.push_back({IndirectLocalPathEntry::VarInit, DRE, VD}); - visitLocalsRetainedByInitializer( - Path, VD->getInit(), Visit, true, EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, VD->getInit(), Visit, + true); } } else if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L)) { if (MTE->getType().isConstQualified()) visitLocalsRetainedByInitializer(Path, MTE->getSubExpr(), - Visit, true, - EnableLifetimeWarnings); + Visit, true); } return false; - }, - EnableLifetimeWarnings); + }); // We assume that objects can be retained by pointers cast to integers, // but not if the integer is cast to floating-point type or to _Complex. @@ -703,9 +686,8 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, // Model array-to-pointer decay as taking the address of the array // lvalue. Path.push_back({IndirectLocalPathEntry::AddressOf, CE}); - return visitLocalsRetainedByReferenceBinding(Path, CE->getSubExpr(), - RK_ReferenceBinding, Visit, - EnableLifetimeWarnings); + return visitLocalsRetainedByReferenceBinding( + Path, CE->getSubExpr(), RK_ReferenceBinding, Visit); default: return; @@ -720,8 +702,7 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, // lifetime of the array exactly like binding a reference to a temporary. if (auto *ILE = dyn_cast<CXXStdInitializerListExpr>(Init)) return visitLocalsRetainedByReferenceBinding(Path, ILE->getSubExpr(), - RK_StdInitializerList, Visit, - EnableLifetimeWarnings); + RK_StdInitializerList, Visit); if (InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) { // We already visited the elements of this initializer list while @@ -732,14 +713,12 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, if (ILE->isTransparent()) return visitLocalsRetainedByInitializer(Path, ILE->getInit(0), Visit, - RevisitSubinits, - EnableLifetimeWarnings); + RevisitSubinits); if (ILE->getType()->isArrayType()) { for (unsigned I = 0, N = ILE->getNumInits(); I != N; ++I) visitLocalsRetainedByInitializer(Path, ILE->getInit(I), Visit, - RevisitSubinits, - EnableLifetimeWarnings); + RevisitSubinits); return; } @@ -752,14 +731,12 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, if (RD->isUnion() && ILE->getInitializedFieldInUnion() && ILE->getInitializedFieldInUnion()->getType()->isReferenceType()) visitLocalsRetainedByReferenceBinding(Path, ILE->getInit(0), - RK_ReferenceBinding, Visit, - EnableLifetimeWarnings); + RK_ReferenceBinding, Visit); else { unsigned Index = 0; for (; Index < RD->getNumBases() && Index < ILE->getNumInits(); ++Index) visitLocalsRetainedByInitializer(Path, ILE->getInit(Index), Visit, - RevisitSubinits, - EnableLifetimeWarnings); + RevisitSubinits); for (const auto *I : RD->fields()) { if (Index >= ILE->getNumInits()) break; @@ -768,14 +745,13 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Expr *SubInit = ILE->getInit(Index); if (I->getType()->isReferenceType()) visitLocalsRetainedByReferenceBinding(Path, SubInit, - RK_ReferenceBinding, Visit, - EnableLifetimeWarnings); + RK_ReferenceBinding, Visit); else // This might be either aggregate-initialization of a member or // initialization of a std::initializer_list object. Regardless, // we should recursively lifetime-extend that initializer. - visitLocalsRetainedByInitializer( - Path, SubInit, Visit, RevisitSubinits, EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, SubInit, Visit, + RevisitSubinits); ++Index; } } @@ -796,10 +772,9 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Path.push_back({IndirectLocalPathEntry::LambdaCaptureInit, E, &Cap}); if (E->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, E, RK_ReferenceBinding, - Visit, EnableLifetimeWarnings); + Visit); else - visitLocalsRetainedByInitializer(Path, E, Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, E, Visit, true); if (Cap.capturesVariable()) Path.pop_back(); } @@ -813,16 +788,14 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Expr *Arg = MTE->getSubExpr(); Path.push_back({IndirectLocalPathEntry::TemporaryCopy, Arg, CCE->getConstructor()}); - visitLocalsRetainedByInitializer(Path, Arg, Visit, true, - /*EnableLifetimeWarnings*/ false); + visitLocalsRetainedByInitializer(Path, Arg, Visit, true); Path.pop_back(); } } } if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) - return visitFunctionCallArguments(Path, Init, Visit, - EnableLifetimeWarnings); + return visitFunctionCallArguments(Path, Init, Visit); switch (Init->getStmtClass()) { case Stmt::UnaryOperatorClass: { @@ -838,8 +811,7 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, Path.push_back({IndirectLocalPathEntry::AddressOf, UO}); visitLocalsRetainedByReferenceBinding(Path, UO->getSubExpr(), - RK_ReferenceBinding, Visit, - EnableLifetimeWarnings); + RK_ReferenceBinding, Visit); } break; } @@ -852,11 +824,9 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, break; if (BO->getLHS()->getType()->isPointerType()) - visitLocalsRetainedByInitializer(Path, BO->getLHS(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, BO->getLHS(), Visit, true); else if (BO->getRHS()->getType()->isPointerType()) - visitLocalsRetainedByInitializer(Path, BO->getRHS(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, BO->getRHS(), Visit, true); break; } @@ -866,11 +836,9 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, // In C++, we can have a throw-expression operand, which has 'void' type // and isn't interesting from a lifetime perspective. if (!C->getTrueExpr()->getType()->isVoidType()) - visitLocalsRetainedByInitializer(Path, C->getTrueExpr(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, C->getTrueExpr(), Visit, true); if (!C->getFalseExpr()->getType()->isVoidType()) - visitLocalsRetainedByInitializer(Path, C->getFalseExpr(), Visit, true, - EnableLifetimeWarnings); + visitLocalsRetainedByInitializer(Path, C->getFalseExpr(), Visit, true); break; } @@ -972,8 +940,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, const InitializedEntity *ExtendingEntity, LifetimeKind LK, - const AssignedEntity *AEntity, Expr *Init, - bool EnableLifetimeWarnings) { + const AssignedEntity *AEntity, Expr *Init) { assert((AEntity && LK == LK_Assignment) || (InitEntity && LK != LK_Assignment)); // If this entity doesn't have an interesting lifetime, don't bother looking @@ -1267,19 +1234,20 @@ static void checkExprLifetimeImpl(Sema &SemaRef, }; llvm::SmallVector<IndirectLocalPathEntry, 8> Path; - if (EnableLifetimeWarnings && LK == LK_Assignment && + if (!S... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/105884 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
