https://github.com/aeft updated https://github.com/llvm/llvm-project/pull/197864
>From 648ef003aa300c1f289faea2ccfdd71d544d60ef Mon Sep 17 00:00:00 2001 From: Zhijie Wang <[email protected]> Date: Thu, 14 May 2026 21:03:48 -0700 Subject: [PATCH] [LifetimeSafety] Fix false negative for GSL Owner methods inherited from a non-Owner base --- .../LifetimeSafety/LifetimeAnnotations.h | 7 ++- .../LifetimeSafety/FactsGenerator.cpp | 5 ++- .../LifetimeSafety/LifetimeAnnotations.cpp | 18 ++++---- clang/test/Sema/warn-lifetime-safety.cpp | 44 +++++++++++++++++++ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index f418f8a5132ec..67ca4e80ca74b 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -57,8 +57,13 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD); // container iterators (begin, end), data accessors (c_str, data, get), // element accessors (operator[], operator*, front, back, at), or propagating // operations (operator+, operator-, operator++, operator--). +// +// `ActualObjectType` is the object's actual type at the call site (e.g., the +// type before any `DerivedToBase` cast). It is included in the `gsl::Owner` +// check, so inherited methods on non-annotated bases are still tracked. bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee, - bool RunningUnderLifetimeSafety); + bool RunningUnderLifetimeSafety, + QualType ActualObjectType = QualType()); // Returns true if the first argument of a free function should be tracked for // GSL lifetime analysis. This applies to STL free functions that take a pointer diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 0a06548d881d1..566535357d233 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -882,7 +882,7 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, handleImplicitObjectFieldUses(Call, FD); if (!CallList) return; - auto IsArgLifetimeBound = [FD](unsigned I) -> bool { + auto IsArgLifetimeBound = [FD, &Args](unsigned I) -> bool { const ParmVarDecl *PVD = nullptr; if (const auto *Method = dyn_cast<CXXMethodDecl>(FD); Method && Method->isInstance() && !isa<CXXConstructorDecl>(FD)) { @@ -890,7 +890,8 @@ void FactsGenerator::handleFunctionCall(const Expr *Call, // For the 'this' argument, the attribute is on the method itself. return implicitObjectParamIsLifetimeBound(Method) || shouldTrackImplicitObjectArg( - Method, /*RunningUnderLifetimeSafety=*/true); + Method, /*RunningUnderLifetimeSafety=*/true, + Args[0]->IgnoreParenBaseCasts()->getType()); if ((I - 1) < Method->getNumParams()) // For explicit arguments, find the corresponding parameter // declaration. diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 393e558fd39c3..985722fee5833 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -121,15 +121,18 @@ static bool isReferenceOrPointerLikeType(QualType QT) { } bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee, - bool RunningUnderLifetimeSafety) { + bool RunningUnderLifetimeSafety, + QualType ActualObjectType) { if (!Callee) return false; + const bool IsObjectGslOwner = + isGslOwnerType(Callee->getFunctionObjectParameterType()) || + (!ActualObjectType.isNull() && isGslOwnerType(ActualObjectType)); if (auto *Conv = dyn_cast<CXXConversionDecl>(Callee)) - if (isGslPointerType(Conv->getConversionType()) && - Callee->getParent()->hasAttr<OwnerAttr>()) + if (isGslPointerType(Conv->getConversionType()) && IsObjectGslOwner) return true; if (!isGslPointerType(Callee->getFunctionObjectParameterType()) && - !isGslOwnerType(Callee->getFunctionObjectParameterType())) + !IsObjectGslOwner) return false; // Begin and end iterators. @@ -173,9 +176,8 @@ bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee, if (!Callee->getIdentifier()) // e.g., std::optional<T>::operator->() returns T*. return RunningUnderLifetimeSafety - ? Callee->getParent()->hasAttr<OwnerAttr>() && - Callee->getOverloadedOperator() == - OverloadedOperatorKind::OO_Arrow + ? IsObjectGslOwner && Callee->getOverloadedOperator() == + OverloadedOperatorKind::OO_Arrow : false; return IteratorMembers.contains(Callee->getName()) || InnerPointerGetters.contains(Callee->getName()) || @@ -184,7 +186,7 @@ bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee, if (Callee->getReturnType()->isReferenceType()) { if (!Callee->getIdentifier()) { auto OO = Callee->getOverloadedOperator(); - if (!Callee->getParent()->hasAttr<OwnerAttr>()) + if (!IsObjectGslOwner) return false; return OO == OverloadedOperatorKind::OO_Subscript || OO == OverloadedOperatorKind::OO_Star; diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index 30b450c333fbd..f627382448c52 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -3284,3 +3284,47 @@ void assign_non_capturing_to_function_ref(function_ref &r) { } } // namespace GH126600 + +namespace GH188832 { +inline namespace __1 { +template <class T> +struct __optional_storage_base { + T& operator*() &; + T* operator->(); + T& value() &; +}; + +template <class T> +struct [[gsl::Owner]] optional : __optional_storage_base<T> { + ~optional(); +}; +} // namespace __1 + +const MyObj& return_optional_deref() { + optional<MyObj> opt; + return *opt; // expected-warning {{address of stack memory is returned later}} \ + // expected-note {{returned here}} +} + +const MyObj& return_optional_value() { + optional<MyObj> opt; + return opt.value(); // expected-warning {{address of stack memory is returned later}} \ + // expected-note {{returned here}} +} + +const int* return_optional_arrow() { + optional<MyObj> opt; + return &opt->id; // expected-warning {{address of stack memory is returned later}} \ + // expected-note {{returned here}} +} + +void deref_use_after_scope() { + const MyObj* p; + { + optional<MyObj> opt; + p = &*opt; // expected-warning {{object whose reference is captured does not live long enough}} + } // expected-note {{destroyed here}} + (void)p->id; // expected-note {{later used here}} +} + +} // namespace GH188832 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
