llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Haojian Wu (hokein) <details> <summary>Changes</summary> We currently have ad-hoc filtering logic for temporary object member access in `VisitGSLPointerArg`. This logic filters out more cases than it should, leading to false negatives. Furthermore, this location lacks sufficient context to implement a more accurate solution. This patch refines the filtering logic by moving it to the central filtering location, `analyzePathForGSLPointer`, consolidating the logic and avoiding scattered filtering across multiple places. As a result, the special handling for conditional operators (#<!-- -->120233) is no longer necessary. This change also resolves #<!-- -->120543. --- Full diff: https://github.com/llvm/llvm-project/pull/122088.diff 3 Files Affected: - (modified) clang/lib/Sema/CheckExprLifetime.cpp (+29-13) - (modified) clang/test/Sema/Inputs/lifetime-analysis.h (+1-1) - (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+28) ``````````diff 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 `````````` </details> https://github.com/llvm/llvm-project/pull/122088 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
