https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/104556
>From ea86000bcba54e542a9a0f940451f15ac521e32b Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Mon, 19 Aug 2024 13:46:16 +0200 Subject: [PATCH] [clang] Diagnose dangling issues for cases where a gsl-pointer is construct from a gsl-owner object in a container. --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Sema/CheckExprLifetime.cpp | 28 ++++++++++--------- .../Sema/warn-lifetime-analysis-nocfg.cpp | 28 +++++++++++++++++++ 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ffdd063ec99037..c9864abc593e5e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -214,6 +214,8 @@ Improvements to Clang's diagnostics - Clang now diagnoses the use of ``main`` in an ``extern`` context as invalid according to [basic.start.main] p3. Fixes #GH101512. +- Clang now diagnoses dangling cases where a gsl-pointer is constructed from a gsl-owner object inside a container (#GH100384). + Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7389046eaddde1..9c5b00da592417 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -328,13 +328,16 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call, LocalVisitor Visit) { - auto VisitPointerArg = [&](const Decl *D, Expr *Arg, bool Value) { + auto VisitPointerArg = [&](const FunctionDecl *Callee, Expr *Arg) { // 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) { + auto ReturnType = Callee->getReturnType(); + // Once we initialized a value with a non gsl-owner reference, it can no + // longer dangle. + if (ReturnType->isReferenceType() && + !isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) { for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) { if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit) continue; @@ -344,9 +347,11 @@ static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call, break; } } - Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit - : IndirectLocalPathEntry::GslReferenceInit, - Arg, D}); + + Path.push_back({!ReturnType->isReferenceType() + ? IndirectLocalPathEntry::GslPointerInit + : IndirectLocalPathEntry::GslReferenceInit, + Arg, Callee}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, Visit, @@ -360,21 +365,18 @@ static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call, 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()); + VisitPointerArg(MD, MCE->getImplicitObjectArgument()); 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()); + VisitPointerArg(Callee, OCE->getArg(0)); 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()); + VisitPointerArg(Callee, CE->getArg(0)); return; } @@ -382,7 +384,7 @@ static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *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); + VisitPointerArg(Ctor, CCE->getArgs()[0]); } } diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 09dfb2b5d96a89..710eb361bfc915 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -275,6 +275,34 @@ int &danglingRawPtrFromLocal3() { return *o; // expected-warning {{reference to stack memory associated with local variable 'o' returned}} } +// GH100384 +std::string_view containerWithAnnotatedElements() { + std::string_view c1 = std::vector<std::string>().at(0); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} + c1 = std::vector<std::string>().at(0); // expected-warning {{object backing the pointer}} + + // no warning on constructing from gsl-pointer + std::string_view c2 = std::vector<std::string_view>().at(0); + + std::vector<std::string> local; + return local.at(0); // expected-warning {{address of stack memory associated with local variable}} +} + +std::string_view localUniquePtr(int i) { + std::unique_ptr<std::string> c1; + if (i) + return *c1; // expected-warning {{address of stack memory associated with local variable}} + std::unique_ptr<std::string_view> c2; + return *c2; // expect no-warning. +} + +std::string_view localOptional(int i) { + std::optional<std::string> o; + if (i) + return o.value(); // expected-warning {{address of stack memory associated with local variable}} + std::optional<std::string_view> abc; + return abc.value(); // expect no warning +} + const char *danglingRawPtrFromTemp() { return std::basic_string<char>().c_str(); // expected-warning {{returning address of local temporary object}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits