[clang] d0646cb - [LifetimeSafety] Fix false negative for GSL Owner methods inherited from a non-Owner base (#197864)
via cfe-commits
cfe-commits at lists.llvm.org
Mon May 25 14:21:40 PDT 2026
Author: Zhijie Wang
Date: 2026-05-25T14:21:35-07:00
New Revision: d0646cbe307dd2bd10686f923061a7d981263e40
URL: https://github.com/llvm/llvm-project/commit/d0646cbe307dd2bd10686f923061a7d981263e40
DIFF: https://github.com/llvm/llvm-project/commit/d0646cbe307dd2bd10686f923061a7d981263e40.diff
LOG: [LifetimeSafety] Fix false negative for GSL Owner methods inherited from a non-Owner base (#197864)
- Take the implicit object's actual type (e.g., the type before any
`DerivedToBase` cast) into account when checking for GSL Owner. Other
`isGslOwnerType` call sites with the same pattern (e.g.,
`isGslOwnerType(MCE->getImplicitObjectArgument()->getType())` in
`VisitCXXMemberCallExpr`) lack a real-world trigger today and are
deferred to a follow-up.
- Unify the GSL Owner checks inside `shouldTrackImplicitObjectArg` so
they share a single source of truth.
Fixes: #188832
Added:
Modified:
clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
clang/lib/Sema/CheckExprLifetime.cpp
clang/test/Sema/warn-lifetime-safety.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
index ec05e67b05853..a97df7a08dfeb 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h
@@ -62,7 +62,8 @@ 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--).
-bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
+bool shouldTrackImplicitObjectArg(const Expr &ImplicitObjectArgument,
+ const CXXMethodDecl *Callee,
bool RunningUnderLifetimeSafety);
// Returns true if the first argument of a free function should be tracked for
@@ -81,6 +82,7 @@ bool shouldTrackSecondArgument(const FunctionDecl *FD);
bool isGslPointerType(QualType QT);
// Tells whether the type is annotated with [[gsl::Owner]].
bool isGslOwnerType(QualType QT);
+bool isGslOwnerType(const CXXRecordDecl *RD);
// Returns true if the given method is std::unique_ptr::release().
// This is treated as a move in lifetime analysis to avoid false-positives
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 582a8fd1645a8..9038f56689779 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -886,7 +886,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)) {
@@ -894,7 +894,7 @@ void FactsGenerator::handleFunctionCall(const Expr *Call,
// For the 'this' argument, the attribute is on the method itself.
return implicitObjectParamIsLifetimeBound(Method) ||
shouldTrackImplicitObjectArg(
- Method, /*RunningUnderLifetimeSafety=*/true);
+ *Args[0], Method, /*RunningUnderLifetimeSafety=*/true);
if ((I - 1) < Method->getNumParams())
// For explicit arguments, find the corresponding parameter
// declaration.
@@ -909,13 +909,13 @@ void FactsGenerator::handleFunctionCall(const Expr *Call,
}
return PVD ? PVD->hasAttr<clang::LifetimeBoundAttr>() : false;
};
- auto shouldTrackPointerImplicitObjectArg = [FD](unsigned I) -> bool {
+ auto shouldTrackPointerImplicitObjectArg = [FD, &Args](unsigned I) -> bool {
const auto *Method = dyn_cast<CXXMethodDecl>(FD);
if (!Method || !Method->isInstance())
return false;
return I == 0 &&
isGslPointerType(Method->getFunctionObjectParameterType()) &&
- shouldTrackImplicitObjectArg(Method,
+ shouldTrackImplicitObjectArg(*Args[0], Method,
/*RunningUnderLifetimeSafety=*/true);
};
if (Args.empty())
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
index 903edfbab5489..2f26e77d5a0eb 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp
@@ -128,16 +128,22 @@ static bool isReferenceOrPointerLikeType(QualType QT) {
return QT->isReferenceType() || isPointerLikeType(QT);
}
-bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
+bool shouldTrackImplicitObjectArg(const Expr &ImplicitObjectArgument,
+ const CXXMethodDecl *Callee,
bool RunningUnderLifetimeSafety) {
if (!Callee)
return false;
+ // Check both the declaring class and the call-site object: a gsl::Owner
+ // may inherit its accessors from a non-Owner base (e.g. libc++ optional).
+ const bool IsGslOwnerImplicitObject =
+ isGslOwnerType(Callee->getFunctionObjectParameterType()) ||
+ (RunningUnderLifetimeSafety &&
+ isGslOwnerType(ImplicitObjectArgument.getBestDynamicClassType()));
if (auto *Conv = dyn_cast<CXXConversionDecl>(Callee))
- if (isGslPointerType(Conv->getConversionType()) &&
- Callee->getParent()->hasAttr<OwnerAttr>())
+ if (isGslPointerType(Conv->getConversionType()) && IsGslOwnerImplicitObject)
return true;
if (!isGslPointerType(Callee->getFunctionObjectParameterType()) &&
- !isGslOwnerType(Callee->getFunctionObjectParameterType()))
+ !IsGslOwnerImplicitObject)
return false;
// Begin and end iterators.
@@ -181,7 +187,7 @@ bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
if (!Callee->getIdentifier())
// e.g., std::optional<T>::operator->() returns T*.
return RunningUnderLifetimeSafety
- ? Callee->getParent()->hasAttr<OwnerAttr>() &&
+ ? IsGslOwnerImplicitObject &&
Callee->getOverloadedOperator() ==
OverloadedOperatorKind::OO_Arrow
: false;
@@ -192,7 +198,7 @@ bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee,
if (Callee->getReturnType()->isReferenceType()) {
if (!Callee->getIdentifier()) {
auto OO = Callee->getOverloadedOperator();
- if (!Callee->getParent()->hasAttr<OwnerAttr>())
+ if (!IsGslOwnerImplicitObject)
return false;
return OO == OverloadedOperatorKind::OO_Subscript ||
OO == OverloadedOperatorKind::OO_Star;
@@ -272,8 +278,7 @@ bool shouldTrackSecondArgument(const FunctionDecl *FD) {
!isa<CXXMethodDecl>(FD);
}
-template <typename T> static bool isRecordWithAttr(QualType Type) {
- auto *RD = Type->getAsCXXRecordDecl();
+template <typename T> static bool isRecordWithAttr(const CXXRecordDecl *RD) {
if (!RD)
return false;
// Generally, if a primary template class declaration is annotated with an
@@ -299,8 +304,15 @@ template <typename T> static bool isRecordWithAttr(QualType Type) {
return Result;
}
+template <typename T> static bool isRecordWithAttr(QualType Type) {
+ return isRecordWithAttr<T>(Type->getAsCXXRecordDecl());
+}
+
bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); }
bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); }
+bool isGslOwnerType(const CXXRecordDecl *RD) {
+ return isRecordWithAttr<OwnerAttr>(RD);
+}
static StringRef getName(const CXXRecordDecl &RD) {
if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(&RD))
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 4e050e9bf6045..d15b2c6518cec 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -483,7 +483,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
else if (EnableGSLAnalysis) {
if (auto *CME = dyn_cast<CXXMethodDecl>(Callee);
CME && lifetimes::shouldTrackImplicitObjectArg(
- CME, /*RunningUnderLifetimeSafety=*/false))
+ *ObjectArg, CME, /*RunningUnderLifetimeSafety=*/false))
VisitGSLPointerArg(Callee, ObjectArg);
}
}
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index 70ec968dfd5c1..0619fda738fa4 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -3332,3 +3332,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
More information about the cfe-commits
mailing list