Reapply "[clang] Introduce [[clang::lifetime_capture_by(X)]] (#115823)

Fix compile time regression and memory leak

In the previous change, we saw:
- Memory leak: https://lab.llvm.org/buildbot/#/builders/169/builds/5193
- 0.5% Compile time regression
[link](https://llvm-compile-time-tracker.com/compare.php?from=4a68e4cbd2423dcacada8162ab7c4bb8d7f7e2cf&to=8c4331c1abeb33eabf3cdbefa7f2b6e0540e7f4f&stat=instructions:u)

For compile time regression, we make the Param->Idx `StringMap` for
**all** functions. This `StringMap` is expensive and should not be
computed when none of the params are annotated with
`[[clang::lifetime_capture_by(X)]]`.

For the memory leak, the small vectors used in Attribute are not
destroyed because the attributes are allocated through ASTContext's
allocator. We therefore need a raw array in this case.
This commit is contained in:
Utkarsh Saxena 2024-11-13 11:07:20 +01:00 committed by GitHub
parent 2a1586dfb5
commit 3e20bae827
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 323 additions and 0 deletions

View File

@ -450,6 +450,9 @@ Attribute Changes in Clang
- Fix a bug where clang doesn't automatically apply the ``[[gsl::Owner]]`` or
``[[gsl::Pointer]]`` to STL explicit template specialization decls. (#GH109442)
- Clang now supports ``[[clang::lifetime_capture_by(X)]]``. Similar to lifetimebound, this can be
used to specify when a reference to a function parameter is captured by another capturing entity ``X``.
Improvements to Clang's diagnostics
-----------------------------------

View File

@ -1889,6 +1889,37 @@ def LifetimeBound : DeclOrTypeAttr {
let SimpleHandler = 1;
}
def LifetimeCaptureBy : DeclOrTypeAttr {
let Spellings = [Clang<"lifetime_capture_by", 0>];
let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;
let Args = [VariadicParamOrParamIdxArgument<"Params">];
let Documentation = [LifetimeCaptureByDocs];
let AdditionalMembers = [{
private:
ArrayRef<IdentifierInfo*> ArgIdents;
ArrayRef<SourceLocation> ArgLocs;
public:
static constexpr int THIS = 0;
static constexpr int INVALID = -1;
static constexpr int UNKNOWN = -2;
static constexpr int GLOBAL = -3;
void setArgs(ArrayRef<IdentifierInfo*> Idents, ArrayRef<SourceLocation> Locs) {
assert(Idents.size() == params_Size);
assert(Locs.size() == params_Size);
ArgIdents = Idents;
ArgLocs = Locs;
}
auto getArgIdents() const { return ArgIdents; }
auto getArgLocs() const { return ArgLocs; }
void setParamIdx(size_t Idx, int Val) {
assert(Idx < params_Size);
params_[Idx] = Val;
}
}];
}
def TrivialABI : InheritableAttr {
// This attribute does not have a C [[]] spelling because it requires the
// CPlusPlus language option.

View File

@ -3918,6 +3918,75 @@ have their lifetimes extended.
}];
}
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``.
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.
The capturing entity ``X`` can be one of the following:
- Another (named) function parameter.
.. code-block:: c++
void addToSet(std::string_view a [[clang::lifetime_capture_by(s)]], std::set<std::string_view>& s) {
s.insert(a);
}
- ``this`` (in case of member functions).
.. code-block:: c++
class S {
void addToSet(std::string_view a [[clang::lifetime_capture_by(this)]]) {
s.insert(a);
}
std::set<std::string_view> s;
};
- 'global', 'unknown' (without quotes).
.. code-block:: c++
std::set<std::string_view> s;
void addToSet(std::string_view a [[clang::lifetime_capture_by(global)]]) {
s.insert(a);
}
void addSomewhere(std::string_view a [[clang::lifetime_capture_by(unknown)]]);
The attribute can be applied to the implicit ``this`` parameter of a member
function by writing the attribute after the function type:
.. code-block:: c++
struct S {
const char *data(std::set<S*>& s) [[clang::lifetime_capture_by(s)]] {
s.insert(this);
}
};
The attribute supports specifying more than one capturing entities:
.. code-block:: c++
void addToSets(std::string_view a [[clang::lifetime_capture_by(s1, s2)]],
std::set<std::string_view>& s1,
std::set<std::string_view>& s2) {
s1.insert(a);
s2.insert(a);
}
.. _`lifetimebound`: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
}];
}
def TrivialABIDocs : Documentation {
let Category = DocCatDecl;
let Content = [{

View File

@ -3383,6 +3383,20 @@ def err_callback_callee_is_variadic : Error<
"'callback' attribute callee may not be variadic">;
def err_callback_implicit_this_not_available : Error<
"'callback' argument at position %0 references unavailable implicit 'this'">;
def err_capture_by_attribute_multiple : Error<
"multiple 'lifetime_capture' attributes specified">;
def err_capture_by_attribute_no_entity : Error<
"'lifetime_capture_by' attribute specifies no capturing entity">;
def err_capture_by_implicit_this_not_available : Error<
"'lifetime_capture_by' argument references unavailable implicit 'this'">;
def err_capture_by_attribute_argument_unknown : Error<
"'lifetime_capture_by' attribute argument %0 is not a known function parameter"
"; must be a function parameter, 'this', 'global' or 'unknown'">;
def err_capture_by_references_itself : Error<"'lifetime_capture_by' argument references itself">;
def err_capture_by_param_uses_reserved_name : Error<
"parameter cannot be named '%select{global|unknown}0' while using 'lifetime_capture_by(%select{global|unknown}0)'">;
def err_init_method_bad_return_type : Error<
"init methods must return an object pointer type, not %0">;
def err_attribute_invalid_size : Error<

View File

@ -1760,6 +1760,14 @@ public:
/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);
LifetimeCaptureByAttr *ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
StringRef ParamName);
// Processes the argument 'X' in [[clang::lifetime_capture_by(X)]]. Since 'X'
// can be the name of a function parameter, we need to parse the function
// declaration and rest of the parameters before processesing 'X'. Therefore
// do this lazily instead of processing while parsing the annotation itself.
void LazyProcessLifetimeCaptureByParams(FunctionDecl *FD);
/// Add _Nullable attributes for std:: types.
void inferNullableClassAttribute(CXXRecordDecl *CRD);

View File

@ -25,6 +25,7 @@
#include "clang/AST/TextNodeDumper.h"
#include "clang/AST/Type.h"
#include "clang/Basic/AddressSpaces.h"
#include "clang/Basic/AttrKinds.h"
#include "clang/Basic/ExceptionSpecificationType.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
@ -1909,6 +1910,14 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
OS << " [[clang::lifetimebound]]";
return;
}
if (T->getAttrKind() == attr::LifetimeCaptureBy) {
OS << " [[clang::lifetime_capture_by(";
if (auto *attr = dyn_cast_or_null<LifetimeCaptureByAttr>(T->getAttr()))
llvm::interleaveComma(attr->getArgIdents(), OS,
[&](auto it) { OS << it->getName(); });
OS << ")]]";
return;
}
// The printing of the address_space attribute is handled by the qualifier
// since it is still stored in the qualifier. Return early to prevent printing
@ -1976,6 +1985,7 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
case attr::SizedBy:
case attr::SizedByOrNull:
case attr::LifetimeBound:
case attr::LifetimeCaptureBy:
case attr::TypeNonNull:
case attr::TypeNullable:
case attr::TypeNullableResult:

View File

@ -16711,6 +16711,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
}
}
LazyProcessLifetimeCaptureByParams(FD);
inferLifetimeBoundAttribute(FD);
AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(FD);

View File

@ -14,6 +14,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@ -64,6 +65,7 @@
#include "llvm/ADT/StringExtras.h"
#include "llvm/Demangle/Demangle.h"
#include "llvm/IR/Assumptions.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/MC/MCSectionMachO.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MathExtras.h"
@ -3867,6 +3869,119 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
S.Context, AL, EncodingIndices.data(), EncodingIndices.size()));
}
LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
StringRef ParamName) {
// Atleast one capture by is required.
if (AL.getNumArgs() == 0) {
Diag(AL.getLoc(), diag::err_capture_by_attribute_no_entity)
<< AL.getRange();
return nullptr;
}
unsigned N = AL.getNumArgs();
auto ParamIdents =
MutableArrayRef<IdentifierInfo *>(new (Context) IdentifierInfo *[N], N);
auto ParamLocs =
MutableArrayRef<SourceLocation>(new (Context) SourceLocation[N], N);
bool IsValid = true;
for (unsigned I = 0; I < N; ++I) {
if (AL.isArgExpr(I)) {
Expr *E = AL.getArgAsExpr(I);
Diag(E->getExprLoc(), diag::err_capture_by_attribute_argument_unknown)
<< E << E->getExprLoc();
IsValid = false;
continue;
}
assert(AL.isArgIdent(I));
IdentifierLoc *IdLoc = AL.getArgAsIdent(I);
if (IdLoc->Ident->getName() == ParamName) {
Diag(IdLoc->Loc, diag::err_capture_by_references_itself) << IdLoc->Loc;
IsValid = false;
continue;
}
ParamIdents[I] = IdLoc->Ident;
ParamLocs[I] = IdLoc->Loc;
}
if (!IsValid)
return nullptr;
SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::INVALID);
auto *CapturedBy =
LifetimeCaptureByAttr::Create(Context, FakeParamIndices.data(), N, AL);
CapturedBy->setArgs(ParamIdents, ParamLocs);
return CapturedBy;
}
static void handleLifetimeCaptureByAttr(Sema &S, Decl *D,
const ParsedAttr &AL) {
// Do not allow multiple attributes.
if (D->hasAttr<LifetimeCaptureByAttr>()) {
S.Diag(AL.getLoc(), diag::err_capture_by_attribute_multiple)
<< AL.getRange();
return;
}
auto *PVD = dyn_cast<ParmVarDecl>(D);
assert(PVD);
auto *CaptureByAttr = S.ParseLifetimeCaptureByAttr(AL, PVD->getName());
if (CaptureByAttr)
D->addAttr(CaptureByAttr);
}
void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
bool HasImplicitThisParam = isInstanceMethod(FD);
SmallVector<LifetimeCaptureByAttr *, 1> Attrs;
for (ParmVarDecl *PVD : FD->parameters())
if (auto *A = PVD->getAttr<LifetimeCaptureByAttr>())
Attrs.push_back(A);
if (HasImplicitThisParam) {
TypeSourceInfo *TSI = FD->getTypeSourceInfo();
if (!TSI)
return;
AttributedTypeLoc ATL;
for (TypeLoc TL = TSI->getTypeLoc();
(ATL = TL.getAsAdjusted<AttributedTypeLoc>());
TL = ATL.getModifiedLoc()) {
if (auto *A = ATL.getAttrAs<LifetimeCaptureByAttr>())
Attrs.push_back(const_cast<LifetimeCaptureByAttr *>(A));
}
}
if (Attrs.empty())
return;
llvm::StringMap<int> NameIdxMapping = {
{"global", LifetimeCaptureByAttr::GLOBAL},
{"unknown", LifetimeCaptureByAttr::UNKNOWN}};
int Idx = 0;
if (HasImplicitThisParam) {
NameIdxMapping["this"] = 0;
Idx++;
}
for (const ParmVarDecl *PVD : FD->parameters())
NameIdxMapping[PVD->getName()] = Idx++;
auto DisallowReservedParams = [&](StringRef Reserved) {
for (const ParmVarDecl *PVD : FD->parameters())
if (PVD->getName() == Reserved)
Diag(PVD->getLocation(), diag::err_capture_by_param_uses_reserved_name)
<< (PVD->getName() == "unknown");
};
for (auto *CapturedBy : Attrs) {
const auto &Entities = CapturedBy->getArgIdents();
for (size_t I = 0; I < Entities.size(); ++I) {
StringRef Name = Entities[I]->getName();
auto It = NameIdxMapping.find(Name);
if (It == NameIdxMapping.end()) {
auto Loc = CapturedBy->getArgLocs()[I];
if (!HasImplicitThisParam && Name == "this")
Diag(Loc, diag::err_capture_by_implicit_this_not_available) << Loc;
else
Diag(Loc, diag::err_capture_by_attribute_argument_unknown)
<< Entities[I] << Loc;
continue;
}
if (Name == "unknown" || Name == "global")
DisallowReservedParams(Name);
CapturedBy->setParamIdx(I, It->second);
}
}
}
static bool isFunctionLike(const Type &T) {
// Check for explicit function types.
// 'called_once' is only supported in Objective-C and it has
@ -6644,6 +6759,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_Callback:
handleCallbackAttr(S, D, AL);
break;
case ParsedAttr::AT_LifetimeCaptureBy:
handleLifetimeCaptureByAttr(S, D, AL);
break;
case ParsedAttr::AT_CalledOnce:
handleCalledOnceAttr(S, D, AL);
break;

View File

@ -8609,6 +8609,15 @@ static void HandleLifetimeBoundAttr(TypeProcessingState &State,
}
}
static void HandleLifetimeCaptureByAttr(TypeProcessingState &State,
QualType &CurType, ParsedAttr &PA) {
if (State.getDeclarator().isDeclarationOfFunction()) {
auto *Attr = State.getSema().ParseLifetimeCaptureByAttr(PA, "this");
if (Attr)
CurType = State.getAttributedType(Attr, CurType, CurType);
}
}
static void HandleHLSLParamModifierAttr(TypeProcessingState &State,
QualType &CurType,
const ParsedAttr &Attr, Sema &S) {
@ -8770,6 +8779,10 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
if (TAL == TAL_DeclChunk)
HandleLifetimeBoundAttr(state, type, attr);
break;
case ParsedAttr::AT_LifetimeCaptureBy:
if (TAL == TAL_DeclChunk)
HandleLifetimeCaptureByAttr(state, type, attr);
break;
case ParsedAttr::AT_NoDeref: {
// FIXME: `noderef` currently doesn't work correctly in [[]] syntax.

View File

@ -0,0 +1,9 @@
// RUN: %clang_cc1 %s -ast-dump | FileCheck %s
// Verify that we print the [[clang::lifetime_capture_by(X)]] attribute.
struct S {
void foo(int &a, int &b) [[clang::lifetime_capture_by(a, b, global)]];
};
// CHECK: CXXMethodDecl {{.*}}clang::lifetime_capture_by(a, b, global)

View File

@ -0,0 +1,47 @@
// RUN: %clang_cc1 -std=c++23 -verify %s
struct S {
const int *x;
void captureInt(const int&x [[clang::lifetime_capture_by(this)]]) { this->x = &x; }
};
///////////////////////////
// Test for valid usages.
///////////////////////////
[[clang::lifetime_capture_by(unknown)]] // expected-error {{'lifetime_capture_by' attribute only applies to parameters and implicit object parameters}}
void nonMember(
const int &x1 [[clang::lifetime_capture_by(s, t)]],
S &s,
S &t,
const int &x2 [[clang::lifetime_capture_by(12345 + 12)]], // expected-error {{'lifetime_capture_by' attribute argument 12345 + 12 is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
const int &x3 [[clang::lifetime_capture_by(abcdefgh)]], // expected-error {{'lifetime_capture_by' attribute argument 'abcdefgh' is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
const int &x4 [[clang::lifetime_capture_by("abcdefgh")]], // expected-error {{'lifetime_capture_by' attribute argument "abcdefgh" is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
const int &x5 [[clang::lifetime_capture_by(this)]], // expected-error {{'lifetime_capture_by' argument references unavailable implicit 'this'}}
const int &x6 [[clang::lifetime_capture_by()]], // expected-error {{'lifetime_capture_by' attribute specifies no capturing entity}}
const int& x7 [[clang::lifetime_capture_by(u,
x7)]], // expected-error {{'lifetime_capture_by' argument references itself}}
const int &x8 [[clang::lifetime_capture_by(global)]],
const int &x9 [[clang::lifetime_capture_by(unknown)]],
const int &test_memory_leak[[clang::lifetime_capture_by(x1,x2, x3, x4, x5, x6, x7, x8, x9)]],
const S& u
)
{
s.captureInt(x1);
}
void unknown_param_name(const int& unknown, // expected-error {{parameter cannot be named 'unknown' while using 'lifetime_capture_by(unknown)'}}
const int& s [[clang::lifetime_capture_by(unknown)]]);
void global_param_name(const int& global, // expected-error {{parameter cannot be named 'global' while using 'lifetime_capture_by(global)'}}
const int& s [[clang::lifetime_capture_by(global)]]);
struct T {
void member(
const int &x [[clang::lifetime_capture_by(s)]],
S &s,
S &t,
const int &y [[clang::lifetime_capture_by(s)]],
const int &z [[clang::lifetime_capture_by(this, x, y)]],
const int &u [[clang::lifetime_capture_by(global, unknown, x, s)]])
{
s.captureInt(x);
}
};