Skip to content

[clang] Add lifetimebound attr to std::span/std::string_view constructor #103716

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ Attribute Changes in Clang
- ``[[clang::lifetimebound]]`` is now explicitly disallowed on explicit object member functions
where they were previously silently ignored.

- Clang now automatically adds ``[[clang::lifetimebound]]`` to the parameters of
``std::span, std::string_view`` constructors, this enables Clang to capture
more cases where the returned reference outlives the object.
(#GH100567)

Improvements to Clang's diagnostics
-----------------------------------

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -1806,6 +1806,9 @@ class Sema final : public SemaBase {
/// Add [[gsl::Owner]] and [[gsl::Pointer]] attributes for std:: types.
void inferGslOwnerPointerAttribute(CXXRecordDecl *Record);

/// Add [[clang:::lifetimebound]] attr for std:: functions and methods.
void inferLifetimeBoundAttribute(FunctionDecl *FD);

/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);

Expand Down
53 changes: 53 additions & 0 deletions clang/lib/Sema/SemaAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,59 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
inferGslPointerAttribute(Record, Record);
}

void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
if (FD->getNumParams() == 0)
return;

if (unsigned BuiltinID = FD->getBuiltinID()) {
// Add lifetime attribute to std::move, std::fowrard et al.
switch (BuiltinID) {
case Builtin::BIaddressof:
case Builtin::BI__addressof:
case Builtin::BI__builtin_addressof:
case Builtin::BIas_const:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I start to have the feeling we are writing code like this over and over again.
Some examples:

case Builtin::BIas_const:

case Builtin::BIas_const:

But there are more, and I always wonder if they can get out of sync. I wonder if this is time to introduce some helper functions that can be reused.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The latter two cases are identical, we could potentially abstract a common function.)

I think this should be handled on a case-by-case basis, depending on the specific logic involved. It seems fine to me have this small amount of duplication.

case Builtin::BIforward:
case Builtin::BIforward_like:
case Builtin::BImove:
case Builtin::BImove_if_noexcept:
if (ParmVarDecl *P = FD->getParamDecl(0u);
!P->hasAttr<LifetimeBoundAttr>())
P->addAttr(
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
break;
default:
break;
}
return;
}
if (auto *CMD = dyn_cast<CXXMethodDecl>(FD)) {
const auto *CRD = CMD->getParent();
if (!CRD->isInStdNamespace() || !CRD->getIdentifier())
return;

if (isa<CXXConstructorDecl>(CMD)) {
auto *Param = CMD->getParamDecl(0);
if (Param->hasAttr<LifetimeBoundAttr>())
return;
if (CRD->getName() == "basic_string_view" &&
Param->getType()->isPointerType()) {
// construct from a char array pointed by a pointer.
// basic_string_view(const CharT* s);
// basic_string_view(const CharT* s, size_type count);
Param->addAttr(
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
} else if (CRD->getName() == "span") {
// construct from a reference of array.
// span(std::type_identity_t<element_type> (&arr)[N]);
const auto *LRT = Param->getType()->getAs<LValueReferenceType>();
if (LRT && LRT->getPointeeType().IgnoreParens()->isArrayType())
Param->addAttr(
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
}
}
}
}

void Sema::inferNullableClassAttribute(CXXRecordDecl *CRD) {
static const llvm::StringSet<> Nullable{
"auto_ptr", "shared_ptr", "unique_ptr", "exception_ptr",
Expand Down
20 changes: 1 addition & 19 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16608,27 +16608,9 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
default:
break;
}

// Add lifetime attribute to std::move, std::fowrard et al.
switch (BuiltinID) {
case Builtin::BIaddressof:
case Builtin::BI__addressof:
case Builtin::BI__builtin_addressof:
case Builtin::BIas_const:
case Builtin::BIforward:
case Builtin::BIforward_like:
case Builtin::BImove:
case Builtin::BImove_if_noexcept:
if (ParmVarDecl *P = FD->getParamDecl(0u);
!P->hasAttr<LifetimeBoundAttr>())
P->addAttr(
LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
break;
default:
break;
}
}

inferLifetimeBoundAttribute(FD);
AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(FD);

// If C++ exceptions are enabled but we are told extern "C" functions cannot
Expand Down
21 changes: 21 additions & 0 deletions clang/test/SemaCXX/attr-lifetimebound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,16 @@ template <class T> T *addressof(T &arg) {
&const_cast<char &>(reinterpret_cast<const volatile char &>(arg)));
}

template<typename T>
struct basic_string_view {
basic_string_view(const T *);
};

template <class T> struct span {
template<size_t _ArrayExtent>
span(const T (&__arr)[_ArrayExtent]) noexcept;
};

} // namespace foo
} // namespace std

Expand Down Expand Up @@ -266,3 +276,14 @@ namespace move_forward_et_al_examples {
S *AddressOfOk = std::addressof(X);
} // namespace move_forward_et_al_examples

namespace ctor_cases {
std::basic_string_view<char> test1() {
char abc[10];
return abc; // expected-warning {{address of stack memory associated with local variable}}
}

std::span<int> test2() {
int abc[10];
return abc; // expected-warning {{address of stack memory associated with local variable}}
}
} // namespace ctor_cases
Loading