Skip to content

Commit 94213a4

Browse files
authored
[LifetimeSafety] Add support for GSL Pointer types (#154009)
This extends the lifetime safety analysis to support C++ types annotated with `gsl::Pointer`, which represent non-owning "view" types like `std::string_view`. These types have the same lifetime safety concerns as raw pointers and references. - Added support for detecting and analyzing `gsl::Pointer` annotated types in lifetime safety analysis - Implemented handling for various expressions involving `gsl::Pointer` types: - Constructor expressions - Member call expressions (especially conversion operators) - Functional cast expressions - Initialization list expressions - Materialized temporary expressions - Updated the pointer type detection to recognize `gsl::Pointer` types - Added handling for function calls that create borrows through reference parameters Fixes: #152513
1 parent 91d4c0d commit 94213a4

File tree

3 files changed

+381
-15
lines changed

3 files changed

+381
-15
lines changed

clang/lib/Analysis/LifetimeSafety.cpp

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,25 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
478478
}
479479
}
480480

481+
void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
482+
if (isGslPointerType(CCE->getType())) {
483+
handleGSLPointerConstruction(CCE);
484+
return;
485+
}
486+
}
487+
488+
void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
489+
// Specifically for conversion operators,
490+
// like `std::string_view p = std::string{};`
491+
if (isGslPointerType(MCE->getType()) &&
492+
isa<CXXConversionDecl>(MCE->getCalleeDecl())) {
493+
// The argument is the implicit object itself.
494+
handleFunctionCall(MCE, MCE->getMethodDecl(),
495+
{MCE->getImplicitObjectArgument()});
496+
}
497+
// FIXME: A more general VisitCallExpr could also be used here.
498+
}
499+
481500
void VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *N) {
482501
/// TODO: Handle nullptr expr as a special 'null' loan. Uninitialized
483502
/// pointers can use the same type of loan.
@@ -530,8 +549,27 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
530549
void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *FCE) {
531550
// Check if this is a test point marker. If so, we are done with this
532551
// expression.
533-
if (VisitTestPoint(FCE))
552+
if (handleTestPoint(FCE))
534553
return;
554+
if (isGslPointerType(FCE->getType()))
555+
addAssignOriginFact(*FCE, *FCE->getSubExpr());
556+
}
557+
558+
void VisitInitListExpr(const InitListExpr *ILE) {
559+
if (!hasOrigin(ILE))
560+
return;
561+
// For list initialization with a single element, like `View{...}`, the
562+
// origin of the list itself is the origin of its single element.
563+
if (ILE->getNumInits() == 1)
564+
addAssignOriginFact(*ILE, *ILE->getInit(0));
565+
}
566+
567+
void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) {
568+
if (!hasOrigin(MTE))
569+
return;
570+
// A temporary object's origin is the same as the origin of the
571+
// expression that initializes it.
572+
addAssignOriginFact(*MTE, *MTE->getSubExpr());
535573
}
536574

537575
void handleDestructor(const CFGAutomaticObjDtor &DtorOpt) {
@@ -557,10 +595,21 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
557595
}
558596

559597
private:
560-
static bool isPointerType(QualType QT) {
561-
return QT->isPointerOrReferenceType();
598+
static bool isGslPointerType(QualType QT) {
599+
if (const auto *RD = QT->getAsCXXRecordDecl()) {
600+
// We need to check the template definition for specializations.
601+
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
602+
return CTSD->getSpecializedTemplate()
603+
->getTemplatedDecl()
604+
->hasAttr<PointerAttr>();
605+
return RD->hasAttr<PointerAttr>();
606+
}
607+
return false;
562608
}
563609

610+
static bool isPointerType(QualType QT) {
611+
return QT->isPointerOrReferenceType() || isGslPointerType(QT);
612+
}
564613
// Check if a type has an origin.
565614
static bool hasOrigin(const Expr *E) {
566615
return E->isGLValue() || isPointerType(E->getType());
@@ -570,6 +619,41 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
570619
return isPointerType(VD->getType());
571620
}
572621

622+
void handleGSLPointerConstruction(const CXXConstructExpr *CCE) {
623+
assert(isGslPointerType(CCE->getType()));
624+
if (CCE->getNumArgs() != 1)
625+
return;
626+
if (hasOrigin(CCE->getArg(0)))
627+
addAssignOriginFact(*CCE, *CCE->getArg(0));
628+
else
629+
// This could be a new borrow.
630+
handleFunctionCall(CCE, CCE->getConstructor(),
631+
{CCE->getArgs(), CCE->getNumArgs()});
632+
}
633+
634+
/// Checks if a call-like expression creates a borrow by passing a value to a
635+
/// reference parameter, creating an IssueFact if it does.
636+
void handleFunctionCall(const Expr *Call, const FunctionDecl *FD,
637+
ArrayRef<const Expr *> Args) {
638+
if (!FD)
639+
return;
640+
// TODO: Handle more than one arguments.
641+
for (unsigned I = 0; I <= 0 /*Args.size()*/; ++I) {
642+
const Expr *ArgExpr = Args[I];
643+
644+
// Propagate origins for CXX this.
645+
if (FD->isCXXClassMember() && I == 0) {
646+
addAssignOriginFact(*Call, *ArgExpr);
647+
continue;
648+
}
649+
// The parameter is a pointer, reference, or gsl::Pointer.
650+
// This is a borrow. We propagate the origin from the argument expression
651+
// at the call site to the parameter declaration in the callee.
652+
if (hasOrigin(ArgExpr))
653+
addAssignOriginFact(*Call, *ArgExpr);
654+
}
655+
}
656+
573657
/// Creates a loan for the storage path of a given declaration reference.
574658
/// This function should be called whenever a DeclRefExpr represents a borrow.
575659
/// \param DRE The declaration reference expression that initiates the borrow.
@@ -593,7 +677,7 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
593677

594678
/// Checks if the expression is a `void("__lifetime_test_point_...")` cast.
595679
/// If so, creates a `TestPointFact` and returns true.
596-
bool VisitTestPoint(const CXXFunctionalCastExpr *FCE) {
680+
bool handleTestPoint(const CXXFunctionalCastExpr *FCE) {
597681
if (!FCE->getType()->isVoidType())
598682
return false;
599683

@@ -641,6 +725,8 @@ class FactGenerator : public ConstStmtVisitor<FactGenerator> {
641725
}
642726

643727
void markUseAsWrite(const DeclRefExpr *DRE) {
728+
if (!isPointerType(DRE->getType()))
729+
return;
644730
assert(UseFacts.contains(DRE));
645731
UseFacts[DRE]->markAsWritten();
646732
}

clang/test/Sema/warn-lifetime-safety.cpp

Lines changed: 110 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ struct MyObj {
66
MyObj operator+(MyObj);
77
};
88

9+
struct [[gsl::Pointer()]] View {
10+
View(const MyObj&); // Borrows from MyObj
11+
View();
12+
void use() const;
13+
};
14+
915
//===----------------------------------------------------------------------===//
1016
// Basic Definite Use-After-Free (-W...permissive)
1117
// These are cases where the pointer is guaranteed to be dangling at the use site.
@@ -20,12 +26,31 @@ void definite_simple_case() {
2026
(void)*p; // expected-note {{later used here}}
2127
}
2228

29+
void definite_simple_case_gsl() {
30+
View v;
31+
{
32+
MyObj s;
33+
v = s; // expected-warning {{object whose reference is captured does not live long enough}}
34+
} // expected-note {{destroyed here}}
35+
v.use(); // expected-note {{later used here}}
36+
}
37+
2338
void no_use_no_error() {
2439
MyObj* p;
2540
{
2641
MyObj s;
2742
p = &s;
2843
}
44+
// 'p' is dangling here, but since it is never used, no warning is issued.
45+
}
46+
47+
void no_use_no_error_gsl() {
48+
View v;
49+
{
50+
MyObj s;
51+
v = s;
52+
}
53+
// 'v' is dangling here, but since it is never used, no warning is issued.
2954
}
3055

3156
void definite_pointer_chain() {
@@ -39,6 +64,16 @@ void definite_pointer_chain() {
3964
(void)*q; // expected-note {{later used here}}
4065
}
4166

67+
void definite_propagation_gsl() {
68+
View v1, v2;
69+
{
70+
MyObj s;
71+
v1 = s; // expected-warning {{object whose reference is captured does not live long enough}}
72+
v2 = v1;
73+
} // expected-note {{destroyed here}}
74+
v2.use(); // expected-note {{later used here}}
75+
}
76+
4277
void definite_multiple_uses_one_warning() {
4378
MyObj* p;
4479
{
@@ -78,6 +113,19 @@ void definite_single_pointer_multiple_loans(bool cond) {
78113
(void)*p; // expected-note 2 {{later used here}}
79114
}
80115

116+
void definite_single_pointer_multiple_loans_gsl(bool cond) {
117+
View v;
118+
if (cond){
119+
MyObj s;
120+
v = s; // expected-warning {{object whose reference is captured does not live long enough}}
121+
} // expected-note {{destroyed here}}
122+
else {
123+
MyObj t;
124+
v = t; // expected-warning {{object whose reference is captured does not live long enough}}
125+
} // expected-note {{destroyed here}}
126+
v.use(); // expected-note 2 {{later used here}}
127+
}
128+
81129

82130
//===----------------------------------------------------------------------===//
83131
// Potential (Maybe) Use-After-Free (-W...strict)
@@ -94,18 +142,14 @@ void potential_if_branch(bool cond) {
94142
(void)*p; // expected-note {{later used here}}
95143
}
96144

97-
// If all paths lead to a dangle, it becomes a definite error.
98-
void potential_becomes_definite(bool cond) {
99-
MyObj* p;
145+
void potential_if_branch_gsl(bool cond) {
146+
MyObj safe;
147+
View v = safe;
100148
if (cond) {
101-
MyObj temp1;
102-
p = &temp1; // expected-warning {{does not live long enough}}
103-
} // expected-note {{destroyed here}}
104-
else {
105-
MyObj temp2;
106-
p = &temp2; // expected-warning {{does not live long enough}}
149+
MyObj temp;
150+
v = temp; // expected-warning {{object whose reference is captured may not live long enough}}
107151
} // expected-note {{destroyed here}}
108-
(void)*p; // expected-note 2 {{later used here}}
152+
v.use(); // expected-note {{later used here}}
109153
}
110154

111155
void definite_potential_together(bool cond) {
@@ -159,6 +203,16 @@ void potential_for_loop_use_after_loop_body(MyObj safe) {
159203
(void)*p; // expected-note {{later used here}}
160204
}
161205

206+
void potential_for_loop_gsl() {
207+
MyObj safe;
208+
View v = safe;
209+
for (int i = 0; i < 1; ++i) {
210+
MyObj s;
211+
v = s; // expected-warning {{object whose reference is captured may not live long enough}}
212+
} // expected-note {{destroyed here}}
213+
v.use(); // expected-note {{later used here}}
214+
}
215+
162216
void potential_for_loop_use_before_loop_body(MyObj safe) {
163217
MyObj* p = &safe;
164218
for (int i = 0; i < 1; ++i) {
@@ -182,6 +236,19 @@ void potential_loop_with_break(bool cond) {
182236
(void)*p; // expected-note {{later used here}}
183237
}
184238

239+
void potential_loop_with_break_gsl(bool cond) {
240+
MyObj safe;
241+
View v = safe;
242+
for (int i = 0; i < 10; ++i) {
243+
if (cond) {
244+
MyObj temp;
245+
v = temp; // expected-warning {{object whose reference is captured may not live long enough}}
246+
break; // expected-note {{destroyed here}}
247+
}
248+
}
249+
v.use(); // expected-note {{later used here}}
250+
}
251+
185252
void potential_multiple_expiry_of_same_loan(bool cond) {
186253
// Choose the last expiry location for the loan.
187254
MyObj safe;
@@ -258,6 +325,28 @@ void definite_switch(int mode) {
258325
(void)*p; // expected-note 3 {{later used here}}
259326
}
260327

328+
void definite_switch_gsl(int mode) {
329+
View v;
330+
switch (mode) {
331+
case 1: {
332+
MyObj temp1;
333+
v = temp1; // expected-warning {{object whose reference is captured does not live long enough}}
334+
break; // expected-note {{destroyed here}}
335+
}
336+
case 2: {
337+
MyObj temp2;
338+
v = temp2; // expected-warning {{object whose reference is captured does not live long enough}}
339+
break; // expected-note {{destroyed here}}
340+
}
341+
default: {
342+
MyObj temp3;
343+
v = temp3; // expected-warning {{object whose reference is captured does not live long enough}}
344+
break; // expected-note {{destroyed here}}
345+
}
346+
}
347+
v.use(); // expected-note 3 {{later used here}}
348+
}
349+
261350
//===----------------------------------------------------------------------===//
262351
// No-Error Cases
263352
//===----------------------------------------------------------------------===//
@@ -271,3 +360,14 @@ void no_error_if_dangle_then_rescue() {
271360
p = &safe; // p is "rescued" before use.
272361
(void)*p; // This is safe.
273362
}
363+
364+
void no_error_if_dangle_then_rescue_gsl() {
365+
MyObj safe;
366+
View v;
367+
{
368+
MyObj temp;
369+
v = temp; // 'v' is temporarily dangling.
370+
}
371+
v = safe; // 'v' is "rescued" before use by reassigning to a valid object.
372+
v.use(); // This is safe.
373+
}

0 commit comments

Comments
 (0)