https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/105838
>From c248c80164965e82807a8682d30ce387f7801cc4 Mon Sep 17 00:00:00 2001 From: Haojian Wu <[email protected]> Date: Fri, 23 Aug 2024 16:20:45 +0200 Subject: [PATCH 1/2] Reland "[clang] Merge lifetimebound and GSL code paths for lifetime analysis (#104906)" --- clang/docs/ReleaseNotes.rst | 2 + clang/lib/Sema/CheckExprLifetime.cpp | 126 ++++++++---------- .../Sema/warn-lifetime-analysis-nocfg.cpp | 20 +++ 3 files changed, 76 insertions(+), 72 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 70ff5dedab217f..a75798ad6bd251 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -231,6 +231,8 @@ Improvements to Clang's diagnostics - Clang now diagnoses when the result of a [[nodiscard]] function is discarded after being cast in C. Fixes #GH104391. +- Don't emit duplicated dangling diagnostics. (#GH93386). + - Improved diagnostic when trying to befriend a concept. (#GH45182). Improvements to Clang's time-trace diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7389046eaddde1..0459ead46a1716 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -326,66 +326,6 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { return false; } -static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call, - LocalVisitor Visit) { - auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) { - // We are not interested in the temporary base objects of gsl Pointers: - // Temp().ptr; // Here ptr might not dangle. - if (isa<MemberExpr>(Arg->IgnoreImpCasts())) - return; - // 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) - continue; - if (PE.Kind == IndirectLocalPathEntry::GslPointerInit || - PE.Kind == IndirectLocalPathEntry::GslPointerAssignment) - return; - break; - } - } - Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit - : IndirectLocalPathEntry::GslReferenceInit, - Arg, D}); - if (Arg->isGLValue()) - visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, - Visit, - /*EnableLifetimeWarnings=*/true); - else - visitLocalsRetainedByInitializer(Path, Arg, Visit, true, - /*EnableLifetimeWarnings=*/true); - Path.pop_back(); - }; - - if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) { - const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee()); - if (MD && shouldTrackImplicitObjectArg(MD)) - VisitPointerArg(MD, MCE->getImplicitObjectArgument(), - !MD->getReturnType()->isReferenceType()); - return; - } else if (auto *OCE = dyn_cast<CXXOperatorCallExpr>(Call)) { - FunctionDecl *Callee = OCE->getDirectCallee(); - if (Callee && Callee->isCXXInstanceMember() && - shouldTrackImplicitObjectArg(cast<CXXMethodDecl>(Callee))) - VisitPointerArg(Callee, OCE->getArg(0), - !Callee->getReturnType()->isReferenceType()); - return; - } else if (auto *CE = dyn_cast<CallExpr>(Call)) { - FunctionDecl *Callee = CE->getDirectCallee(); - if (Callee && shouldTrackFirstArgument(Callee)) - VisitPointerArg(Callee, CE->getArg(0), - !Callee->getReturnType()->isReferenceType()); - return; - } - - if (auto *CCE = dyn_cast<CXXConstructExpr>(Call)) { - const auto *Ctor = CCE->getConstructor(); - const CXXRecordDecl *RD = Ctor->getParent(); - if (CCE->getNumArgs() > 0 && RD->hasAttr<PointerAttr>()) - VisitPointerArg(Ctor->getParamDecl(0), CCE->getArgs()[0], true); - } -} - static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); if (!TSI) @@ -423,8 +363,10 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return false; } -static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call, - LocalVisitor Visit) { +// Visit lifetimebound or gsl-pointer arguments. +static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, + LocalVisitor Visit, + bool EnableLifetimeWarnings) { const FunctionDecl *Callee; ArrayRef<Expr *> Args; @@ -458,6 +400,34 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call, /*EnableLifetimeWarnings=*/false); Path.pop_back(); }; + auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) { + // We are not interested in the temporary base objects of gsl Pointers: + // Temp().ptr; // Here ptr might not dangle. + if (isa<MemberExpr>(Arg->IgnoreImpCasts())) + return; + // 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) + continue; + if (PE.Kind == IndirectLocalPathEntry::GslPointerInit || + PE.Kind == IndirectLocalPathEntry::GslPointerAssignment) + return; + break; + } + } + Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit + : IndirectLocalPathEntry::GslReferenceInit, + Arg, D}); + if (Arg->isGLValue()) + visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, + Visit, + /*EnableLifetimeWarnings=*/true); + else + visitLocalsRetainedByInitializer(Path, Arg, Visit, true, + /*EnableLifetimeWarnings=*/true); + Path.pop_back(); + }; bool CheckCoroCall = false; if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) { @@ -478,6 +448,12 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call, CheckCoroObjArg = false; if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg) VisitLifetimeBoundArg(Callee, ObjectArg); + else if (EnableLifetimeWarnings) { + if (auto *CME = dyn_cast<CXXMethodDecl>(Callee); + CME && shouldTrackImplicitObjectArg(CME)) + VisitGSLPointerArg(Callee, ObjectArg, + !Callee->getReturnType()->isReferenceType()); + } } for (unsigned I = 0, @@ -485,6 +461,17 @@ static void visitLifetimeBoundArguments(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) { + 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); + } + } } } @@ -557,11 +544,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, EnableLifetimeWarnings); } - if (isa<CallExpr>(Init)) { - if (EnableLifetimeWarnings) - handleGslAnnotatedTypes(Path, Init, Visit); - return visitLifetimeBoundArguments(Path, Init, Visit); - } + if (isa<CallExpr>(Init)) + return visitFunctionCallArguments(Path, Init, Visit, + EnableLifetimeWarnings); switch (Init->getStmtClass()) { case Stmt::DeclRefExprClass: { @@ -835,11 +820,8 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, } } - if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) { - if (EnableLifetimeWarnings) - handleGslAnnotatedTypes(Path, Init, Visit); - return visitLifetimeBoundArguments(Path, Init, Visit); - } + if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) + return visitFunctionCallArguments(Path, Init, Visit, EnableLifetimeWarnings); switch (Init->getStmtClass()) { case Stmt::UnaryOperatorClass: { diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 09dfb2b5d96a89..cd1904db327105 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -479,3 +479,23 @@ void testForBug49342() { auto it = std::iter<char>{} - 2; // Used to be false positive. } + +namespace GH93386 { +// verify no duplicated diagnostics are emitted. +struct [[gsl::Pointer]] S { + S(const std::vector<int>& abc [[clang::lifetimebound]]); +}; + +S test(std::vector<int> a) { + return S(a); // expected-warning {{address of stack memory associated with}} +} + +auto s = S(std::vector<int>()); // expected-warning {{temporary whose address is used as value of local variable}} + +// Verify no regression on the follow case. +std::string_view test2(int i, std::optional<std::string_view> a) { + if (i) + return std::move(*a); + return std::move(a.value()); +} +} >From 29f028a6eb75a66ab7646510c9fcc4bf0d5bf543 Mon Sep 17 00:00:00 2001 From: Haojian Wu <[email protected]> Date: Fri, 23 Aug 2024 17:06:59 +0200 Subject: [PATCH 2/2] clang-format --- clang/lib/Sema/CheckExprLifetime.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 0459ead46a1716..c1362559536962 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -452,7 +452,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, if (auto *CME = dyn_cast<CXXMethodDecl>(Callee); CME && shouldTrackImplicitObjectArg(CME)) VisitGSLPointerArg(Callee, ObjectArg, - !Callee->getReturnType()->isReferenceType()); + !Callee->getReturnType()->isReferenceType()); } } @@ -821,7 +821,8 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, } if (isa<CallExpr>(Init) || isa<CXXConstructExpr>(Init)) - return visitFunctionCallArguments(Path, Init, Visit, EnableLifetimeWarnings); + return visitFunctionCallArguments(Path, Init, Visit, + EnableLifetimeWarnings); switch (Init->getStmtClass()) { case Stmt::UnaryOperatorClass: { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
