https://github.com/hokein created
https://github.com/llvm/llvm-project/pull/107213
This pull request enhances the GSL lifetime analysis to detect situations where
a dangling `Container<GSLPointer>` object is constructed:
```cpp
std::vector<std::string_view> bad = {std::string()}; // dangling
```
The assignment case is not yet supported, but they will be addressed in a
follow-up.
Fixes #100526 (excluding the `push_back` case).
>From 650054c3fa9a640b9cd255f98c46dbfbd0b8c590 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein...@gmail.com>
Date: Wed, 28 Aug 2024 09:59:41 +0200
Subject: [PATCH] [clang] Diagnose dangling issues for "Container<GSLPointer>"
case.
We teach the GSL lifetime analysis to handle this code path, particuly
when constructing the container<GSLPointer> object from a GSLOwner.
---
clang/lib/Sema/CheckExprLifetime.cpp | 28 +++++++++---
.../Sema/warn-lifetime-analysis-nocfg.cpp | 44 +++++++++++++++++++
2 files changed, 67 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp
b/clang/lib/Sema/CheckExprLifetime.cpp
index f7540a6e3a8979..a58ea39c6bd5e5 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -363,10 +363,14 @@ static bool implicitObjectParamIsLifetimeBound(const
FunctionDecl *FD) {
if (ATL.getAttrAs<LifetimeBoundAttr>())
return true;
}
-
return isNormalAsisgnmentOperator(FD);
}
+bool isFirstTemplateArgumentGSLPointer(const TemplateArgumentList &TAs) {
+ return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
+ isRecordWithAttr<PointerAttr>(TAs[0].getAsType());
+}
+
// Visit lifetimebound or gsl-pointer arguments.
static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
LocalVisitor Visit,
@@ -470,10 +474,24 @@ static void visitFunctionCallArguments(IndirectLocalPath
&Path, Expr *Call,
VisitGSLPointerArg(Callee, Args[0],
!Callee->getReturnType()->isReferenceType());
} else {
- if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
- CCE && CCE->getConstructor()->getParent()->hasAttr<PointerAttr>())
- VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0],
- true);
+ if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call)) {
+ const auto *ClassD = Ctor->getConstructor()->getParent();
+ // Constructing the Container<GSLPointer> case (e.g.
+ // std::optional<string_view>) case.
+ if (const auto *CTSD =
+ dyn_cast<ClassTemplateSpecializationDecl>(ClassD)) {
+ if (isFirstTemplateArgumentGSLPointer(CTSD->getTemplateArgs()) &&
+ CTSD->hasAttr<OwnerAttr>()) {
+ VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0),
+ Args[0], true);
+ return;
+ }
+ }
+ // Constructing the GSLPointer (e.g. std::string_view) case.
+ if (ClassD->hasAttr<PointerAttr>())
+ VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0),
Args[0],
+ true);
+ }
}
}
}
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index cd1904db327105..dd0ed138c99676 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -158,17 +158,26 @@ auto begin(C &c) -> decltype(c.begin());
template<typename T, int N>
T *begin(T (&array)[N]);
+using size_t = decltype(sizeof(0));
+
+template<typename T>
+struct initializer_list {
+ const T* ptr; size_t sz;
+};
template <typename T>
struct vector {
typedef __gnu_cxx::basic_iterator<T> iterator;
iterator begin();
iterator end();
const T *data() const;
+ vector();
+ vector(initializer_list<T> __l);
T &at(int n);
};
template<typename T>
struct basic_string_view {
+ basic_string_view();
basic_string_view(const T *);
const T *begin() const;
};
@@ -203,11 +212,21 @@ template<typename T>
struct optional {
optional();
optional(const T&);
+
+ template<typename U = T>
+ optional(U&& t);
+
+ template<typename U>
+ optional(optional<U>&& __t);
+
T &operator*() &;
T &&operator*() &&;
T &value() &;
T &&value() &&;
};
+template<typename T>
+optional<T> make_optional(T&&);
+
template<typename T>
struct stack {
@@ -499,3 +518,28 @@ std::string_view test2(int i,
std::optional<std::string_view> a) {
return std::move(a.value());
}
}
+
+namespace GH100526 {
+void test() {
+ std::vector<std::string_view> t1 = {std::string()}; // expected-warning
{{object backing the pointer will be destroyed at the end}}
+ std::optional<std::string_view> t2 = std::string(); // expected-warning
{{object backing the pointer}}
+
+ std::string s;
+ // This is a tricky use-after-free case, what it does:
+ // 1. make_optional creates a temporary "optional<string>"" object
+ // 2. the temporary object owns the underlying string which is copied from
s.
+ // 3. the t3 object holds the view to the underlying string of the
temporary object.
+ std::optional<std::string_view> t3 = std::make_optional(s); //
expected-warning {{object backing the pointer}}
+
+ // FIXME: should work for assignment cases
+ t1 = {std::string()};
+ t2 = std::string();
+
+ // no warning on copying pointers.
+ std::vector<std::string_view> n1 = {std::string_view()};
+ std::optional<std::string_view> n2 = {std::string_view()};
+ std::optional<std::string_view> n3 = std::make_optional(std::string_view());
+
+}
+
+} // namespace GH100526
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
