diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 70ff5dedab217..a75798ad6bd25 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 7389046eaddde..c136255953696 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(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(Call)) { - const auto *MD = cast_or_null(MCE->getDirectCallee()); - if (MD && shouldTrackImplicitObjectArg(MD)) - VisitPointerArg(MD, MCE->getImplicitObjectArgument(), - !MD->getReturnType()->isReferenceType()); - return; - } else if (auto *OCE = dyn_cast(Call)) { - FunctionDecl *Callee = OCE->getDirectCallee(); - if (Callee && Callee->isCXXInstanceMember() && - shouldTrackImplicitObjectArg(cast(Callee))) - VisitPointerArg(Callee, OCE->getArg(0), - !Callee->getReturnType()->isReferenceType()); - return; - } else if (auto *CE = dyn_cast(Call)) { - FunctionDecl *Callee = CE->getDirectCallee(); - if (Callee && shouldTrackFirstArgument(Callee)) - VisitPointerArg(Callee, CE->getArg(0), - !Callee->getReturnType()->isReferenceType()); - return; - } - - if (auto *CCE = dyn_cast(Call)) { - const auto *Ctor = CCE->getConstructor(); - const CXXRecordDecl *RD = Ctor->getParent(); - if (CCE->getNumArgs() > 0 && RD->hasAttr()) - 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 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(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(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()) 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(Call); + CCE && CCE->getConstructor()->getParent()->hasAttr()) + VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0], + true); + } + } } } @@ -557,11 +544,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, EnableLifetimeWarnings); } - if (isa(Init)) { - if (EnableLifetimeWarnings) - handleGslAnnotatedTypes(Path, Init, Visit); - return visitLifetimeBoundArguments(Path, Init, Visit); - } + if (isa(Init)) + return visitFunctionCallArguments(Path, Init, Visit, + EnableLifetimeWarnings); switch (Init->getStmtClass()) { case Stmt::DeclRefExprClass: { @@ -835,11 +820,9 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path, } } - if (isa(Init) || isa(Init)) { - if (EnableLifetimeWarnings) - handleGslAnnotatedTypes(Path, Init, Visit); - return visitLifetimeBoundArguments(Path, Init, Visit); - } + if (isa(Init) || isa(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 09dfb2b5d96a8..cd1904db32710 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{} - 2; // Used to be false positive. } + +namespace GH93386 { +// verify no duplicated diagnostics are emitted. +struct [[gsl::Pointer]] S { + S(const std::vector& abc [[clang::lifetimebound]]); +}; + +S test(std::vector a) { + return S(a); // expected-warning {{address of stack memory associated with}} +} + +auto s = S(std::vector()); // 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 a) { + if (i) + return std::move(*a); + return std::move(a.value()); +} +}