Author: Haojian Wu Date: 2024-09-06T12:37:21+02:00 New Revision: a918fa117acfbb20d29039cb8bddab159a8173dc
URL: https://github.com/llvm/llvm-project/commit/a918fa117acfbb20d29039cb8bddab159a8173dc DIFF: https://github.com/llvm/llvm-project/commit/a918fa117acfbb20d29039cb8bddab159a8173dc.diff LOG: [clang] Emit -Wdangling diagnoses for cases where a gsl-pointer is construct from a gsl-owner object in a container. (#104556) The warning is not emitted for the case `string_view c = std::vector<std::string>({""}).at(0);` because we bail out during the visit of the LHS at [this point](https://github.com/llvm/llvm-project/blob/5d2c324fea2d7cf86ec50e4bb6b680acf89b2ed5/clang/lib/Sema/CheckExprLifetime.cpp#L341-L343) in the code. This bailout was introduced in [this commit](https://github.com/llvm/llvm-project/commit/bcd0798c47ca865f95226859893016a17402441e) to address a false positive with `vector<vector::iterator>({""}).at(0);`. This PR refines that fix by ensuring that, for initialization involving a gsl-pointer, we only consider constructor calls that take the gsl-owner object. Fixes #100384. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Sema/CheckExprLifetime.cpp clang/test/Sema/warn-lifetime-analysis-nocfg.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a2e91fd648cce2..684484ccd298fb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -288,6 +288,8 @@ Improvements to Clang's diagnostics - The lifetimebound and GSL analysis in clang are coherent, allowing clang to detect more use-after-free bugs. (#GH100549). +- Clang now diagnoses dangling cases where a gsl-pointer is constructed from a gsl-owner object inside a container (#GH100384). + - Clang now warns for u8 character literals used in C23 with ``-Wpre-c23-compat`` instead of ``-Wpre-c++17-compat``. Improvements to Clang's time-trace diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 8f4d5d50669f14..f1507ebb9a5068 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -403,13 +403,17 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, visitLocalsRetainedByInitializer(Path, Arg, Visit, true); Path.pop_back(); }; - auto VisitGSLPointerArg = [&](const Decl *D, Expr *Arg, bool Value) { + auto VisitGSLPointerArg = [&](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 || PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall) @@ -420,9 +424,10 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, break; } } - Path.push_back({Value ? IndirectLocalPathEntry::GslPointerInit - : IndirectLocalPathEntry::GslReferenceInit, - Arg, D}); + Path.push_back({ReturnType->isReferenceType() + ? IndirectLocalPathEntry::GslReferenceInit + : IndirectLocalPathEntry::GslPointerInit, + Arg, Callee}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, Visit); @@ -453,8 +458,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, else if (EnableGSLAnalysis) { if (auto *CME = dyn_cast<CXXMethodDecl>(Callee); CME && shouldTrackImplicitObjectArg(CME)) - VisitGSLPointerArg(Callee, ObjectArg, - !Callee->getReturnType()->isReferenceType()); + VisitGSLPointerArg(Callee, ObjectArg); } } @@ -465,13 +469,11 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); else if (EnableGSLAnalysis && I == 0) { if (shouldTrackFirstArgument(Callee)) { - VisitGSLPointerArg(Callee, Args[0], - !Callee->getReturnType()->isReferenceType()); + VisitGSLPointerArg(Callee, Args[0]); } else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call); CCE && CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) { - VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0], - true); + VisitGSLPointerArg(CCE->getConstructor(), Args[0]); } } } diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 67d1ceaa02d039..59357d0730a7d9 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
