https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/122088
>From 8c4f9bcb00c56f564abf6cfa115854681b48f7d9 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Tue, 17 Dec 2024 14:28:00 +0100 Subject: [PATCH 1/2] [clang] Refine the temporay object member access filtering for GSL pointer. --- clang/lib/Sema/CheckExprLifetime.cpp | 42 +++++++++++++------ clang/test/Sema/Inputs/lifetime-analysis.h | 2 +- .../Sema/warn-lifetime-analysis-nocfg.cpp | 28 +++++++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7109de03cadd12..d0d05e4ed980d5 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -200,6 +200,7 @@ struct IndirectLocalPathEntry { LifetimeBoundCall, TemporaryCopy, LambdaCaptureInit, + MemberExpr, GslReferenceInit, GslPointerInit, GslPointerAssignment, @@ -578,19 +579,6 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, Path.pop_back(); }; 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; - // Avoid false positives when the object is constructed from a conditional - // operator argument. A common case is: - // // 'ptr' might not be owned by the Owner object. - // std::string_view s = cond() ? Owner().ptr : sv; - if (const auto *Cond = - dyn_cast<AbstractConditionalOperator>(Arg->IgnoreImpCasts()); - Cond && isPointerLikeType(Cond->getType())) - return; - auto ReturnType = Callee->getReturnType(); // Once we initialized a value with a non gsl-owner reference, it can no @@ -710,6 +698,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, Init = ILE->getInit(0); } + if (MemberExpr *ME = dyn_cast<MemberExpr>(Init->IgnoreImpCasts())) + Path.push_back( + {IndirectLocalPathEntry::MemberExpr, ME, ME->getMemberDecl()}); // Step over any subobject adjustments; we may have a materialized // temporary inside them. Init = const_cast<Expr *>(Init->skipRValueSubobjectAdjustments()); @@ -1103,6 +1094,8 @@ shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) { for (auto Elem : Path) { if (Elem.Kind == IndirectLocalPathEntry::DefaultInit) return PathLifetimeKind::Extend; + if (Elem.Kind == IndirectLocalPathEntry::MemberExpr) + continue; if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit) return PathLifetimeKind::NoExtend; } @@ -1122,6 +1115,7 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I, case IndirectLocalPathEntry::GslPointerInit: case IndirectLocalPathEntry::GslPointerAssignment: case IndirectLocalPathEntry::ParenAggInit: + case IndirectLocalPathEntry::MemberExpr: // These exist primarily to mark the path as not permitting or // supporting lifetime extension. break; @@ -1151,6 +1145,7 @@ static bool pathOnlyHandlesGslPointer(const IndirectLocalPath &Path) { case IndirectLocalPathEntry::VarInit: case IndirectLocalPathEntry::AddressOf: case IndirectLocalPathEntry::LifetimeBoundCall: + case IndirectLocalPathEntry::MemberExpr: continue; case IndirectLocalPathEntry::GslPointerInit: case IndirectLocalPathEntry::GslReferenceInit: @@ -1184,6 +1179,26 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // At this point, Path represents a series of operations involving a // GSLPointer, either in the process of initialization or assignment. + // Process temporary base objects for MemberExpr cases, e.g. Temp().field. + for (const auto &E : Path) { + if (E.Kind == IndirectLocalPathEntry::MemberExpr) { + // Avoid interfering with the local base object. + if (pathContainsInit(Path)) + return Abandon; + + // We are not interested in the temporary base objects of gsl Pointers: + // auto p1 = Temp().ptr; // Here p1 might not dangle. + // However, we want to diagnose for gsl owner fields: + // auto p2 = Temp().owner; // Here p2 is dangling. + if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D); + FD && !FD->getType()->isReferenceType() && + isRecordWithAttr<OwnerAttr>(FD->getType())) { + return Report; + } + return Abandon; + } + } + // Note: A LifetimeBoundCall can appear interleaved in this sequence. // For example: // const std::string& Ref(const std::string& a [[clang::lifetimebound]]); @@ -1510,6 +1525,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, case IndirectLocalPathEntry::LifetimeBoundCall: case IndirectLocalPathEntry::TemporaryCopy: + case IndirectLocalPathEntry::MemberExpr: case IndirectLocalPathEntry::GslPointerInit: case IndirectLocalPathEntry::GslReferenceInit: case IndirectLocalPathEntry::GslPointerAssignment: diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h index f888e6ab94bb6a..d318033ff0cc45 100644 --- a/clang/test/Sema/Inputs/lifetime-analysis.h +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -52,7 +52,7 @@ struct vector { void push_back(const T&); void push_back(T&&); - + const T& back() const; void insert(iterator, T&&); }; diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 4c19367bb7f3dd..3b271af9dfa13f 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -806,3 +806,31 @@ std::string_view test2(int c, std::string_view sv) { } } // namespace GH120206 + +namespace GH120543 { +struct S { + std::string_view sv; + std::string s; +}; +struct Q { + const S* get() const [[clang::lifetimebound]]; +}; +void test1() { + std::string_view k1 = S().sv; // OK + std::string_view k2 = S().s; // expected-warning {{object backing the pointer will}} + + std::string_view k3 = Q().get()->sv; // OK + std::string_view k4 = Q().get()->s; // expected-warning {{object backing the pointer will}} +} + +struct Bar {}; +struct Foo { + std::vector<Bar> v; +}; +Foo getFoo(); +void test2() { + const Foo& foo = getFoo(); + const Bar& bar = foo.v.back(); // OK +} + +} // namespace GH120543 >From b5f6d0a46e4fbed78a30404bdb036e0b7387cab7 Mon Sep 17 00:00:00 2001 From: Haojian Wu <hokein...@gmail.com> Date: Thu, 9 Jan 2025 09:03:22 +0100 Subject: [PATCH 2/2] Add a lifetimebound testcase --- clang/test/Sema/warn-lifetime-analysis-nocfg.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 3b271af9dfa13f..48f52b7fc2788c 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -815,12 +815,18 @@ struct S { struct Q { const S* get() const [[clang::lifetimebound]]; }; + +std::string_view foo(std::string_view sv [[clang::lifetimebound]]); + void test1() { std::string_view k1 = S().sv; // OK std::string_view k2 = S().s; // expected-warning {{object backing the pointer will}} std::string_view k3 = Q().get()->sv; // OK std::string_view k4 = Q().get()->s; // expected-warning {{object backing the pointer will}} + + std::string_view lb1 = foo(S().s); // expected-warning {{object backing the pointer will}} + std::string_view lb2 = foo(Q().get()->s); // expected-warning {{object backing the pointer will}} } struct Bar {}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits