Skip to content

Commit ea6fffa

Browse files
committed
fix another false positive caused by the interaction between the gsl::Owner and lifetimebound attributes.
1 parent 102fd6c commit ea6fffa

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
}
@@ -298,8 +313,7 @@ static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
298313
Callee->getFunctionObjectParameterType()) &&
299314
!isGSLOwner(Callee->getFunctionObjectParameterType()))
300315
return false;
301-
if (Callee->getReturnType()->isPointerType() ||
302-
isRecordWithAttr<PointerAttr>(Callee->getReturnType())) {
316+
if (isPointerLikeType(Callee->getReturnType())) {
303317
if (!Callee->getIdentifier())
304318
return false;
305319
return llvm::StringSwitch<bool>(Callee->getName())
@@ -347,6 +361,30 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
347361
return false;
348362
}
349363

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

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

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

636+
template <typename T>
637+
struct [[gsl::Owner]] StatusOr {
638+
const T &value() [[clang::lifetimebound]];
639+
};
640+
std::vector<int*> test5(StatusOr<std::vector<int*>> aa) {
641+
return aa.value(); // fine
642+
}
643+
636644
} // namespace GH100526

0 commit comments

Comments
 (0)