Skip to content

Commit 0d9a597

Browse files
committed
[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.
1 parent d21e731 commit 0d9a597

File tree

2 files changed

+67
-6
lines changed

2 files changed

+67
-6
lines changed

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,14 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
361361
if (ATL.getAttrAs<LifetimeBoundAttr>())
362362
return true;
363363
}
364-
365364
return isNormalAsisgnmentOperator(FD);
366365
}
367366

367+
bool isFirstTemplateArgumentGSLPointer(const TemplateArgumentList &TAs) {
368+
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
369+
isRecordWithAttr<PointerAttr>(TAs[0].getAsType());
370+
}
371+
368372
// Visit lifetimebound or gsl-pointer arguments.
369373
static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
370374
LocalVisitor Visit) {
@@ -465,14 +469,27 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
465469
if (shouldTrackFirstArgument(Callee)) {
466470
VisitGSLPointerArg(Callee, Args[0],
467471
!Callee->getReturnType()->isReferenceType());
468-
} else if (auto *CCE = dyn_cast<CXXConstructExpr>(Call);
469-
CCE &&
470-
CCE->getConstructor()->getParent()->hasAttr<PointerAttr>()) {
471-
VisitGSLPointerArg(CCE->getConstructor()->getParamDecl(0), Args[0],
472-
true);
472+
} else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call)) {
473+
const auto *ClassD = Ctor->getConstructor()->getParent();
474+
// Constructing the Container<GSLPointer> case (e.g.
475+
// std::optional<string_view>) case.
476+
if (const auto *CTSD =
477+
dyn_cast<ClassTemplateSpecializationDecl>(ClassD)) {
478+
if (isFirstTemplateArgumentGSLPointer(CTSD->getTemplateArgs()) &&
479+
CTSD->hasAttr<OwnerAttr>()) {
480+
VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0), Args[0],
481+
true);
482+
return;
483+
}
484+
}
485+
// Constructing the GSLPointer (e.g. std::string_view) case.
486+
if (ClassD->hasAttr<PointerAttr>())
487+
VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0), Args[0],
488+
true);
473489
}
474490
}
475491
}
492+
}
476493
}
477494

478495
/// Visit the locals that would be reachable through a reference bound to the

clang/test/Sema/warn-lifetime-analysis-nocfg.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,26 @@ auto begin(C &c) -> decltype(c.begin());
158158
template<typename T, int N>
159159
T *begin(T (&array)[N]);
160160

161+
using size_t = decltype(sizeof(0));
162+
163+
template<typename T>
164+
struct initializer_list {
165+
const T* ptr; size_t sz;
166+
};
161167
template <typename T>
162168
struct vector {
163169
typedef __gnu_cxx::basic_iterator<T> iterator;
164170
iterator begin();
165171
iterator end();
166172
const T *data() const;
173+
vector();
174+
vector(initializer_list<T> __l);
167175
T &at(int n);
168176
};
169177

170178
template<typename T>
171179
struct basic_string_view {
180+
basic_string_view();
172181
basic_string_view(const T *);
173182
const T *begin() const;
174183
};
@@ -203,11 +212,21 @@ template<typename T>
203212
struct optional {
204213
optional();
205214
optional(const T&);
215+
216+
template<typename U = T>
217+
optional(U&& t);
218+
219+
template<typename U>
220+
optional(optional<U>&& __t);
221+
206222
T &operator*() &;
207223
T &&operator*() &&;
208224
T &value() &;
209225
T &&value() &&;
210226
};
227+
template<typename T>
228+
optional<T> make_optional(T&&);
229+
211230

212231
template<typename T>
213232
struct stack {
@@ -525,3 +544,28 @@ void test() {
525544
std::string_view svjkk1 = ReturnStringView(StrCat("bar", "x")); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}}
526545
}
527546
} // namespace GH100549
547+
548+
namespace GH100526 {
549+
void test() {
550+
std::vector<std::string_view> t1 = {std::string()}; // expected-warning {{object backing the pointer will be destroyed at the end}}
551+
std::optional<std::string_view> t2 = std::string(); // expected-warning {{object backing the pointer}}
552+
553+
std::string s;
554+
// This is a tricky use-after-free case, what it does:
555+
// 1. make_optional creates a temporary "optional<string>"" object
556+
// 2. the temporary object owns the underlying string which is copied from s.
557+
// 3. the t3 object holds the view to the underlying string of the temporary object.
558+
std::optional<std::string_view> t3 = std::make_optional(s); // expected-warning {{object backing the pointer}}
559+
560+
// FIXME: should work for assignment cases
561+
t1 = {std::string()};
562+
t2 = std::string();
563+
564+
// no warning on copying pointers.
565+
std::vector<std::string_view> n1 = {std::string_view()};
566+
std::optional<std::string_view> n2 = {std::string_view()};
567+
std::optional<std::string_view> n3 = std::make_optional(std::string_view());
568+
569+
}
570+
571+
} // namespace GH100526

0 commit comments

Comments
 (0)