Skip to content

Commit 50daa23

Browse files
committed
fix another false positive caused by the interaction between the gsl::Owner and lifetimebound attributes.
1 parent 15728aa commit 50daa23

File tree

2 files changed

+54
-13
lines changed

2 files changed

+54
-13
lines changed

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,11 @@ static bool isInStlNamespace(const Decl *D) {
267267
return DC->isStdNamespace();
268268
}
269269

270+
static bool isPointerLikeType(QualType Type) {
271+
return isRecordWithAttr<PointerAttr>(Type) || Type->isPointerType() ||
272+
Type->isNullPtrType();
273+
}
274+
270275
// Returns true if the given Record decl is a form of `GSLOwner<Pointer>`
271276
// type, e.g. std::vector<string_view>, std::optional<string_view>.
272277
static bool isContainerOfPointer(const RecordDecl *Container) {
@@ -276,9 +281,19 @@ static bool isContainerOfPointer(const RecordDecl *Container) {
276281
return false;
277282
const auto &TAs = CTSD->getTemplateArgs();
278283
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
279-
(isRecordWithAttr<PointerAttr>(TAs[0].getAsType()) ||
280-
TAs[0].getAsType()->isPointerType() ||
281-
TAs[0].getAsType()->isNullPtrType());
284+
isPointerLikeType(TAs[0].getAsType());
285+
}
286+
return false;
287+
}
288+
// Returns true if the given Record is `std::initializer_list<pointer>`.
289+
static bool isStdInitializerListOfPointer(const RecordDecl *RD) {
290+
if (const auto *CTSD =
291+
dyn_cast_if_present<ClassTemplateSpecializationDecl>(RD)) {
292+
const auto &TAs = CTSD->getTemplateArgs();
293+
return isInStlNamespace(RD) && RD->getIdentifier() &&
294+
RD->getName() == "initializer_list" && TAs.size() > 0 &&
295+
TAs[0].getKind() == TemplateArgument::Type &&
296+
isPointerLikeType(TAs[0].getAsType());
282297
}
283298
return false;
284299
}
@@ -299,8 +314,7 @@ static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
299314
Callee->getFunctionObjectParameterType()) &&
300315
!isGSLOwner(Callee->getFunctionObjectParameterType()))
301316
return false;
302-
if (Callee->getReturnType()->isPointerType() ||
303-
isRecordWithAttr<PointerAttr>(Callee->getReturnType())) {
317+
if (isPointerLikeType(Callee->getReturnType())) {
304318
if (!Callee->getIdentifier())
305319
return false;
306320
return llvm::StringSwitch<bool>(Callee->getName())
@@ -348,6 +362,30 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
348362
return false;
349363
}
350364

365+
// Returns true if we should perform the GSL analysis on the first argument for
366+
// the given constructor.
367+
static bool
368+
shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
369+
const auto *ClassD = Ctor->getConstructor()->getParent();
370+
371+
auto FirstArgType = Ctor->getArg(0)->getType();
372+
// Case 1, construct a GSL pointer, e.g. std::string_view
373+
if (ClassD->hasAttr<PointerAttr>())
374+
return true;
375+
376+
// case 2: construct a container of pointer (std::vector<std::string_view>)
377+
// from an owner or a std::initilizer_list.
378+
//
379+
// std::initializer_list is a proxy object that provides access to the backing
380+
// array. We perform analysis on it to determine if there are any dangling
381+
// temporaries in the backing array.
382+
if (Ctor->getConstructor()->getNumParams() != 1 ||
383+
!isContainerOfPointer(ClassD))
384+
return false;
385+
return isGSLOwner(FirstArgType) ||
386+
isStdInitializerListOfPointer(FirstArgType->getAsRecordDecl());
387+
}
388+
351389
// Return true if this is an "normal" assignment operator.
352390
// We assuments that a normal assingment operator always returns *this, that is,
353391
// an lvalue reference that is the same type as the implicit object parameter
@@ -493,14 +531,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
493531
// Perform GSL analysis for the first argument
494532
if (shouldTrackFirstArgument(Callee)) {
495533
VisitGSLPointerArg(Callee, Args[0]);
496-
} else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call)) {
497-
const auto *ClassD = Ctor->getConstructor()->getParent();
498-
// Two cases:
499-
// a GSL pointer, e.g. std::string_view
500-
// a container of GSL pointer, e.g. std::vector<string_view>
501-
if (ClassD->hasAttr<PointerAttr>() ||
502-
(isContainerOfPointer(ClassD) && Callee->getNumParams() == 1))
503-
VisitGSLPointerArg(Ctor->getConstructor(), Args[0]);
534+
} else if (auto *Ctor = dyn_cast<CXXConstructExpr>(Call);
535+
Ctor && shouldTrackFirstArgumentForConstructor(Ctor)) {
536+
VisitGSLPointerArg(Ctor->getConstructor(), Args[0]);
504537
}
505538
}
506539
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,4 +667,12 @@ std::optional<int*> test4(int a) {
667667
return std::make_optional(nullptr); // fine
668668
}
669669

670+
template <typename T>
671+
struct [[gsl::Owner]] StatusOr {
672+
const T &value() [[clang::lifetimebound]];
673+
};
674+
std::vector<int*> test5(StatusOr<std::vector<int*>> aa) {
675+
return aa.value(); // fine
676+
}
677+
670678
} // namespace GH100526

0 commit comments

Comments
 (0)