https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/115921
>From 2cef37ecdb81452a8f5882dfe765167c1e45b7b6 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 13 Nov 2024 10:24:33 +0000 Subject: [PATCH 1/7] Implement semantics for lifetime analysis --- clang/include/clang/Basic/DiagnosticGroups.td | 2 + .../clang/Basic/DiagnosticSemaKinds.td | 7 +- clang/include/clang/Sema/Sema.h | 3 + clang/lib/Sema/CheckExprLifetime.cpp | 66 ++++++++--- clang/lib/Sema/CheckExprLifetime.h | 13 +++ clang/lib/Sema/SemaChecking.cpp | 47 +++++++- .../Sema/warn-lifetime-analysis-nocfg.cpp | 105 ++++++++++++++++++ clang/test/SemaCXX/attr-lifetimebound.cpp | 4 +- 8 files changed, 228 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 72eada50a56cc9..df9bf94b5d0398 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -453,6 +453,7 @@ def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">; def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">; def DanglingAssignment: DiagGroup<"dangling-assignment">; def DanglingAssignmentGsl : DiagGroup<"dangling-assignment-gsl">; +def DanglingCapture : DiagGroup<"dangling-capture">; def DanglingElse: DiagGroup<"dangling-else">; def DanglingField : DiagGroup<"dangling-field">; def DanglingInitializerList : DiagGroup<"dangling-initializer-list">; @@ -462,6 +463,7 @@ def ReturnStackAddress : DiagGroup<"return-stack-address">; def : DiagGroup<"return-local-addr", [ReturnStackAddress]>; def Dangling : DiagGroup<"dangling", [DanglingAssignment, DanglingAssignmentGsl, + DanglingCapture, DanglingField, DanglingInitializerList, DanglingGsl, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 2f5d672e2f0035..58745d450ed63f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10132,10 +10132,10 @@ def err_lifetimebound_ctor_dtor : Error< "%select{constructor|destructor}0">; def err_lifetimebound_parameter_void_return_type : Error< "'lifetimebound' attribute cannot be applied to a parameter of a function " - "that returns void">; + "that returns void; did you mean 'lifetime_capture_by(X)'">; def err_lifetimebound_implicit_object_parameter_void_return_type : Error< "'lifetimebound' attribute cannot be applied to an implicit object " - "parameter of a function that returns void">; + "parameter of a function that returns void; did you mean 'lifetime_capture_by(X)'">; // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< @@ -10230,6 +10230,9 @@ def warn_dangling_pointer_assignment : Warning< "object backing %select{|the pointer }0%1 " "will be destroyed at the end of the full-expression">, InGroup<DanglingAssignment>; +def warn_dangling_reference_captured : Warning< + "object captured by '%0' will be destroyed at the end of the full-expression">, + InGroup<DanglingCapture>; // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index d6f3508a5243f3..6ea6c67447b6f0 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2323,6 +2323,9 @@ class Sema final : public SemaBase { bool BuiltinVectorMath(CallExpr *TheCall, QualType &Res, bool FPOnly = false); bool BuiltinVectorToScalarMath(CallExpr *TheCall); + void checkLifetimeCaptureBy(FunctionDecl *FDecl, bool IsMemberFunction, + const Expr *ThisArg, ArrayRef<const Expr *> Args); + /// Handles the checks for format strings, non-POD arguments to vararg /// functions, NULL arguments passed to non-NULL parameters, diagnose_if /// attributes and AArch64 SME attributes. diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index a1a402b4a2b530..81e26f48fb8851 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -45,10 +45,14 @@ enum LifetimeKind { /// a default member initializer), the program is ill-formed. LK_MemInitializer, - /// The lifetime of a temporary bound to this entity probably ends too soon, + /// The lifetime of a temporary bound to this entity may end too soon, /// because the entity is a pointer and we assign the address of a temporary /// object to it. LK_Assignment, + + /// The lifetime of a temporary bound to this entity may end too soon, + /// because the entity may capture the reference to a temporary object. + LK_LifetimeCapture, }; using LifetimeResult = llvm::PointerIntPair<const InitializedEntity *, 3, LifetimeKind>; @@ -193,6 +197,7 @@ struct IndirectLocalPathEntry { VarInit, LValToRVal, LifetimeBoundCall, + LifetimeCapture, TemporaryCopy, LambdaCaptureInit, GslReferenceInit, @@ -249,9 +254,10 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, LocalVisitor Visit); template <typename T> static bool isRecordWithAttr(QualType Type) { - if (auto *RD = Type->getAsCXXRecordDecl()) - return RD->hasAttr<T>(); - return false; + CXXRecordDecl *RD = Type.getNonReferenceType()->getAsCXXRecordDecl(); + if (!RD) + return false; + return RD->hasAttr<T>(); } // Decl::isInStdNamespace will return false for iterators in some STL @@ -1049,6 +1055,7 @@ static SourceRange nextPathEntryRange(const IndirectLocalPath &Path, unsigned I, case IndirectLocalPathEntry::AddressOf: case IndirectLocalPathEntry::LValToRVal: case IndirectLocalPathEntry::LifetimeBoundCall: + case IndirectLocalPathEntry::LifetimeCapture: case IndirectLocalPathEntry::TemporaryCopy: case IndirectLocalPathEntry::GslReferenceInit: case IndirectLocalPathEntry::GslPointerInit: @@ -1082,6 +1089,7 @@ static bool pathOnlyHandlesGslPointer(const IndirectLocalPath &Path) { case IndirectLocalPathEntry::VarInit: case IndirectLocalPathEntry::AddressOf: case IndirectLocalPathEntry::LifetimeBoundCall: + case IndirectLocalPathEntry::LifetimeCapture: continue; case IndirectLocalPathEntry::GslPointerInit: case IndirectLocalPathEntry::GslReferenceInit: @@ -1110,12 +1118,13 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef, isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator))); } -static void checkExprLifetimeImpl(Sema &SemaRef, - const InitializedEntity *InitEntity, - const InitializedEntity *ExtendingEntity, - LifetimeKind LK, - const AssignedEntity *AEntity, Expr *Init) { +static void +checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, + const InitializedEntity *ExtendingEntity, LifetimeKind LK, + const AssignedEntity *AEntity, + const CapturingEntity *CapEntity, Expr *Init) { assert((AEntity && LK == LK_Assignment) || + (CapEntity && LK == LK_LifetimeCapture) || (InitEntity && LK != LK_Assignment)); // If this entity doesn't have an interesting lifetime, don't bother looking // for temporaries within its initializer. @@ -1199,6 +1208,17 @@ static void checkExprLifetimeImpl(Sema &SemaRef, break; } + case LK_LifetimeCapture: { + if (!MTE) + return false; + assert(shouldLifetimeExtendThroughPath(Path) == + PathLifetimeKind::NoExtend && + "No lifetime extension in function calls"); + SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) + << CapEntity->Entity << DiagRange; + return false; + } + case LK_Assignment: { if (!MTE || pathContainsInit(Path)) return false; @@ -1359,6 +1379,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, break; case IndirectLocalPathEntry::LifetimeBoundCall: + case IndirectLocalPathEntry::LifetimeCapture: case IndirectLocalPathEntry::TemporaryCopy: case IndirectLocalPathEntry::GslPointerInit: case IndirectLocalPathEntry::GslReferenceInit: @@ -1411,7 +1432,7 @@ static void checkExprLifetimeImpl(Sema &SemaRef, // warnings or errors on inner temporaries within this one's initializer. return false; }; - + bool HasReferenceBinding = Init->isGLValue(); llvm::SmallVector<IndirectLocalPathEntry, 8> Path; if (LK == LK_Assignment && shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) { @@ -1420,9 +1441,18 @@ static void checkExprLifetimeImpl(Sema &SemaRef, ? IndirectLocalPathEntry::LifetimeBoundCall : IndirectLocalPathEntry::GslPointerAssignment, Init}); + } else if (LK == LK_LifetimeCapture) { + Path.push_back({IndirectLocalPathEntry::LifetimeCapture, Init}); + if (isRecordWithAttr<PointerAttr>(Init->getType())) + HasReferenceBinding = false; + // Skip the top MTE if it is a temporary object of the pointer-like type + // itself. + if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init); + MTE && isPointerLikeType(Init->getType())) + Init = MTE->getSubExpr(); } - if (Init->isGLValue()) + if (HasReferenceBinding) visitLocalsRetainedByReferenceBinding(Path, Init, RK_ReferenceBinding, TemporaryVisitor); else @@ -1438,13 +1468,13 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, LifetimeKind LK = LTResult.getInt(); const InitializedEntity *ExtendingEntity = LTResult.getPointer(); checkExprLifetimeImpl(SemaRef, &Entity, ExtendingEntity, LK, - /*AEntity*/ nullptr, Init); + /*AEntity*/ nullptr, /*CapEntity=*/nullptr, Init); } void checkExprLifetimeMustTailArg(Sema &SemaRef, const InitializedEntity &Entity, Expr *Init) { checkExprLifetimeImpl(SemaRef, &Entity, nullptr, LK_MustTail, - /*AEntity*/ nullptr, Init); + /*AEntity*/ nullptr, /*CapEntity=*/nullptr, Init); } void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, @@ -1460,7 +1490,15 @@ void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, checkExprLifetimeImpl(SemaRef, /*InitEntity=*/nullptr, /*ExtendingEntity=*/nullptr, LK_Assignment, &Entity, - Init); + /*CapEntity=*/nullptr, Init); +} + +void checkExprLifetime(Sema &SemaRef, const CapturingEntity &Entity, + Expr *Init) { + return checkExprLifetimeImpl(SemaRef, /*InitEntity=*/nullptr, + /*ExtendingEntity=*/nullptr, LK_LifetimeCapture, + /*AEntity=*/nullptr, + /*CapEntity=*/&Entity, Init); } } // namespace clang::sema diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index 903f312f3533e5..e109b73b280e9e 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -25,6 +25,16 @@ struct AssignedEntity { CXXMethodDecl *AssignmentOperator = nullptr; }; +struct CapturingEntity { + // In an function call involving a lifetime capture, this would be the + // argument capturing the lifetime of another argument. + // void addToSet(std::string_view sv [[clang::lifetime_capture_by(setsv)]], + // set<std::string_view>& setsv); + // set<std::string_view> setsv; + // addToSet(std::string(), setsv); // Here 'setsv' is the 'Entity'. + Expr *Entity = nullptr; +}; + /// Check that the lifetime of the given expr (and its subobjects) is /// sufficient for initializing the entity, and perform lifetime extension /// (when permitted) if not. @@ -35,6 +45,9 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, /// sufficient for assigning to the entity. void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init); +void checkExprLifetime(Sema &SemaRef, const CapturingEntity &Entity, + Expr *Init); + /// Check that the lifetime of the given expr (and its subobjects) is /// sufficient, assuming that it is passed as an argument to a musttail /// function. diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 96008b14225a4c..27edb56df3abc7 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -11,6 +11,7 @@ // //===----------------------------------------------------------------------===// +#include "CheckExprLifetime.h" #include "clang/AST/APValue.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" @@ -3229,6 +3230,49 @@ void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl, << ParamName << (FDecl != nullptr) << FDecl; } +void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction, + const Expr *ThisArg, + ArrayRef<const Expr *> Args) { + auto GetArgAt = [&](int Idx) { + if (IsMemberFunction && Idx == 0) + return const_cast<Expr *>(ThisArg); + return const_cast<Expr *>(Args[Idx - int(IsMemberFunction)]); + }; + for (unsigned I = 0; I < FD->getNumParams(); ++I) { + auto *CapturedByAttr = + FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(); + if (!CapturedByAttr) + continue; + for (int CapturingParamIdx : CapturedByAttr->params()) { + Expr *Capturing = GetArgAt(CapturingParamIdx); + Expr *Captured = GetArgAt(I + IsMemberFunction); + CapturingEntity CE{Capturing}; + // Ensure that 'Captured' outlives the 'Capturing' entity. + checkExprLifetime(*this, CE, Captured); + } + } + // Check when the 'this' object is captured. + if (IsMemberFunction) { + TypeSourceInfo *TSI = FD->getTypeSourceInfo(); + if (!TSI) + return; + AttributedTypeLoc ATL; + for (TypeLoc TL = TSI->getTypeLoc(); + (ATL = TL.getAsAdjusted<AttributedTypeLoc>()); + TL = ATL.getModifiedLoc()) { + auto *CapturedByAttr = ATL.getAttrAs<LifetimeCaptureByAttr>(); + if (!CapturedByAttr) + continue; + Expr *Captured = GetArgAt(0); + for (int CapturingParamIdx : CapturedByAttr->params()) { + Expr *Capturing = GetArgAt(CapturingParamIdx); + CapturingEntity CE{Capturing}; + checkExprLifetime(*this, CE, Captured); + } + } + } +} + void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, const Expr *ThisArg, ArrayRef<const Expr *> Args, bool IsMemberFunction, SourceLocation Loc, @@ -3269,7 +3313,8 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, } } } - + if (FD) + checkLifetimeCaptureBy(FD, IsMemberFunction, ThisArg, Args); if (FDecl || Proto) { CheckNonNullArguments(*this, FDecl, Proto, Args, Loc); diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 6a2af01ea5116c..49bbc05c009ff9 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -793,3 +793,108 @@ void test13() { } } // namespace GH100526 + +namespace lifetime_capture_by { +struct S { + const int *x; + void captureInt(const int&x [[clang::lifetime_capture_by(this)]]) { this->x = &x; } + void captureSV(std::string_view sv [[clang::lifetime_capture_by(this)]]); +}; +/////////////////////////// +// Detect dangling cases. +/////////////////////////// +void captureInt(const int&x [[clang::lifetime_capture_by(s)]], S&s); +void captureRValInt(int&&x [[clang::lifetime_capture_by(s)]], S&s); +void noCaptureInt(int x [[clang::lifetime_capture_by(s)]], S&s); +std::string_view substr(const std::string& s [[clang::lifetimebound]]); +std::string_view strcopy(const std::string& s); +void captureSV(std::string_view x [[clang::lifetime_capture_by(s)]], S&s); +void captureRValSV(std::string_view&& x [[clang::lifetime_capture_by(s)]], S&s); +void noCaptureSV(std::string_view x, S&s); +void captureS(const std::string& x [[clang::lifetime_capture_by(s)]], S&s); +void captureRValS(std::string&& x [[clang::lifetime_capture_by(s)]], S&s); +const std::string* getPointerLB(const std::string& s[[clang::lifetimebound]]); +const std::string* getPointerNoLB(const std::string& s); +void capturePointer(const std::string* x [[clang::lifetime_capture_by(s)]], S&s); +struct ThisIsCaptured { + void capture(S& s) [[clang::lifetime_capture_by(s)]]; + void bar(S& s) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} + void baz(S& s) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} +}; +void use() { + std::string_view local_sv; + std::string local_s; + S s; + // Capture an 'int'. + int local; + captureInt(1, // expected-warning {{object captured by 's' will be destroyed at the end of the full-expression}} + s); + captureRValInt(1, s); // expected-warning {{object captured by 's'}} + captureInt(local, s); + noCaptureInt(1, s); + noCaptureInt(local, s); + + // Capture lifetimebound pointer. + capturePointer(getPointerLB(std::string()), s); // expected-warning {{object captured by 's'}} + capturePointer(getPointerLB(*getPointerLB(std::string())), s); // expected-warning {{object captured by 's'}} + capturePointer(getPointerNoLB(std::string()), s); + + // Capture using std::string_view. + captureSV(local_sv, s); + captureSV(std::string(), // expected-warning {{object captured by 's'}} + s); + captureSV(substr( + std::string() // expected-warning {{object captured by 's'}} + ), s); + captureSV(substr(local_s), s); + captureSV(strcopy(std::string()), s); + captureRValSV(std::move(local_sv), s); + captureRValSV(std::string(), s); // expected-warning {{object captured by 's'}} + captureRValSV(std::string_view{"abcd"}, s); + captureRValSV(substr(local_s), s); + captureRValSV(substr(std::string()), s); // expected-warning {{object captured by 's'}} + captureRValSV(strcopy(std::string()), s); + noCaptureSV(local_sv, s); + noCaptureSV(std::string(), s); + noCaptureSV(substr(std::string()), s); + + // Capture using std::string. + captureS(std::string(), s); // expected-warning {{object captured by 's'}} + captureS(local_s, s); + captureRValS(std::move(local_s), s); + captureRValS(std::string(), s); // expected-warning {{object captured by 's'}} + + // Member functions. + s.captureInt(1); // expected-warning {{object captured by 's'}} + s.captureSV(std::string()); // expected-warning {{object captured by 's'}} + s.captureSV(substr(std::string())); // expected-warning {{object captured by 's'}} + s.captureSV(strcopy(std::string())); + + // 'this' is captured. + ThisIsCaptured{}.capture(s); // expected-warning {{object captured by 's'}} + ThisIsCaptured TIS; + TIS.capture(s); +} +class [[gsl::Pointer()]] my_string_view : public std::string_view {}; +class my_string_view_not_pointer : public std::string_view {}; +std::optional<std::string_view> getOptionalSV(); +std::optional<std::string> getOptionalS(); +std::optional<my_string_view> getOptionalMySV(); +std::optional<my_string_view_not_pointer> getOptionalMySVNotP(); +my_string_view getMySV(); +my_string_view_not_pointer getMySVNotP(); + +template<class T> +struct MySet { +void insert(T&& t [[clang::lifetime_capture_by(this)]]); +void insert(const T& t [[clang::lifetime_capture_by(this)]]); +}; +void user_defined_containers() { + MySet<int> set_of_int; + set_of_int.insert(1); // expected-warning {{object captured by 'set_of_int' will be destroyed}} + MySet<std::string_view> set_of_sv; + set_of_sv.insert(std::string()); // expected-warning {{object captured by 'set_of_sv' will be destroyed}} +} +} // namespace lifetime_capture_by +// Test for templated code. +// 2 nested function calls foo(sv, bar(sv, setsv)); \ No newline at end of file diff --git a/clang/test/SemaCXX/attr-lifetimebound.cpp b/clang/test/SemaCXX/attr-lifetimebound.cpp index 81e9193cf76a04..f89b556f5bba08 100644 --- a/clang/test/SemaCXX/attr-lifetimebound.cpp +++ b/clang/test/SemaCXX/attr-lifetimebound.cpp @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -std=c++23 -verify %s namespace usage_invalid { - void void_return(int ¶m [[clang::lifetimebound]]); // expected-error {{'lifetimebound' attribute cannot be applied to a parameter of a function that returns void}} + void void_return(int ¶m [[clang::lifetimebound]]); // expected-error {{'lifetimebound' attribute cannot be applied to a parameter of a function that returns void; did you mean 'lifetime_capture_by(X)'}} int *not_class_member() [[clang::lifetimebound]]; // expected-error {{non-member function has no implicit object parameter}} struct A { @@ -11,7 +11,7 @@ namespace usage_invalid { int *explicit_object(this A&) [[clang::lifetimebound]]; // expected-error {{explicit object member function has no implicit object parameter}} int not_function [[clang::lifetimebound]]; // expected-error {{only applies to parameters and implicit object parameters}} int [[clang::lifetimebound]] also_not_function; // expected-error {{cannot be applied to types}} - void void_return_member() [[clang::lifetimebound]]; // expected-error {{'lifetimebound' attribute cannot be applied to an implicit object parameter of a function that returns void}} + void void_return_member() [[clang::lifetimebound]]; // expected-error {{'lifetimebound' attribute cannot be applied to an implicit object parameter of a function that returns void; did you mean 'lifetime_capture_by(X)'}} }; int *attr_with_param(int ¶m [[clang::lifetimebound(42)]]); // expected-error {{takes no arguments}} } >From 8522c58c9778463e0e67b1bf49dc26408d906d50 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 13 Nov 2024 14:48:58 +0000 Subject: [PATCH 2/7] add tests for template containers and implement capture by global/unknown --- .../clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/CheckExprLifetime.cpp | 8 +- clang/lib/Sema/CheckExprLifetime.h | 6 +- clang/lib/Sema/SemaChecking.cpp | 6 +- .../Sema/warn-lifetime-analysis-nocfg.cpp | 171 ++++++++++++++---- 5 files changed, 155 insertions(+), 38 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 58745d450ed63f..62c1fc7e001d46 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10231,7 +10231,7 @@ def warn_dangling_pointer_assignment : Warning< "will be destroyed at the end of the full-expression">, InGroup<DanglingAssignment>; def warn_dangling_reference_captured : Warning< - "object captured by '%0' will be destroyed at the end of the full-expression">, + "object whose reference is captured%select{| by '%1'}0 will be destroyed at the end of the full-expression">, InGroup<DanglingCapture>; // For non-floating point, expressions of the form x == x or x != x diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 81e26f48fb8851..7f52055d4a9c61 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -1214,8 +1214,12 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, assert(shouldLifetimeExtendThroughPath(Path) == PathLifetimeKind::NoExtend && "No lifetime extension in function calls"); - SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) - << CapEntity->Entity << DiagRange; + if (CapEntity->Entity != nullptr) + SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) + << true << CapEntity->Entity << DiagRange; + else + SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) + << false << "<ignore>" << DiagRange; return false; } diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index e109b73b280e9e..bcc29f502708c6 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -26,12 +26,14 @@ struct AssignedEntity { }; struct CapturingEntity { - // In an function call involving a lifetime capture, this would be the - // argument capturing the lifetime of another argument. + // In an function call involving a lifetime capture, this would be the + // argument capturing the lifetime of another argument. // void addToSet(std::string_view sv [[clang::lifetime_capture_by(setsv)]], // set<std::string_view>& setsv); // set<std::string_view> setsv; // addToSet(std::string(), setsv); // Here 'setsv' is the 'Entity'. + // + // This is 'nullptr' when the capturing entity is 'global' or 'unknown'. Expr *Entity = nullptr; }; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 27edb56df3abc7..9f8174d206a80a 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -16,6 +16,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" #include "clang/AST/AttrIterator.h" +#include "clang/AST/Attrs.inc" #include "clang/AST/CharUnits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" @@ -3233,7 +3234,10 @@ void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl, void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction, const Expr *ThisArg, ArrayRef<const Expr *> Args) { - auto GetArgAt = [&](int Idx) { + auto GetArgAt = [&](int Idx) -> Expr * { + if (Idx == LifetimeCaptureByAttr::GLOBAL || + Idx == LifetimeCaptureByAttr::UNKNOWN) + return nullptr; if (IsMemberFunction && Idx == 0) return const_cast<Expr *>(ThisArg); return const_cast<Expr *>(Args[Idx - int(IsMemberFunction)]); diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 49bbc05c009ff9..9627054167b78f 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s +// RUN: %clang_cc1 --std=c++20 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s struct [[gsl::Owner(int)]] MyIntOwner { MyIntOwner(); int &operator*(); @@ -252,6 +252,19 @@ struct reference_wrapper { template<typename T> reference_wrapper<T> ref(T& t) noexcept; + +struct false_type { + static constexpr bool value = false; + constexpr operator bool() const noexcept { return value; } +}; +struct true_type { + static constexpr bool value = true; + constexpr operator bool() const noexcept { return value; } +}; + +template<class T> struct is_pointer : false_type {}; +template<class T> struct is_pointer<T*> : true_type {}; +template<class T> struct is_pointer<T* const> : true_type {}; } struct Unannotated { @@ -806,95 +819,189 @@ struct S { void captureInt(const int&x [[clang::lifetime_capture_by(s)]], S&s); void captureRValInt(int&&x [[clang::lifetime_capture_by(s)]], S&s); void noCaptureInt(int x [[clang::lifetime_capture_by(s)]], S&s); + std::string_view substr(const std::string& s [[clang::lifetimebound]]); std::string_view strcopy(const std::string& s); + void captureSV(std::string_view x [[clang::lifetime_capture_by(s)]], S&s); void captureRValSV(std::string_view&& x [[clang::lifetime_capture_by(s)]], S&s); void noCaptureSV(std::string_view x, S&s); void captureS(const std::string& x [[clang::lifetime_capture_by(s)]], S&s); void captureRValS(std::string&& x [[clang::lifetime_capture_by(s)]], S&s); + +const std::string& getLB(const std::string& s[[clang::lifetimebound]]); +const std::string& getLB(std::string_view sv[[clang::lifetimebound]]); const std::string* getPointerLB(const std::string& s[[clang::lifetimebound]]); const std::string* getPointerNoLB(const std::string& s); + void capturePointer(const std::string* x [[clang::lifetime_capture_by(s)]], S&s); + struct ThisIsCaptured { void capture(S& s) [[clang::lifetime_capture_by(s)]]; void bar(S& s) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} void baz(S& s) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} }; + +void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); +void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]); + void use() { std::string_view local_sv; std::string local_s; S s; // Capture an 'int'. int local; - captureInt(1, // expected-warning {{object captured by 's' will be destroyed at the end of the full-expression}} + captureInt(1, // expected-warning {{object whose reference is captured by 's' will be destroyed at the end of the full-expression}} s); - captureRValInt(1, s); // expected-warning {{object captured by 's'}} + captureRValInt(1, s); // expected-warning {{object whose reference is captured by 's'}} captureInt(local, s); noCaptureInt(1, s); noCaptureInt(local, s); - // Capture lifetimebound pointer. - capturePointer(getPointerLB(std::string()), s); // expected-warning {{object captured by 's'}} - capturePointer(getPointerLB(*getPointerLB(std::string())), s); // expected-warning {{object captured by 's'}} - capturePointer(getPointerNoLB(std::string()), s); - // Capture using std::string_view. captureSV(local_sv, s); - captureSV(std::string(), // expected-warning {{object captured by 's'}} + captureSV(std::string(), // expected-warning {{object whose reference is captured by 's'}} s); captureSV(substr( - std::string() // expected-warning {{object captured by 's'}} + std::string() // expected-warning {{object whose reference is captured by 's'}} ), s); captureSV(substr(local_s), s); captureSV(strcopy(std::string()), s); captureRValSV(std::move(local_sv), s); - captureRValSV(std::string(), s); // expected-warning {{object captured by 's'}} + captureRValSV(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} captureRValSV(std::string_view{"abcd"}, s); captureRValSV(substr(local_s), s); - captureRValSV(substr(std::string()), s); // expected-warning {{object captured by 's'}} + captureRValSV(substr(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} captureRValSV(strcopy(std::string()), s); noCaptureSV(local_sv, s); noCaptureSV(std::string(), s); noCaptureSV(substr(std::string()), s); // Capture using std::string. - captureS(std::string(), s); // expected-warning {{object captured by 's'}} + captureS(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} captureS(local_s, s); captureRValS(std::move(local_s), s); - captureRValS(std::string(), s); // expected-warning {{object captured by 's'}} + captureRValS(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} + + // Capture with lifetimebound. + captureSV(getLB(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} + captureSV(getLB(substr(std::string())), s); // expected-warning {{object whose reference is captured by 's'}} + captureSV(getLB(getLB( + std::string() // expected-warning {{object whose reference is captured by 's'}} + )), s); + capturePointer(getPointerLB(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} + capturePointer(getPointerLB(*getPointerLB( + std::string() // expected-warning {{object whose reference is captured by 's'}} + )), s); + capturePointer(getPointerNoLB(std::string()), s); // Member functions. - s.captureInt(1); // expected-warning {{object captured by 's'}} - s.captureSV(std::string()); // expected-warning {{object captured by 's'}} - s.captureSV(substr(std::string())); // expected-warning {{object captured by 's'}} + s.captureInt(1); // expected-warning {{object whose reference is captured by 's'}} + s.captureSV(std::string()); // expected-warning {{object whose reference is captured by 's'}} + s.captureSV(substr(std::string())); // expected-warning {{object whose reference is captured by 's'}} s.captureSV(strcopy(std::string())); // 'this' is captured. - ThisIsCaptured{}.capture(s); // expected-warning {{object captured by 's'}} + ThisIsCaptured{}.capture(s); // expected-warning {{object whose reference is captured by 's'}} ThisIsCaptured TIS; TIS.capture(s); + + // capture by global. + captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} + captureByGlobal(substr(std::string())); // expected-warning {{captured}} + captureByGlobal(local_s); + captureByGlobal(local_sv); + + // // capture by unknown. + captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} + captureByGlobal(substr(std::string())); // expected-warning {{captured}} + captureByGlobal(local_s); + captureByGlobal(local_sv); } -class [[gsl::Pointer()]] my_string_view : public std::string_view {}; -class my_string_view_not_pointer : public std::string_view {}; -std::optional<std::string_view> getOptionalSV(); -std::optional<std::string> getOptionalS(); -std::optional<my_string_view> getOptionalMySV(); -std::optional<my_string_view_not_pointer> getOptionalMySVNotP(); -my_string_view getMySV(); -my_string_view_not_pointer getMySVNotP(); +template<typename T> struct IsPointerLikeTypeImpl : std::false_type {}; +template<> struct IsPointerLikeTypeImpl<std::string_view> : std::true_type {}; +template<typename T> concept IsPointerLikeType = std::is_pointer<T>::value || IsPointerLikeTypeImpl<T>::value; + +// Templated containers having no distinction between pointer-like and other element type. template<class T> struct MySet { -void insert(T&& t [[clang::lifetime_capture_by(this)]]); -void insert(const T& t [[clang::lifetime_capture_by(this)]]); + void insert(T&& t [[clang::lifetime_capture_by(this)]]); + void insert(const T& t [[clang::lifetime_capture_by(this)]]); }; void user_defined_containers() { MySet<int> set_of_int; - set_of_int.insert(1); // expected-warning {{object captured by 'set_of_int' will be destroyed}} + set_of_int.insert(1); // expected-warning {{object whose reference is captured by 'set_of_int' will be destroyed}} MySet<std::string_view> set_of_sv; - set_of_sv.insert(std::string()); // expected-warning {{object captured by 'set_of_sv' will be destroyed}} + set_of_sv.insert(std::string()); // expected-warning {{object whose reference is captured by 'set_of_sv' will be destroyed}} +} + +// Templated containers having **which distinguishes** between pointer-like and other element type. +template<class T> +struct MyVector { + void push_back(T&& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; + void push_back(const T& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; + + void push_back(T&& t) requires (!IsPointerLikeType<T>); + void push_back(const T& t) requires (!IsPointerLikeType<T>); +}; + +// Container of pointers. +struct [[gsl::Pointer()]] MyStringView : public std::string_view { + MyStringView(); + MyStringView(std::string_view&&); + MyStringView(const MyStringView&); + MyStringView(const std::string&); +}; +template<> struct IsPointerLikeTypeImpl<MyStringView> : std::true_type {}; + +std::optional<std::string_view> getOptionalSV(); +std::optional<std::string> getOptionalS(); +std::optional<MyStringView> getOptionalMySV(); +MyStringView getMySV(); + +class MyStringViewNotPointer : public std::string_view {}; +std::optional<MyStringViewNotPointer> getOptionalMySVNotP(); +MyStringViewNotPointer getMySVNotP(); + +void container_of_pointers() { + std::string local; + MyVector<std::string> vs; + vs.push_back(std::string()); // Ok. + + MyVector<std::string_view> vsv; + vsv.push_back(std::string()); // expected-warning {{object whose reference is captured by 'vsv'}} + vsv.push_back(substr(std::string())); // expected-warning {{object whose reference is captured by 'vsv'}} + + MyVector<const std::string*> vp; + vp.push_back(getPointerLB(std::string())); // expected-warning {{object whose reference is captured by 'vp'}} + vp.push_back(getPointerLB(*getPointerLB(std::string()))); // expected-warning {{object whose reference is captured by 'vp'}} + vp.push_back(getPointerLB(local)); + vp.push_back(getPointerNoLB(std::string())); + + // User-defined [[gsl::Pointer]] + vsv.push_back(getMySV()); + vsv.push_back(getMySVNotP()); + + // Vector of user defined gsl::Pointer. + MyVector<MyStringView> vmysv; + vmysv.push_back(getMySV()); + vmysv.push_back(MyStringView{}); + vmysv.push_back(std::string_view{}); + vmysv.push_back(std::string{}); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(substr(std::string{})); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(getLB(substr(std::string{}))); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(strcopy(getLB(substr(std::string{})))); + + // With std::optional container. + std::optional<std::string_view> optional; + vsv.push_back(optional.value()); + vsv.push_back(getOptionalS().value()); // expected-warning {{object whose reference is captured by 'vsv'}} + vsv.push_back(getOptionalSV().value()); + vsv.push_back(getOptionalMySV().value()); + + // (maybe) FIXME: We may choose to diagnose the following case. + // This happens because 'MyStringViewNotPointer' is not marked as a [[gsl::Pointer]] but is derived from one. + vsv.push_back(getOptionalMySVNotP().value()); // expected-warning {{object whose reference is captured by 'vsv'}} } } // namespace lifetime_capture_by -// Test for templated code. -// 2 nested function calls foo(sv, bar(sv, setsv)); \ No newline at end of file >From f349aa811fe312247e3bebb064b374cf454ed798 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 13 Nov 2024 14:58:46 +0000 Subject: [PATCH 3/7] simply some code --- clang/lib/Sema/CheckExprLifetime.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 7f52055d4a9c61..d5918c85ae3fa4 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -254,10 +254,9 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, LocalVisitor Visit); template <typename T> static bool isRecordWithAttr(QualType Type) { - CXXRecordDecl *RD = Type.getNonReferenceType()->getAsCXXRecordDecl(); - if (!RD) - return false; - return RD->hasAttr<T>(); + if (auto *RD = Type.getNonReferenceType()->getAsCXXRecordDecl()) + return RD->hasAttr<T>(); + return false; } // Decl::isInStdNamespace will return false for iterators in some STL >From ab30f17705f73a7674d062c82f09ac5d8620bd5c Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Wed, 13 Nov 2024 18:12:57 +0000 Subject: [PATCH 4/7] address comments --- .../clang/Basic/DiagnosticSemaKinds.td | 10 +- clang/lib/Sema/CheckExprLifetime.cpp | 14 ++- clang/lib/Sema/SemaChecking.cpp | 1 - .../Sema/warn-lifetime-analysis-nocfg.cpp | 110 +++++++++--------- 4 files changed, 70 insertions(+), 65 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 62c1fc7e001d46..daa89553a4abd8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10135,7 +10135,8 @@ def err_lifetimebound_parameter_void_return_type : Error< "that returns void; did you mean 'lifetime_capture_by(X)'">; def err_lifetimebound_implicit_object_parameter_void_return_type : Error< "'lifetimebound' attribute cannot be applied to an implicit object " - "parameter of a function that returns void; did you mean 'lifetime_capture_by(X)'">; + "parameter of a function that returns void; " + "did you mean 'lifetime_capture_by(X)'">; // CHECK: returning address/reference of stack memory def warn_ret_stack_addr_ref : Warning< @@ -10231,8 +10232,11 @@ def warn_dangling_pointer_assignment : Warning< "will be destroyed at the end of the full-expression">, InGroup<DanglingAssignment>; def warn_dangling_reference_captured : Warning< - "object whose reference is captured%select{| by '%1'}0 will be destroyed at the end of the full-expression">, - InGroup<DanglingCapture>; + "object whose reference is captured by '%0' will be destroyed at the end of " + "the full-expression">, InGroup<DanglingCapture>; +def warn_dangling_reference_captured_by_unknown : Warning< + "object whose reference is captured will be destroyed at the end of " + "the full-expression">, InGroup<DanglingCapture>; // For non-floating point, expressions of the form x == x or x != x // should result in a warning, since these always evaluate to a constant. diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index d5918c85ae3fa4..475c8f93c68f85 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -1208,17 +1208,19 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, } case LK_LifetimeCapture: { + // The captured entity has lifetime beyond the full-expression, + // and the capturing entity does too, so don't warn. if (!MTE) return false; assert(shouldLifetimeExtendThroughPath(Path) == PathLifetimeKind::NoExtend && "No lifetime extension in function calls"); - if (CapEntity->Entity != nullptr) + if (CapEntity->Entity) SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) - << true << CapEntity->Entity << DiagRange; + << CapEntity->Entity << DiagRange; else - SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured) - << false << "<ignore>" << DiagRange; + SemaRef.Diag(DiagLoc, diag::warn_dangling_reference_captured_by_unknown) + << DiagRange; return false; } @@ -1471,13 +1473,13 @@ void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, LifetimeKind LK = LTResult.getInt(); const InitializedEntity *ExtendingEntity = LTResult.getPointer(); checkExprLifetimeImpl(SemaRef, &Entity, ExtendingEntity, LK, - /*AEntity*/ nullptr, /*CapEntity=*/nullptr, Init); + /*AEntity=*/nullptr, /*CapEntity=*/nullptr, Init); } void checkExprLifetimeMustTailArg(Sema &SemaRef, const InitializedEntity &Entity, Expr *Init) { checkExprLifetimeImpl(SemaRef, &Entity, nullptr, LK_MustTail, - /*AEntity*/ nullptr, /*CapEntity=*/nullptr, Init); + /*AEntity=*/nullptr, /*CapEntity=*/nullptr, Init); } void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 9f8174d206a80a..8df2d0acd352ab 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -16,7 +16,6 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/Attr.h" #include "clang/AST/AttrIterator.h" -#include "clang/AST/Attrs.inc" #include "clang/AST/CharUnits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 9627054167b78f..34b50d09cd94c7 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -808,38 +808,38 @@ void test13() { } // namespace GH100526 namespace lifetime_capture_by { -struct S { +struct X { const int *x; - void captureInt(const int&x [[clang::lifetime_capture_by(this)]]) { this->x = &x; } + void captureInt(const int& x [[clang::lifetime_capture_by(this)]]) { this->x = &x; } void captureSV(std::string_view sv [[clang::lifetime_capture_by(this)]]); }; /////////////////////////// // Detect dangling cases. /////////////////////////// -void captureInt(const int&x [[clang::lifetime_capture_by(s)]], S&s); -void captureRValInt(int&&x [[clang::lifetime_capture_by(s)]], S&s); -void noCaptureInt(int x [[clang::lifetime_capture_by(s)]], S&s); +void captureInt(const int& i [[clang::lifetime_capture_by(x)]], X& x); +void captureRValInt(int&& i [[clang::lifetime_capture_by(x)]], X& x); +void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X& x); std::string_view substr(const std::string& s [[clang::lifetimebound]]); std::string_view strcopy(const std::string& s); -void captureSV(std::string_view x [[clang::lifetime_capture_by(s)]], S&s); -void captureRValSV(std::string_view&& x [[clang::lifetime_capture_by(s)]], S&s); -void noCaptureSV(std::string_view x, S&s); -void captureS(const std::string& x [[clang::lifetime_capture_by(s)]], S&s); -void captureRValS(std::string&& x [[clang::lifetime_capture_by(s)]], S&s); +void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X& x); +void captureRValSV(std::string_view&& sv [[clang::lifetime_capture_by(x)]], X& x); +void noCaptureSV(std::string_view sv, X& x); +void captureS(const std::string& s [[clang::lifetime_capture_by(x)]], X& x); +void captureRValS(std::string&& s [[clang::lifetime_capture_by(x)]], X& x); const std::string& getLB(const std::string& s[[clang::lifetimebound]]); const std::string& getLB(std::string_view sv[[clang::lifetimebound]]); const std::string* getPointerLB(const std::string& s[[clang::lifetimebound]]); const std::string* getPointerNoLB(const std::string& s); -void capturePointer(const std::string* x [[clang::lifetime_capture_by(s)]], S&s); +void capturePointer(const std::string* sp [[clang::lifetime_capture_by(x)]], X& x); struct ThisIsCaptured { - void capture(S& s) [[clang::lifetime_capture_by(s)]]; - void bar(S& s) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} - void baz(S& s) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} + void capture(X& x) [[clang::lifetime_capture_by(x)]]; + void bar(X& x) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} + void baz(X& x) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} }; void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); @@ -848,63 +848,63 @@ void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]] void use() { std::string_view local_sv; std::string local_s; - S s; + X x; // Capture an 'int'. int local; - captureInt(1, // expected-warning {{object whose reference is captured by 's' will be destroyed at the end of the full-expression}} - s); - captureRValInt(1, s); // expected-warning {{object whose reference is captured by 's'}} - captureInt(local, s); - noCaptureInt(1, s); - noCaptureInt(local, s); + captureInt(1, // expected-warning {{object whose reference is captured by 'x' will be destroyed at the end of the full-expression}} + x); + captureRValInt(1, x); // expected-warning {{object whose reference is captured by 'x'}} + captureInt(local, x); + noCaptureInt(1, x); + noCaptureInt(local, x); // Capture using std::string_view. - captureSV(local_sv, s); - captureSV(std::string(), // expected-warning {{object whose reference is captured by 's'}} - s); + captureSV(local_sv, x); + captureSV(std::string(), // expected-warning {{object whose reference is captured by 'x'}} + x); captureSV(substr( - std::string() // expected-warning {{object whose reference is captured by 's'}} - ), s); - captureSV(substr(local_s), s); - captureSV(strcopy(std::string()), s); - captureRValSV(std::move(local_sv), s); - captureRValSV(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} - captureRValSV(std::string_view{"abcd"}, s); - captureRValSV(substr(local_s), s); - captureRValSV(substr(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} - captureRValSV(strcopy(std::string()), s); - noCaptureSV(local_sv, s); - noCaptureSV(std::string(), s); - noCaptureSV(substr(std::string()), s); + std::string() // expected-warning {{object whose reference is captured by 'x'}} + ), x); + captureSV(substr(local_s), x); + captureSV(strcopy(std::string()), x); + captureRValSV(std::move(local_sv), x); + captureRValSV(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValSV(std::string_view{"abcd"}, x); + captureRValSV(substr(local_s), x); + captureRValSV(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValSV(strcopy(std::string()), x); + noCaptureSV(local_sv, x); + noCaptureSV(std::string(), x); + noCaptureSV(substr(std::string()), x); // Capture using std::string. - captureS(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} - captureS(local_s, s); - captureRValS(std::move(local_s), s); - captureRValS(std::string(), s); // expected-warning {{object whose reference is captured by 's'}} + captureS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureS(local_s, x); + captureRValS(std::move(local_s), x); + captureRValS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} // Capture with lifetimebound. - captureSV(getLB(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} - captureSV(getLB(substr(std::string())), s); // expected-warning {{object whose reference is captured by 's'}} + captureSV(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureSV(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}} captureSV(getLB(getLB( - std::string() // expected-warning {{object whose reference is captured by 's'}} - )), s); - capturePointer(getPointerLB(std::string()), s); // expected-warning {{object whose reference is captured by 's'}} + std::string() // expected-warning {{object whose reference is captured by 'x'}} + )), x); + capturePointer(getPointerLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} capturePointer(getPointerLB(*getPointerLB( - std::string() // expected-warning {{object whose reference is captured by 's'}} - )), s); - capturePointer(getPointerNoLB(std::string()), s); + std::string() // expected-warning {{object whose reference is captured by 'x'}} + )), x); + capturePointer(getPointerNoLB(std::string()), x); // Member functions. - s.captureInt(1); // expected-warning {{object whose reference is captured by 's'}} - s.captureSV(std::string()); // expected-warning {{object whose reference is captured by 's'}} - s.captureSV(substr(std::string())); // expected-warning {{object whose reference is captured by 's'}} - s.captureSV(strcopy(std::string())); + x.captureInt(1); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(std::string()); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(substr(std::string())); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(strcopy(std::string())); // 'this' is captured. - ThisIsCaptured{}.capture(s); // expected-warning {{object whose reference is captured by 's'}} + ThisIsCaptured{}.capture(x); // expected-warning {{object whose reference is captured by 'x'}} ThisIsCaptured TIS; - TIS.capture(s); + TIS.capture(x); // capture by global. captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} >From 1036946da5a184894fce8240f4a8286280f63c4d Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Thu, 14 Nov 2024 17:27:13 +0000 Subject: [PATCH 5/7] address comments --- clang/lib/Sema/CheckExprLifetime.cpp | 29 ++++++++++------- .../Sema/warn-lifetime-analysis-nocfg.cpp | 32 +++++++++---------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 475c8f93c68f85..2c60ced42eb79b 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -1122,9 +1122,9 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, const InitializedEntity *ExtendingEntity, LifetimeKind LK, const AssignedEntity *AEntity, const CapturingEntity *CapEntity, Expr *Init) { - assert((AEntity && LK == LK_Assignment) || - (CapEntity && LK == LK_LifetimeCapture) || - (InitEntity && LK != LK_Assignment)); + assert(!AEntity || LK == LK_Assignment); + assert(!CapEntity || LK == LK_LifetimeCapture); + assert(!InitEntity || (LK != LK_Assignment && LK != LK_LifetimeCapture)); // If this entity doesn't have an interesting lifetime, don't bother looking // for temporaries within its initializer. if (LK == LK_FullExpression) @@ -1439,14 +1439,17 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, }; bool HasReferenceBinding = Init->isGLValue(); llvm::SmallVector<IndirectLocalPathEntry, 8> Path; - if (LK == LK_Assignment && - shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) { - Path.push_back( - {isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator) - ? IndirectLocalPathEntry::LifetimeBoundCall - : IndirectLocalPathEntry::GslPointerAssignment, - Init}); - } else if (LK == LK_LifetimeCapture) { + switch (LK) { + case LK_Assignment: { + if (shouldRunGSLAssignmentAnalysis(SemaRef, *AEntity)) + Path.push_back( + {isAssignmentOperatorLifetimeBound(AEntity->AssignmentOperator) + ? IndirectLocalPathEntry::LifetimeBoundCall + : IndirectLocalPathEntry::GslPointerAssignment, + Init}); + break; + } + case LK_LifetimeCapture: { Path.push_back({IndirectLocalPathEntry::LifetimeCapture, Init}); if (isRecordWithAttr<PointerAttr>(Init->getType())) HasReferenceBinding = false; @@ -1455,6 +1458,10 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Init); MTE && isPointerLikeType(Init->getType())) Init = MTE->getSubExpr(); + break; + } + default: + break; } if (HasReferenceBinding) diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index 34b50d09cd94c7..edf21def979eaf 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -816,30 +816,30 @@ struct X { /////////////////////////// // Detect dangling cases. /////////////////////////// -void captureInt(const int& i [[clang::lifetime_capture_by(x)]], X& x); -void captureRValInt(int&& i [[clang::lifetime_capture_by(x)]], X& x); -void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X& x); +void captureInt(const int &i [[clang::lifetime_capture_by(x)]], X &x); +void captureRValInt(int &&i [[clang::lifetime_capture_by(x)]], X &x); +void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X &x); std::string_view substr(const std::string& s [[clang::lifetimebound]]); std::string_view strcopy(const std::string& s); -void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X& x); -void captureRValSV(std::string_view&& sv [[clang::lifetime_capture_by(x)]], X& x); -void noCaptureSV(std::string_view sv, X& x); -void captureS(const std::string& s [[clang::lifetime_capture_by(x)]], X& x); -void captureRValS(std::string&& s [[clang::lifetime_capture_by(x)]], X& x); +void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValSV(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x); +void noCaptureSV(std::string_view sv, X &x); +void captureS(const std::string &s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValS(std::string &&s [[clang::lifetime_capture_by(x)]], X &x); -const std::string& getLB(const std::string& s[[clang::lifetimebound]]); -const std::string& getLB(std::string_view sv[[clang::lifetimebound]]); -const std::string* getPointerLB(const std::string& s[[clang::lifetimebound]]); -const std::string* getPointerNoLB(const std::string& s); +const std::string& getLB(const std::string &s [[clang::lifetimebound]]); +const std::string& getLB(std::string_view sv [[clang::lifetimebound]]); +const std::string* getPointerLB(const std::string &s [[clang::lifetimebound]]); +const std::string* getPointerNoLB(const std::string &s); -void capturePointer(const std::string* sp [[clang::lifetime_capture_by(x)]], X& x); +void capturePointer(const std::string* sp [[clang::lifetime_capture_by(x)]], X &x); struct ThisIsCaptured { - void capture(X& x) [[clang::lifetime_capture_by(x)]]; - void bar(X& x) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} - void baz(X& x) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} + void capture(X &x) [[clang::lifetime_capture_by(x)]]; + void bar(X &x) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} + void baz(X &x) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} }; void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); >From c1ac443d2206fe971d625703db2414422ce4e916 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Fri, 15 Nov 2024 10:41:08 +0000 Subject: [PATCH 6/7] move tests; update docs --- clang/include/clang/Basic/AttrDocs.td | 34 +- clang/test/Sema/Inputs/lifetime-analysis.h | 138 +++++++ .../warn-lifetime-analysis-capture-by.cpp | 220 ++++++++++++ .../Sema/warn-lifetime-analysis-nocfg.cpp | 339 +----------------- 4 files changed, 388 insertions(+), 343 deletions(-) create mode 100644 clang/test/Sema/Inputs/lifetime-analysis.h create mode 100644 clang/test/Sema/warn-lifetime-analysis-capture-by.cpp diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 21fcd183e8969c..933ea032d88ab8 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3925,13 +3925,20 @@ def LifetimeCaptureByDocs : Documentation { parameter or implicit object parameter indicates that that objects that are referred to by that parameter may also be referred to by the capturing entity ``X``. -By default, a reference is considered to refer to its referenced object, a -pointer is considered to refer to its pointee, a ``std::initializer_list<T>`` -is considered to refer to its underlying array, and aggregates (arrays and -simple ``struct``\s) are considered to refer to all objects that their -transitive subobjects refer to. +By default: + +- A reference is considered to refer to its referenced object. +- A pointer is considered to refer to its pointee. +- A ``std::initializer_list<T>`` is considered to refer to its underlying array. +- Aggregates (arrays and simple ``struct``\s) are considered to refer to all + objects that their transitive subobjects refer to. +- View types (types annotated with [[``gsl::Pointer()``]]) are considered to refer + to its pointee. This holds true even if the view type appears as a reference. + For example, both ``std::string_view`` and ``const std::string_view &`` are + considered to refer to a ``std::string``. The capturing entity ``X`` can be one of the following: + - Another (named) function parameter. .. code-block:: c++ @@ -3951,7 +3958,7 @@ The capturing entity ``X`` can be one of the following: std::set<std::string_view> s; }; -- 'global', 'unknown' (without quotes). +- `global`, `unknown`. .. code-block:: c++ @@ -3983,6 +3990,21 @@ The attribute supports specifying more than one capturing entities: s2.insert(a); } +Currently clang would diagnose when a temporary is used as an argument to a +parameter whose reference is captured. + +.. code-block:: c++ + + void addToSet(std::string_view a [[clang::lifetime_capture_by(s)]], std::set<std::string_view>& s) { + s.insert(a); + } + void use() { + std::set<std::string_view> s; + addToSet(std::string(), s); // Warning: object whose reference is captured by 's' will be destroyed at the end of the full-expression. + std::string local; + addToSet(local, s); // Ok. + } + .. _`lifetimebound`: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound }]; } diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h new file mode 100644 index 00000000000000..d471973c5fdd50 --- /dev/null +++ b/clang/test/Sema/Inputs/lifetime-analysis.h @@ -0,0 +1,138 @@ + +namespace __gnu_cxx { +template <typename T> +struct basic_iterator { + basic_iterator operator++(); + T& operator*() const; + T* operator->() const; +}; + +template<typename T> +bool operator!=(basic_iterator<T>, basic_iterator<T>); +} + +namespace std { +template<typename T> struct remove_reference { typedef T type; }; +template<typename T> struct remove_reference<T &> { typedef T type; }; +template<typename T> struct remove_reference<T &&> { typedef T type; }; + +template<typename T> +typename remove_reference<T>::type &&move(T &&t) noexcept; + +template <typename C> +auto data(const C &c) -> decltype(c.data()); + +template <typename C> +auto begin(C &c) -> decltype(c.begin()); + +template<typename T, int N> +T *begin(T (&array)[N]); + +using size_t = decltype(sizeof(0)); + +template<typename T> +struct initializer_list { + const T* ptr; size_t sz; +}; +template<typename T> class allocator {}; +template <typename T, typename Alloc = allocator<T>> +struct vector { + typedef __gnu_cxx::basic_iterator<T> iterator; + iterator begin(); + iterator end(); + const T *data() const; + vector(); + vector(initializer_list<T> __l, + const Alloc& alloc = Alloc()); + + template<typename InputIterator> + vector(InputIterator first, InputIterator __last); + + T &at(int n); +}; + +template<typename T> +struct basic_string_view { + basic_string_view(); + basic_string_view(const T *); + const T *begin() const; +}; +using string_view = basic_string_view<char>; + +template<class _Mystr> struct iter { + iter& operator-=(int); + + iter operator-(int _Off) const { + iter _Tmp = *this; + return _Tmp -= _Off; + } +}; + +template<typename T> +struct basic_string { + basic_string(); + basic_string(const T *); + const T *c_str() const; + operator basic_string_view<T> () const; + using const_iterator = iter<T>; +}; +using string = basic_string<char>; + +template<typename T> +struct unique_ptr { + T &operator*(); + T *get() const; +}; + +template<typename T> +struct optional { + optional(); + optional(const T&); + + template<typename U = T> + optional(U&& t); + + template<typename U> + optional(optional<U>&& __t); + + T &operator*() &; + T &&operator*() &&; + T &value() &; + T &&value() &&; +}; +template<typename T> +optional<__decay(T)> make_optional(T&&); + + +template<typename T> +struct stack { + T &top(); +}; + +struct any {}; + +template<typename T> +T any_cast(const any& operand); + +template<typename T> +struct reference_wrapper { + template<typename U> + reference_wrapper(U &&); +}; + +template<typename T> +reference_wrapper<T> ref(T& t) noexcept; + +struct false_type { + static constexpr bool value = false; + constexpr operator bool() const noexcept { return value; } +}; +struct true_type { + static constexpr bool value = true; + constexpr operator bool() const noexcept { return value; } +}; + +template<class T> struct is_pointer : false_type {}; +template<class T> struct is_pointer<T*> : true_type {}; +template<class T> struct is_pointer<T* const> : true_type {}; +} \ No newline at end of file diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp new file mode 100644 index 00000000000000..069fce1f70b9e3 --- /dev/null +++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp @@ -0,0 +1,220 @@ +// RUN: %clang_cc1 --std=c++20 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s + +#include "Inputs/lifetime-analysis.h" + +struct X { + const int *x; + void captureInt(const int& x [[clang::lifetime_capture_by(this)]]) { this->x = &x; } + void captureSV(std::string_view sv [[clang::lifetime_capture_by(this)]]); +}; +/////////////////////////// +// Detect dangling cases. +/////////////////////////// +void captureInt(const int &i [[clang::lifetime_capture_by(x)]], X &x); +void captureRValInt(int &&i [[clang::lifetime_capture_by(x)]], X &x); +void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X &x); + +std::string_view substr(const std::string& s [[clang::lifetimebound]]); +std::string_view strcopy(const std::string& s); + +void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValSV(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x); +void noCaptureSV(std::string_view sv, X &x); +void captureS(const std::string &s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValS(std::string &&s [[clang::lifetime_capture_by(x)]], X &x); + +const std::string& getLB(const std::string &s [[clang::lifetimebound]]); +const std::string& getLB(std::string_view sv [[clang::lifetimebound]]); +const std::string* getPointerLB(const std::string &s [[clang::lifetimebound]]); +const std::string* getPointerNoLB(const std::string &s); + +void capturePointer(const std::string* sp [[clang::lifetime_capture_by(x)]], X &x); + +struct ThisIsCaptured { + void capture(X &x) [[clang::lifetime_capture_by(x)]]; +}; + +void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); +void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]); + +void use() { + std::string_view local_sv; + std::string local_s; + X x; + // Capture an 'int'. + int local; + captureInt(1, // expected-warning {{object whose reference is captured by 'x' will be destroyed at the end of the full-expression}} + x); + captureRValInt(1, x); // expected-warning {{object whose reference is captured by 'x'}} + captureInt(local, x); + noCaptureInt(1, x); + noCaptureInt(local, x); + + // Capture using std::string_view. + captureSV(local_sv, x); + captureSV(std::string(), // expected-warning {{object whose reference is captured by 'x'}} + x); + captureSV(substr( + std::string() // expected-warning {{object whose reference is captured by 'x'}} + ), x); + captureSV(substr(local_s), x); + captureSV(strcopy(std::string()), x); + captureRValSV(std::move(local_sv), x); + captureRValSV(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValSV(std::string_view{"abcd"}, x); + captureRValSV(substr(local_s), x); + captureRValSV(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValSV(strcopy(std::string()), x); + noCaptureSV(local_sv, x); + noCaptureSV(std::string(), x); + noCaptureSV(substr(std::string()), x); + + // Capture using std::string. + captureS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureS(local_s, x); + captureRValS(std::move(local_s), x); + captureRValS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + + // Capture with lifetimebound. + captureSV(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureSV(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}} + captureSV(getLB(getLB( + std::string() // expected-warning {{object whose reference is captured by 'x'}} + )), x); + capturePointer(getPointerLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + capturePointer(getPointerLB(*getPointerLB( + std::string() // expected-warning {{object whose reference is captured by 'x'}} + )), x); + capturePointer(getPointerNoLB(std::string()), x); + + // Member functions. + x.captureInt(1); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(std::string()); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(substr(std::string())); // expected-warning {{object whose reference is captured by 'x'}} + x.captureSV(strcopy(std::string())); + + // 'this' is captured. + ThisIsCaptured{}.capture(x); // expected-warning {{object whose reference is captured by 'x'}} + ThisIsCaptured TIS; + TIS.capture(x); + + // capture by global. + captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} + captureByGlobal(substr(std::string())); // expected-warning {{captured}} + captureByGlobal(local_s); + captureByGlobal(local_sv); + + // // capture by unknown. + captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} + captureByGlobal(substr(std::string())); // expected-warning {{captured}} + captureByGlobal(local_s); + captureByGlobal(local_sv); +} + +template<typename T> struct IsPointerLikeTypeImpl : std::false_type {}; +template<> struct IsPointerLikeTypeImpl<std::string_view> : std::true_type {}; +template<typename T> concept IsPointerLikeType = std::is_pointer<T>::value || IsPointerLikeTypeImpl<T>::value; + +// Templated containers having no distinction between pointer-like and other element type. +template<class T> +struct MySet { + void insert(T&& t [[clang::lifetime_capture_by(this)]]); + void insert(const T& t [[clang::lifetime_capture_by(this)]]); +}; +void user_defined_containers() { + MySet<int> set_of_int; + set_of_int.insert(1); // expected-warning {{object whose reference is captured by 'set_of_int' will be destroyed}} + MySet<std::string_view> set_of_sv; + set_of_sv.insert(std::string()); // expected-warning {{object whose reference is captured by 'set_of_sv' will be destroyed}} +} + +// Templated containers having **which distinguishes** between pointer-like and other element type. +template<class T> +struct MyVector { + void push_back(T&& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; + void push_back(const T& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; + + void push_back(T&& t) requires (!IsPointerLikeType<T>); + void push_back(const T& t) requires (!IsPointerLikeType<T>); +}; + +// Container of pointers. +struct [[gsl::Pointer()]] MyStringView : public std::string_view { + MyStringView(); + MyStringView(std::string_view&&); + MyStringView(const MyStringView&); + MyStringView(const std::string&); +}; +template<> struct IsPointerLikeTypeImpl<MyStringView> : std::true_type {}; + +std::optional<std::string_view> getOptionalSV(); +std::optional<std::string> getOptionalS(); +std::optional<MyStringView> getOptionalMySV(); +MyStringView getMySV(); + +class MyStringViewNotPointer : public std::string_view {}; +std::optional<MyStringViewNotPointer> getOptionalMySVNotP(); +MyStringViewNotPointer getMySVNotP(); + +void container_of_pointers() { + std::string local; + MyVector<std::string> vs; + vs.push_back(std::string()); // Ok. + + MyVector<std::string_view> vsv; + vsv.push_back(std::string()); // expected-warning {{object whose reference is captured by 'vsv'}} + vsv.push_back(substr(std::string())); // expected-warning {{object whose reference is captured by 'vsv'}} + + MyVector<const std::string*> vp; + vp.push_back(getPointerLB(std::string())); // expected-warning {{object whose reference is captured by 'vp'}} + vp.push_back(getPointerLB(*getPointerLB(std::string()))); // expected-warning {{object whose reference is captured by 'vp'}} + vp.push_back(getPointerLB(local)); + vp.push_back(getPointerNoLB(std::string())); + + // User-defined [[gsl::Pointer]] + vsv.push_back(getMySV()); + vsv.push_back(getMySVNotP()); + + // Vector of user defined gsl::Pointer. + MyVector<MyStringView> vmysv; + vmysv.push_back(getMySV()); + vmysv.push_back(MyStringView{}); + vmysv.push_back(std::string_view{}); + vmysv.push_back(std::string{}); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(substr(std::string{})); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(getLB(substr(std::string{}))); // expected-warning {{object whose reference is captured by 'vmysv'}} + vmysv.push_back(strcopy(getLB(substr(std::string{})))); + + // With std::optional container. + std::optional<std::string_view> optionalSV; + vsv.push_back(optionalSV.value()); + vsv.push_back(getOptionalS().value()); // expected-warning {{object whose reference is captured by 'vsv'}} + vsv.push_back(getOptionalSV().value()); + vsv.push_back(getOptionalMySV().value()); + + // (maybe) FIXME: We may choose to diagnose the following case. + // This happens because 'MyStringViewNotPointer' is not marked as a [[gsl::Pointer]] but is derived from one. + vsv.push_back(getOptionalMySVNotP().value()); // expected-warning {{object whose reference is captured by 'vsv'}} +} +namespace temporary_views { +void capture1(std::string_view s [[clang::lifetime_capture_by(x)]], std::vector<std::string_view>& x); + +// Intended to capture the "string_view" itself +void capture2(const std::string_view& s [[clang::lifetime_capture_by(x)]], std::vector<std::string_view*>& x); +// Intended to capture the pointee of the "string_view" +void capture3(const std::string_view& s [[clang::lifetime_capture_by(x)]], std::vector<std::string_view>& x); + +void test1() { + std::vector<std::string_view> x1; + capture1(std::string(), x1); // expected-warning {{captured by 'x1'}} + capture1(std::string_view(), x1); + + std::vector<std::string_view*> x2; + capture2(std::string_view(), x2); // FIXME: Warn when the temporary view itself is captured. + capture2(std::string(), x2); // expected-warning {{captured by 'x2'}} + + std::vector<std::string_view> x3; + capture3(std::string_view(), x3); + capture3(std::string(), x3); // expected-warning {{captured by 'x3'}} +} +} \ No newline at end of file diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp index edf21def979eaf..c18ecd86ad06f0 100644 --- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 --std=c++20 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s +#include "Inputs/lifetime-analysis.h" struct [[gsl::Owner(int)]] MyIntOwner { MyIntOwner(); int &operator*(); @@ -129,143 +130,6 @@ void initLocalGslPtrWithTempOwner() { global2 = MyLongOwnerWithConversion{}; // expected-warning {{object backing the pointer global2 }} } -namespace __gnu_cxx { -template <typename T> -struct basic_iterator { - basic_iterator operator++(); - T& operator*() const; - T* operator->() const; -}; - -template<typename T> -bool operator!=(basic_iterator<T>, basic_iterator<T>); -} - -namespace std { -template<typename T> struct remove_reference { typedef T type; }; -template<typename T> struct remove_reference<T &> { typedef T type; }; -template<typename T> struct remove_reference<T &&> { typedef T type; }; - -template<typename T> -typename remove_reference<T>::type &&move(T &&t) noexcept; - -template <typename C> -auto data(const C &c) -> decltype(c.data()); - -template <typename C> -auto begin(C &c) -> decltype(c.begin()); - -template<typename T, int N> -T *begin(T (&array)[N]); - -using size_t = decltype(sizeof(0)); - -template<typename T> -struct initializer_list { - const T* ptr; size_t sz; -}; -template<typename T> class allocator {}; -template <typename T, typename Alloc = allocator<T>> -struct vector { - typedef __gnu_cxx::basic_iterator<T> iterator; - iterator begin(); - iterator end(); - const T *data() const; - vector(); - vector(initializer_list<T> __l, - const Alloc& alloc = Alloc()); - - template<typename InputIterator> - vector(InputIterator first, InputIterator __last); - - T &at(int n); -}; - -template<typename T> -struct basic_string_view { - basic_string_view(); - basic_string_view(const T *); - const T *begin() const; -}; -using string_view = basic_string_view<char>; - -template<class _Mystr> struct iter { - iter& operator-=(int); - - iter operator-(int _Off) const { - iter _Tmp = *this; - return _Tmp -= _Off; - } -}; - -template<typename T> -struct basic_string { - basic_string(); - basic_string(const T *); - const T *c_str() const; - operator basic_string_view<T> () const; - using const_iterator = iter<T>; -}; -using string = basic_string<char>; - -template<typename T> -struct unique_ptr { - T &operator*(); - T *get() const; -}; - -template<typename T> -struct optional { - optional(); - optional(const T&); - - template<typename U = T> - optional(U&& t); - - template<typename U> - optional(optional<U>&& __t); - - T &operator*() &; - T &&operator*() &&; - T &value() &; - T &&value() &&; -}; -template<typename T> -optional<__decay(T)> make_optional(T&&); - - -template<typename T> -struct stack { - T &top(); -}; - -struct any {}; - -template<typename T> -T any_cast(const any& operand); - -template<typename T> -struct reference_wrapper { - template<typename U> - reference_wrapper(U &&); -}; - -template<typename T> -reference_wrapper<T> ref(T& t) noexcept; - -struct false_type { - static constexpr bool value = false; - constexpr operator bool() const noexcept { return value; } -}; -struct true_type { - static constexpr bool value = true; - constexpr operator bool() const noexcept { return value; } -}; - -template<class T> struct is_pointer : false_type {}; -template<class T> struct is_pointer<T*> : true_type {}; -template<class T> struct is_pointer<T* const> : true_type {}; -} struct Unannotated { typedef std::vector<int>::iterator iterator; @@ -806,202 +670,3 @@ void test13() { } } // namespace GH100526 - -namespace lifetime_capture_by { -struct X { - const int *x; - void captureInt(const int& x [[clang::lifetime_capture_by(this)]]) { this->x = &x; } - void captureSV(std::string_view sv [[clang::lifetime_capture_by(this)]]); -}; -/////////////////////////// -// Detect dangling cases. -/////////////////////////// -void captureInt(const int &i [[clang::lifetime_capture_by(x)]], X &x); -void captureRValInt(int &&i [[clang::lifetime_capture_by(x)]], X &x); -void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X &x); - -std::string_view substr(const std::string& s [[clang::lifetimebound]]); -std::string_view strcopy(const std::string& s); - -void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X &x); -void captureRValSV(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x); -void noCaptureSV(std::string_view sv, X &x); -void captureS(const std::string &s [[clang::lifetime_capture_by(x)]], X &x); -void captureRValS(std::string &&s [[clang::lifetime_capture_by(x)]], X &x); - -const std::string& getLB(const std::string &s [[clang::lifetimebound]]); -const std::string& getLB(std::string_view sv [[clang::lifetimebound]]); -const std::string* getPointerLB(const std::string &s [[clang::lifetimebound]]); -const std::string* getPointerNoLB(const std::string &s); - -void capturePointer(const std::string* sp [[clang::lifetime_capture_by(x)]], X &x); - -struct ThisIsCaptured { - void capture(X &x) [[clang::lifetime_capture_by(x)]]; - void bar(X &x) [[clang::lifetime_capture_by(abcd)]]; // expected-error {{'lifetime_capture_by' attribute argument 'abcd' is not a known function parameter}} - void baz(X &x) [[clang::lifetime_capture_by(this)]]; // expected-error {{'lifetime_capture_by' argument references itself}} -}; - -void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); -void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]); - -void use() { - std::string_view local_sv; - std::string local_s; - X x; - // Capture an 'int'. - int local; - captureInt(1, // expected-warning {{object whose reference is captured by 'x' will be destroyed at the end of the full-expression}} - x); - captureRValInt(1, x); // expected-warning {{object whose reference is captured by 'x'}} - captureInt(local, x); - noCaptureInt(1, x); - noCaptureInt(local, x); - - // Capture using std::string_view. - captureSV(local_sv, x); - captureSV(std::string(), // expected-warning {{object whose reference is captured by 'x'}} - x); - captureSV(substr( - std::string() // expected-warning {{object whose reference is captured by 'x'}} - ), x); - captureSV(substr(local_s), x); - captureSV(strcopy(std::string()), x); - captureRValSV(std::move(local_sv), x); - captureRValSV(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} - captureRValSV(std::string_view{"abcd"}, x); - captureRValSV(substr(local_s), x); - captureRValSV(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} - captureRValSV(strcopy(std::string()), x); - noCaptureSV(local_sv, x); - noCaptureSV(std::string(), x); - noCaptureSV(substr(std::string()), x); - - // Capture using std::string. - captureS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} - captureS(local_s, x); - captureRValS(std::move(local_s), x); - captureRValS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} - - // Capture with lifetimebound. - captureSV(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} - captureSV(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}} - captureSV(getLB(getLB( - std::string() // expected-warning {{object whose reference is captured by 'x'}} - )), x); - capturePointer(getPointerLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} - capturePointer(getPointerLB(*getPointerLB( - std::string() // expected-warning {{object whose reference is captured by 'x'}} - )), x); - capturePointer(getPointerNoLB(std::string()), x); - - // Member functions. - x.captureInt(1); // expected-warning {{object whose reference is captured by 'x'}} - x.captureSV(std::string()); // expected-warning {{object whose reference is captured by 'x'}} - x.captureSV(substr(std::string())); // expected-warning {{object whose reference is captured by 'x'}} - x.captureSV(strcopy(std::string())); - - // 'this' is captured. - ThisIsCaptured{}.capture(x); // expected-warning {{object whose reference is captured by 'x'}} - ThisIsCaptured TIS; - TIS.capture(x); - - // capture by global. - captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} - captureByGlobal(substr(std::string())); // expected-warning {{captured}} - captureByGlobal(local_s); - captureByGlobal(local_sv); - - // // capture by unknown. - captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} - captureByGlobal(substr(std::string())); // expected-warning {{captured}} - captureByGlobal(local_s); - captureByGlobal(local_sv); -} - -template<typename T> struct IsPointerLikeTypeImpl : std::false_type {}; -template<> struct IsPointerLikeTypeImpl<std::string_view> : std::true_type {}; -template<typename T> concept IsPointerLikeType = std::is_pointer<T>::value || IsPointerLikeTypeImpl<T>::value; - -// Templated containers having no distinction between pointer-like and other element type. -template<class T> -struct MySet { - void insert(T&& t [[clang::lifetime_capture_by(this)]]); - void insert(const T& t [[clang::lifetime_capture_by(this)]]); -}; -void user_defined_containers() { - MySet<int> set_of_int; - set_of_int.insert(1); // expected-warning {{object whose reference is captured by 'set_of_int' will be destroyed}} - MySet<std::string_view> set_of_sv; - set_of_sv.insert(std::string()); // expected-warning {{object whose reference is captured by 'set_of_sv' will be destroyed}} -} - -// Templated containers having **which distinguishes** between pointer-like and other element type. -template<class T> -struct MyVector { - void push_back(T&& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; - void push_back(const T& t [[clang::lifetime_capture_by(this)]]) requires IsPointerLikeType<T>; - - void push_back(T&& t) requires (!IsPointerLikeType<T>); - void push_back(const T& t) requires (!IsPointerLikeType<T>); -}; - -// Container of pointers. -struct [[gsl::Pointer()]] MyStringView : public std::string_view { - MyStringView(); - MyStringView(std::string_view&&); - MyStringView(const MyStringView&); - MyStringView(const std::string&); -}; -template<> struct IsPointerLikeTypeImpl<MyStringView> : std::true_type {}; - -std::optional<std::string_view> getOptionalSV(); -std::optional<std::string> getOptionalS(); -std::optional<MyStringView> getOptionalMySV(); -MyStringView getMySV(); - -class MyStringViewNotPointer : public std::string_view {}; -std::optional<MyStringViewNotPointer> getOptionalMySVNotP(); -MyStringViewNotPointer getMySVNotP(); - -void container_of_pointers() { - std::string local; - MyVector<std::string> vs; - vs.push_back(std::string()); // Ok. - - MyVector<std::string_view> vsv; - vsv.push_back(std::string()); // expected-warning {{object whose reference is captured by 'vsv'}} - vsv.push_back(substr(std::string())); // expected-warning {{object whose reference is captured by 'vsv'}} - - MyVector<const std::string*> vp; - vp.push_back(getPointerLB(std::string())); // expected-warning {{object whose reference is captured by 'vp'}} - vp.push_back(getPointerLB(*getPointerLB(std::string()))); // expected-warning {{object whose reference is captured by 'vp'}} - vp.push_back(getPointerLB(local)); - vp.push_back(getPointerNoLB(std::string())); - - // User-defined [[gsl::Pointer]] - vsv.push_back(getMySV()); - vsv.push_back(getMySVNotP()); - - // Vector of user defined gsl::Pointer. - MyVector<MyStringView> vmysv; - vmysv.push_back(getMySV()); - vmysv.push_back(MyStringView{}); - vmysv.push_back(std::string_view{}); - vmysv.push_back(std::string{}); // expected-warning {{object whose reference is captured by 'vmysv'}} - vmysv.push_back(substr(std::string{})); // expected-warning {{object whose reference is captured by 'vmysv'}} - vmysv.push_back(getLB(substr(std::string{}))); // expected-warning {{object whose reference is captured by 'vmysv'}} - vmysv.push_back(strcopy(getLB(substr(std::string{})))); - - // With std::optional container. - std::optional<std::string_view> optional; - vsv.push_back(optional.value()); - vsv.push_back(getOptionalS().value()); // expected-warning {{object whose reference is captured by 'vsv'}} - vsv.push_back(getOptionalSV().value()); - vsv.push_back(getOptionalMySV().value()); - - // (maybe) FIXME: We may choose to diagnose the following case. - // This happens because 'MyStringViewNotPointer' is not marked as a [[gsl::Pointer]] but is derived from one. - vsv.push_back(getOptionalMySVNotP().value()); // expected-warning {{object whose reference is captured by 'vsv'}} -} -} // namespace lifetime_capture_by >From b56631a07d8cbd2f13ca038ef13f035fac764394 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <u...@google.com> Date: Mon, 18 Nov 2024 07:08:22 +0000 Subject: [PATCH 7/7] address comments --- a-84635e71.o.tmp | 0 a-9a215dcd.o.tmp | 0 a-d85fccc7.o.tmp | 0 a-f0f01220.o.tmp | 0 clang/include/clang/Basic/AttrDocs.td | 8 +- clang/lib/Sema/CheckExprLifetime.cpp | 3 +- clang/lib/Sema/SemaChecking.cpp | 43 +++++----- .../warn-lifetime-analysis-capture-by.cpp | 82 +++++++++++-------- 8 files changed, 72 insertions(+), 64 deletions(-) create mode 100644 a-84635e71.o.tmp create mode 100644 a-9a215dcd.o.tmp create mode 100644 a-d85fccc7.o.tmp create mode 100644 a-f0f01220.o.tmp diff --git a/a-84635e71.o.tmp b/a-84635e71.o.tmp new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/a-9a215dcd.o.tmp b/a-9a215dcd.o.tmp new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/a-d85fccc7.o.tmp b/a-d85fccc7.o.tmp new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/a-f0f01220.o.tmp b/a-f0f01220.o.tmp new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 933ea032d88ab8..1d3e514bf9a083 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3922,8 +3922,8 @@ def LifetimeCaptureByDocs : Documentation { let Category = DocCatFunction; let Content = [{ Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a function -parameter or implicit object parameter indicates that that objects that are referred to -by that parameter may also be referred to by the capturing entity ``X``. +parameter or implicit object parameter indicates that objects that are referred to by +that parameter may also be referred to by the capturing entity ``X``. By default: @@ -3932,7 +3932,7 @@ By default: - A ``std::initializer_list<T>`` is considered to refer to its underlying array. - Aggregates (arrays and simple ``struct``\s) are considered to refer to all objects that their transitive subobjects refer to. -- View types (types annotated with [[``gsl::Pointer()``]]) are considered to refer +- View types (types annotated with ``[[gsl::Pointer()]]``) are considered to refer to its pointee. This holds true even if the view type appears as a reference. For example, both ``std::string_view`` and ``const std::string_view &`` are considered to refer to a ``std::string``. @@ -3990,7 +3990,7 @@ The attribute supports specifying more than one capturing entities: s2.insert(a); } -Currently clang would diagnose when a temporary is used as an argument to a +Currently Clang would diagnose when a temporary is used as an argument to a parameter whose reference is captured. .. code-block:: c++ diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index 2c60ced42eb79b..4f3179f190a944 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -254,7 +254,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, LocalVisitor Visit); template <typename T> static bool isRecordWithAttr(QualType Type) { - if (auto *RD = Type.getNonReferenceType()->getAsCXXRecordDecl()) + if (auto *RD = Type->getAsCXXRecordDecl()) return RD->hasAttr<T>(); return false; } @@ -1437,6 +1437,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, // warnings or errors on inner temporaries within this one's initializer. return false; }; + bool HasReferenceBinding = Init->isGLValue(); llvm::SmallVector<IndirectLocalPathEntry, 8> Path; switch (LK) { diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 8df2d0acd352ab..135f9891b44c8b 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -3233,28 +3233,32 @@ void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl, void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction, const Expr *ThisArg, ArrayRef<const Expr *> Args) { - auto GetArgAt = [&](int Idx) -> Expr * { + if (!FD || Args.empty()) + return; + auto GetArgAt = [&](int Idx) -> const Expr * { if (Idx == LifetimeCaptureByAttr::GLOBAL || Idx == LifetimeCaptureByAttr::UNKNOWN) return nullptr; if (IsMemberFunction && Idx == 0) - return const_cast<Expr *>(ThisArg); - return const_cast<Expr *>(Args[Idx - int(IsMemberFunction)]); + return ThisArg; + return Args[Idx - IsMemberFunction]; }; - for (unsigned I = 0; I < FD->getNumParams(); ++I) { - auto *CapturedByAttr = - FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(); - if (!CapturedByAttr) - continue; - for (int CapturingParamIdx : CapturedByAttr->params()) { - Expr *Capturing = GetArgAt(CapturingParamIdx); - Expr *Captured = GetArgAt(I + IsMemberFunction); + auto HandleCaptureByAttr = [&](const LifetimeCaptureByAttr *Attr, + unsigned ArgIdx) { + if (!Attr) + return; + Expr *Captured = const_cast<Expr *>(GetArgAt(ArgIdx)); + for (int CapturingParamIdx : Attr->params()) { + Expr *Capturing = const_cast<Expr *>(GetArgAt(CapturingParamIdx)); CapturingEntity CE{Capturing}; // Ensure that 'Captured' outlives the 'Capturing' entity. checkExprLifetime(*this, CE, Captured); } - } - // Check when the 'this' object is captured. + }; + for (unsigned I = 0; I < FD->getNumParams(); ++I) + HandleCaptureByAttr(FD->getParamDecl(I)->getAttr<LifetimeCaptureByAttr>(), + I + IsMemberFunction); + // Check when the implicit object param is captured. if (IsMemberFunction) { TypeSourceInfo *TSI = FD->getTypeSourceInfo(); if (!TSI) @@ -3262,17 +3266,8 @@ void Sema::checkLifetimeCaptureBy(FunctionDecl *FD, bool IsMemberFunction, AttributedTypeLoc ATL; for (TypeLoc TL = TSI->getTypeLoc(); (ATL = TL.getAsAdjusted<AttributedTypeLoc>()); - TL = ATL.getModifiedLoc()) { - auto *CapturedByAttr = ATL.getAttrAs<LifetimeCaptureByAttr>(); - if (!CapturedByAttr) - continue; - Expr *Captured = GetArgAt(0); - for (int CapturingParamIdx : CapturedByAttr->params()) { - Expr *Capturing = GetArgAt(CapturingParamIdx); - CapturingEntity CE{Capturing}; - checkExprLifetime(*this, CE, Captured); - } - } + TL = ATL.getModifiedLoc()) + HandleCaptureByAttr(ATL.getAttrAs<LifetimeCaptureByAttr>(), 0); } } diff --git a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp index 069fce1f70b9e3..b89e1b8e77939b 100644 --- a/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp +++ b/clang/test/Sema/warn-lifetime-analysis-capture-by.cpp @@ -17,11 +17,11 @@ void noCaptureInt(int i [[clang::lifetime_capture_by(x)]], X &x); std::string_view substr(const std::string& s [[clang::lifetimebound]]); std::string_view strcopy(const std::string& s); -void captureSV(std::string_view s [[clang::lifetime_capture_by(x)]], X &x); -void captureRValSV(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x); -void noCaptureSV(std::string_view sv, X &x); -void captureS(const std::string &s [[clang::lifetime_capture_by(x)]], X &x); -void captureRValS(std::string &&s [[clang::lifetime_capture_by(x)]], X &x); +void captureStringView(std::string_view s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValStringView(std::string_view &&sv [[clang::lifetime_capture_by(x)]], X &x); +void noCaptureStringView(std::string_view sv, X &x); +void captureString(const std::string &s [[clang::lifetime_capture_by(x)]], X &x); +void captureRValString(std::string &&s [[clang::lifetime_capture_by(x)]], X &x); const std::string& getLB(const std::string &s [[clang::lifetimebound]]); const std::string& getLB(std::string_view sv [[clang::lifetimebound]]); @@ -38,8 +38,8 @@ void captureByGlobal(std::string_view s [[clang::lifetime_capture_by(global)]]); void captureByUnknown(std::string_view s [[clang::lifetime_capture_by(unknown)]]); void use() { - std::string_view local_sv; - std::string local_s; + std::string_view local_string_view; + std::string local_string; X x; // Capture an 'int'. int local; @@ -51,34 +51,34 @@ void use() { noCaptureInt(local, x); // Capture using std::string_view. - captureSV(local_sv, x); - captureSV(std::string(), // expected-warning {{object whose reference is captured by 'x'}} + captureStringView(local_string_view, x); + captureStringView(std::string(), // expected-warning {{object whose reference is captured by 'x'}} x); - captureSV(substr( + captureStringView(substr( std::string() // expected-warning {{object whose reference is captured by 'x'}} ), x); - captureSV(substr(local_s), x); - captureSV(strcopy(std::string()), x); - captureRValSV(std::move(local_sv), x); - captureRValSV(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} - captureRValSV(std::string_view{"abcd"}, x); - captureRValSV(substr(local_s), x); - captureRValSV(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} - captureRValSV(strcopy(std::string()), x); - noCaptureSV(local_sv, x); - noCaptureSV(std::string(), x); - noCaptureSV(substr(std::string()), x); + captureStringView(substr(local_string), x); + captureStringView(strcopy(std::string()), x); + captureRValStringView(std::move(local_string_view), x); + captureRValStringView(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValStringView(std::string_view{"abcd"}, x); + captureRValStringView(substr(local_string), x); + captureRValStringView(substr(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureRValStringView(strcopy(std::string()), x); + noCaptureStringView(local_string_view, x); + noCaptureStringView(std::string(), x); + noCaptureStringView(substr(std::string()), x); // Capture using std::string. - captureS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} - captureS(local_s, x); - captureRValS(std::move(local_s), x); - captureRValS(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureString(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} + captureString(local_string, x); + captureRValString(std::move(local_string), x); + captureRValString(std::string(), x); // expected-warning {{object whose reference is captured by 'x'}} // Capture with lifetimebound. - captureSV(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} - captureSV(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}} - captureSV(getLB(getLB( + captureStringView(getLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} + captureStringView(getLB(substr(std::string())), x); // expected-warning {{object whose reference is captured by 'x'}} + captureStringView(getLB(getLB( std::string() // expected-warning {{object whose reference is captured by 'x'}} )), x); capturePointer(getPointerLB(std::string()), x); // expected-warning {{object whose reference is captured by 'x'}} @@ -101,14 +101,23 @@ void use() { // capture by global. captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} captureByGlobal(substr(std::string())); // expected-warning {{captured}} - captureByGlobal(local_s); - captureByGlobal(local_sv); + captureByGlobal(local_string); + captureByGlobal(local_string_view); // // capture by unknown. captureByGlobal(std::string()); // expected-warning {{object whose reference is captured will be destroyed at the end of the full-expression}} captureByGlobal(substr(std::string())); // expected-warning {{captured}} - captureByGlobal(local_s); - captureByGlobal(local_sv); + captureByGlobal(local_string); + captureByGlobal(local_string_view); +} + +void captureDefaultArg(X &x, std::string_view s [[clang::lifetime_capture_by(x)]] = std::string()); +void useCaptureDefaultArg() { + X x; + captureDefaultArg(x); // FIXME: Diagnose temporary default arg. + captureDefaultArg(x, std::string("temp")); // expected-warning {{captured}} + std::string local; + captureDefaultArg(x, local); } template<typename T> struct IsPointerLikeTypeImpl : std::false_type {}; @@ -189,13 +198,16 @@ void container_of_pointers() { std::optional<std::string_view> optionalSV; vsv.push_back(optionalSV.value()); vsv.push_back(getOptionalS().value()); // expected-warning {{object whose reference is captured by 'vsv'}} - vsv.push_back(getOptionalSV().value()); - vsv.push_back(getOptionalMySV().value()); + + // FIXME: Following 2 cases are false positives: + vsv.push_back(getOptionalSV().value()); // expected-warning {{object whose reference}} + vsv.push_back(getOptionalMySV().value()); // expected-warning {{object whose reference}} // (maybe) FIXME: We may choose to diagnose the following case. // This happens because 'MyStringViewNotPointer' is not marked as a [[gsl::Pointer]] but is derived from one. vsv.push_back(getOptionalMySVNotP().value()); // expected-warning {{object whose reference is captured by 'vsv'}} } + namespace temporary_views { void capture1(std::string_view s [[clang::lifetime_capture_by(x)]], std::vector<std::string_view>& x); @@ -217,4 +229,4 @@ void test1() { capture3(std::string_view(), x3); capture3(std::string(), x3); // expected-warning {{captured by 'x3'}} } -} \ No newline at end of file +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits