Skip to content

Commit 9d9c4fb

Browse files
committed
Add comments and add release note.
1 parent 0d9a597 commit 9d9c4fb

File tree

4 files changed

+40
-22
lines changed

4 files changed

+40
-22
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,8 @@ Improvements to Clang's diagnostics
278278
- The lifetimebound and GSL analysis in clang are coherent, allowing clang to
279279
detect more use-after-free bugs. (#GH100549).
280280

281+
- Clang now diagnoses cases where a dangling `GSLOwner<GSLPointer>`` object is constructed, e.g. `std::vector<string_view> v = {std::string}` (#GH100526).
282+
281283
Improvements to Clang's time-trace
282284
----------------------------------
283285

clang/include/clang/Basic/AttrDocs.td

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6690,6 +6690,20 @@ When the Owner's lifetime ends, it will consider the Pointer to be dangling.
66906690
P.getInt(); // P is dangling
66916691
}
66926692

6693+
If a template class is annotated with [[gsl::Owner]], and the first instantiated
6694+
template argument is a [[gsl::Pointer]] type, the analysis will consider the
6695+
instantiated class as a container of the pointer. When constructing such an
6696+
object from a GSL owner object, the analysis will assume that the container
6697+
holds a pointer to the owner object. Consequently, when the owner object is
6698+
destroyed, the pointer will be considered dangling.
6699+
6700+
.. code-block:: c++
6701+
6702+
int f() {
6703+
std::vector<std::string_view> v = {std::string();}; // v holds a dangling pointer.
6704+
std::optional<std::string_view> o = std::string(); // o holds a dangling pointer.
6705+
}
6706+
66936707
}];
66946708
}
66956709

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -361,12 +361,21 @@ static bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
361361
if (ATL.getAttrAs<LifetimeBoundAttr>())
362362
return true;
363363
}
364+
364365
return isNormalAsisgnmentOperator(FD);
365366
}
366367

367-
bool isFirstTemplateArgumentGSLPointer(const TemplateArgumentList &TAs) {
368-
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
369-
isRecordWithAttr<PointerAttr>(TAs[0].getAsType());
368+
// Returns true if the given Record decl is a form of `GSLOwner<GSLPointer>`
369+
// type, e.g. std::vector<string_view>, std::optional<string_view>.
370+
static bool isContainerOfGSLPointer(const CXXRecordDecl *Container) {
371+
if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(Container)) {
372+
if (!CTSD->hasAttr<OwnerAttr>()) // Container must be a GSL owner type.
373+
return false;
374+
const auto &TAs = CTSD->getTemplateArgs();
375+
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
376+
isRecordWithAttr<PointerAttr>(TAs[0].getAsType());
377+
}
378+
return false;
370379
}
371380

372381
// Visit lifetimebound or gsl-pointer arguments.
@@ -471,25 +480,15 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
471480
!Callee->getReturnType()->isReferenceType());
472481
} else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call)) {
473482
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>())
483+
// Two cases:
484+
// a GSL pointer, e.g. std::string_view
485+
// a container of GSL pointer, e.g. std::vector<string_view>
486+
if (ClassD->hasAttr<PointerAttr>() || isContainerOfGSLPointer(ClassD))
487487
VisitGSLPointerArg(Ctor->getConstructor()->getParamDecl(0), Args[0],
488488
true);
489489
}
490490
}
491491
}
492-
}
493492
}
494493

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

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -547,19 +547,22 @@ void test() {
547547

548548
namespace GH100526 {
549549
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}}
550+
std::vector<std::string_view> v1({std::string()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
551+
std::vector<std::string_view> v2({std::string(), std::string_view()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
552+
std::vector<std::string_view> v3({std::string_view(), std::string()}); // expected-warning {{object backing the pointer will be destroyed at the end}}
553+
554+
std::optional<std::string_view> o1 = std::string(); // expected-warning {{object backing the pointer}}
552555

553556
std::string s;
554557
// This is a tricky use-after-free case, what it does:
555558
// 1. make_optional creates a temporary "optional<string>"" object
556559
// 2. the temporary object owns the underlying string which is copied from s.
557560
// 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}}
561+
std::optional<std::string_view> o2 = std::make_optional(s); // expected-warning {{object backing the pointer}}
559562

560563
// FIXME: should work for assignment cases
561-
t1 = {std::string()};
562-
t2 = std::string();
564+
v1 = {std::string()};
565+
o1 = std::string();
563566

564567
// no warning on copying pointers.
565568
std::vector<std::string_view> n1 = {std::string_view()};

0 commit comments

Comments
 (0)