[clang] [clang] Refine the temporay object member access filtering for GSL pointer (PR #122088)
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 13 02:53:51 PST 2025
https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/122088
>From aee3d1a6f5782bb68547679505571cd4b56f3573 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Tue, 17 Dec 2024 14:28:00 +0100
Subject: [PATCH 1/3] [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 7109de03cadd12f..d0d05e4ed980d54 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 f888e6ab94bb6aa..d318033ff0cc457 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 4c19367bb7f3ddc..3b271af9dfa13f1 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 b2cd6ea9ed3c65150aa8424f5765780ac84c7a26 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Thu, 9 Jan 2025 09:03:22 +0100
Subject: [PATCH 2/3] 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 3b271af9dfa13f1..48f52b7fc2788cf 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 {};
>From 6e63ad3ac7fd00bec25204b13cec6af307205bc3 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Mon, 13 Jan 2025 11:52:49 +0100
Subject: [PATCH 3/3] address review comment.
---
clang/lib/Sema/CheckExprLifetime.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index d0d05e4ed980d54..08ef6adb1861c82 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -1092,12 +1092,12 @@ enum PathLifetimeKind {
static PathLifetimeKind
shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
for (auto Elem : Path) {
- if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
- return PathLifetimeKind::Extend;
- if (Elem.Kind == IndirectLocalPathEntry::MemberExpr)
+ if (Elem.Kind == IndirectLocalPathEntry::MemberExpr ||
+ Elem.Kind == IndirectLocalPathEntry::LambdaCaptureInit)
continue;
- if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
- return PathLifetimeKind::NoExtend;
+ return Elem.Kind == IndirectLocalPathEntry::DefaultInit
+ ? PathLifetimeKind::Extend
+ : PathLifetimeKind::NoExtend;
}
return PathLifetimeKind::Extend;
}
More information about the cfe-commits
mailing list