Revert "[LifetimeSafety] Merge lifetimebound attribute on implicit 'this' across method redeclarations (#172146)"
This caused assertion failures, see comment on the PR: clang/lib/Sema/TypeLocBuilder.cpp:89: TypeLoc clang::TypeLocBuilder::pushImpl(QualType, size_t, unsigned int): Assertion `TLast == LastTy && "mismatch between last type and new type's inner type"' failed. > Followup on https://github.com/llvm/llvm-project/pull/107627 > Fixes https://github.com/llvm/llvm-project/issues/62072 > Fixes https://github.com/llvm/llvm-project/issues/172013 > Fixes https://github.com/llvm/llvm-project/issues/175391 > > This PR adds support for merging the `lifetimebound` attribute on the implicit `this` parameter when merging method declarations. Previously, if a method was declared with `lifetimebound` on its function type (which represents the implicit `this` parameter), this attribute would not be propagated to the method definition, causing lifetime safety warnings to be missed. > > The implementation adds helper functions to extract the `lifetimebound` attribute from a function type and to merge this attribute from an old method declaration to a new one when appropriate. This reverts commit ef90ba684d012790c86ac1b5e7c6b325abe78803.
This commit is contained in:
Hans Wennborg
parent
7db584562d
commit
a70f534abc
@ -10,7 +10,6 @@
|
||||
#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H
|
||||
#define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H
|
||||
|
||||
#include "clang/AST/Attr.h"
|
||||
#include "clang/AST/DeclCXX.h"
|
||||
|
||||
namespace clang ::lifetimes {
|
||||
@ -46,12 +45,6 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD);
|
||||
/// method or because it's a normal assignment operator.
|
||||
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD);
|
||||
|
||||
/// Check if a function has a lifetimebound attribute on its function type
|
||||
/// (which represents the implicit 'this' parameter for methods).
|
||||
/// Returns the attribute if found, nullptr otherwise.
|
||||
const LifetimeBoundAttr *
|
||||
getLifetimeBoundAttrFromFunctionType(const TypeSourceInfo &TSI);
|
||||
|
||||
// Returns true if the implicit object argument (this) of a method call should
|
||||
// be tracked for GSL lifetime analysis. This applies to STL methods that return
|
||||
// pointers or references that depend on the lifetime of the object, such as
|
||||
|
||||
@ -52,28 +52,23 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD) {
|
||||
CMD->getParamDecl(0)->hasAttr<clang::LifetimeBoundAttr>();
|
||||
}
|
||||
|
||||
const LifetimeBoundAttr *
|
||||
getLifetimeBoundAttrFromFunctionType(const TypeSourceInfo &TSI) {
|
||||
// Walk through the type layers looking for a lifetimebound attribute.
|
||||
TypeLoc TL = TSI.getTypeLoc();
|
||||
while (true) {
|
||||
auto ATL = TL.getAsAdjusted<AttributedTypeLoc>();
|
||||
if (!ATL)
|
||||
break;
|
||||
if (auto *LBAttr = ATL.getAttrAs<LifetimeBoundAttr>())
|
||||
return LBAttr;
|
||||
TL = ATL.getModifiedLoc();
|
||||
}
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
|
||||
FD = getDeclWithMergedLifetimeBoundAttrs(FD);
|
||||
const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
|
||||
if (!TSI)
|
||||
return false;
|
||||
return getLifetimeBoundAttrFromFunctionType(*TSI) != nullptr ||
|
||||
isNormalAssignmentOperator(FD);
|
||||
// Don't declare this variable in the second operand of the for-statement;
|
||||
// GCC miscompiles that by ending its lifetime before evaluating the
|
||||
// third operand. See gcc.gnu.org/PR86769.
|
||||
AttributedTypeLoc ATL;
|
||||
for (TypeLoc TL = TSI->getTypeLoc();
|
||||
(ATL = TL.getAsAdjusted<AttributedTypeLoc>());
|
||||
TL = ATL.getModifiedLoc()) {
|
||||
if (ATL.getAttrAs<clang::LifetimeBoundAttr>())
|
||||
return true;
|
||||
}
|
||||
|
||||
return isNormalAssignmentOperator(FD);
|
||||
}
|
||||
|
||||
bool isInStlNamespace(const Decl *D) {
|
||||
|
||||
@ -28,7 +28,6 @@
|
||||
#include "clang/AST/Randstruct.h"
|
||||
#include "clang/AST/StmtCXX.h"
|
||||
#include "clang/AST/Type.h"
|
||||
#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h"
|
||||
#include "clang/Basic/Builtins.h"
|
||||
#include "clang/Basic/DiagnosticComment.h"
|
||||
#include "clang/Basic/HLSLRuntime.h"
|
||||
@ -4471,35 +4470,6 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
|
||||
return true;
|
||||
}
|
||||
|
||||
/// Merge lifetimebound attribute on function type (implicit 'this')
|
||||
/// from Old to New method declaration.
|
||||
static void mergeLifetimeBoundAttrOnMethod(Sema &S, CXXMethodDecl *New,
|
||||
const CXXMethodDecl *Old) {
|
||||
const TypeSourceInfo *OldTSI = Old->getTypeSourceInfo();
|
||||
const TypeSourceInfo *NewTSI = New->getTypeSourceInfo();
|
||||
|
||||
if (!OldTSI || !NewTSI)
|
||||
return;
|
||||
|
||||
const LifetimeBoundAttr *OldLBAttr =
|
||||
lifetimes::getLifetimeBoundAttrFromFunctionType(*OldTSI);
|
||||
const LifetimeBoundAttr *NewLBAttr =
|
||||
lifetimes::getLifetimeBoundAttrFromFunctionType(*NewTSI);
|
||||
|
||||
// If Old has lifetimebound but New doesn't, add it to New.
|
||||
if (OldLBAttr && !NewLBAttr) {
|
||||
QualType NewMethodType = New->getType();
|
||||
QualType AttributedType =
|
||||
S.Context.getAttributedType(OldLBAttr, NewMethodType, NewMethodType);
|
||||
TypeLocBuilder TLB;
|
||||
TLB.pushFullCopy(NewTSI->getTypeLoc());
|
||||
AttributedTypeLoc TyLoc = TLB.push<AttributedTypeLoc>(AttributedType);
|
||||
TyLoc.setAttr(OldLBAttr);
|
||||
New->setType(AttributedType);
|
||||
New->setTypeSourceInfo(TLB.getTypeSourceInfo(S.Context, AttributedType));
|
||||
}
|
||||
}
|
||||
|
||||
bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
|
||||
Scope *S, bool MergeTypeWithOld) {
|
||||
// Merge the attributes
|
||||
@ -4516,16 +4486,12 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old,
|
||||
// Merge attributes from the parameters. These can mismatch with K&R
|
||||
// declarations.
|
||||
if (New->getNumParams() == Old->getNumParams())
|
||||
for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) {
|
||||
ParmVarDecl *NewParam = New->getParamDecl(i);
|
||||
ParmVarDecl *OldParam = Old->getParamDecl(i);
|
||||
mergeParamDeclAttributes(NewParam, OldParam, *this);
|
||||
mergeParamDeclTypes(NewParam, OldParam, *this);
|
||||
}
|
||||
|
||||
// Merge function type attributes (e.g., lifetimebound on implicit 'this').
|
||||
if (auto *NewMethod = dyn_cast<CXXMethodDecl>(New))
|
||||
mergeLifetimeBoundAttrOnMethod(*this, NewMethod, cast<CXXMethodDecl>(Old));
|
||||
for (unsigned i = 0, e = New->getNumParams(); i != e; ++i) {
|
||||
ParmVarDecl *NewParam = New->getParamDecl(i);
|
||||
ParmVarDecl *OldParam = Old->getParamDecl(i);
|
||||
mergeParamDeclAttributes(NewParam, OldParam, *this);
|
||||
mergeParamDeclTypes(NewParam, OldParam, *this);
|
||||
}
|
||||
|
||||
if (getLangOpts().CPlusPlus)
|
||||
return MergeCXXFunctionDecl(New, Old, S);
|
||||
|
||||
@ -876,141 +876,3 @@ const char* foo() {
|
||||
}
|
||||
|
||||
} // namespace GH127195
|
||||
|
||||
// Lifetimebound on definition vs declaration on implicit this param.
|
||||
namespace GH175391 {
|
||||
// Version A: Attribute on declaration only
|
||||
class StringA {
|
||||
public:
|
||||
const char* data() const [[clang::lifetimebound]]; // Declaration with attribute
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringA::data() const { // Definition WITHOUT attribute
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Version B: Attribute on definition only
|
||||
class StringB {
|
||||
public:
|
||||
const char* data() const; // No attribute
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringB::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Version C: Attribute on BOTH declaration and definition
|
||||
class StringC {
|
||||
public:
|
||||
const char* data() const [[clang::lifetimebound]];
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringC::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// TEMPLATED VERSIONS
|
||||
|
||||
// Template Version A: Attribute on declaration only
|
||||
template<typename T>
|
||||
class StringTemplateA {
|
||||
public:
|
||||
const T* data() const [[clang::lifetimebound]]; // Declaration with attribute
|
||||
private:
|
||||
T buffer[32];
|
||||
};
|
||||
template<typename T>
|
||||
inline const T* StringTemplateA<T>::data() const { // Definition WITHOUT attribute
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Template Version B: Attribute on definition only
|
||||
template<typename T>
|
||||
class StringTemplateB {
|
||||
public:
|
||||
const T* data() const; // No attribute
|
||||
private:
|
||||
T buffer[32];
|
||||
};
|
||||
template<typename T>
|
||||
inline const T* StringTemplateB<T>::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Template Version C: Attribute on BOTH declaration and definition
|
||||
template<typename T>
|
||||
class StringTemplateC {
|
||||
public:
|
||||
const T* data() const [[clang::lifetimebound]];
|
||||
private:
|
||||
T buffer[32];
|
||||
};
|
||||
template<typename T>
|
||||
inline const T* StringTemplateC<T>::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// TEMPLATE SPECIALIZATION VERSIONS
|
||||
|
||||
// Template predeclarations for specializations
|
||||
template<typename T> class StringTemplateSpecA;
|
||||
template<typename T> class StringTemplateSpecB;
|
||||
template<typename T> class StringTemplateSpecC;
|
||||
|
||||
// Template Specialization Version A: Attribute on declaration only - <char> specialization
|
||||
template<>
|
||||
class StringTemplateSpecA<char> {
|
||||
public:
|
||||
const char* data() const [[clang::lifetimebound]]; // Declaration with attribute
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringTemplateSpecA<char>::data() const { // Definition WITHOUT attribute
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Template Specialization Version B: Attribute on definition only - <char> specialization
|
||||
template<>
|
||||
class StringTemplateSpecB<char> {
|
||||
public:
|
||||
const char* data() const; // No attribute
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringTemplateSpecB<char>::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
// Template Specialization Version C: Attribute on BOTH declaration and definition - <char> specialization
|
||||
template<>
|
||||
class StringTemplateSpecC<char> {
|
||||
public:
|
||||
const char* data() const [[clang::lifetimebound]];
|
||||
private:
|
||||
char buffer[32] = "hello";
|
||||
};
|
||||
inline const char* StringTemplateSpecC<char>::data() const [[clang::lifetimebound]] {
|
||||
return buffer;
|
||||
}
|
||||
|
||||
void test() {
|
||||
// Non-templated tests
|
||||
const auto ptrA = StringA().data(); // Declaration-only attribute // expected-warning {{temporary whose address is used}}
|
||||
const auto ptrB = StringB().data(); // Definition-only attribute // expected-warning {{temporary whose address is used}}
|
||||
const auto ptrC = StringC().data(); // Both have attribute // expected-warning {{temporary whose address is used}}
|
||||
|
||||
// Templated tests (generic templates)
|
||||
const auto ptrTA = StringTemplateA<char>().data(); // Declaration-only attribute // expected-warning {{temporary whose address is used}}
|
||||
// FIXME: Definition is not instantiated until the end of TU. The attribute is not merged when this call is processed.
|
||||
const auto ptrTB = StringTemplateB<char>().data(); // Definition-only attribute
|
||||
const auto ptrTC = StringTemplateC<char>().data(); // Both have attribute // expected-warning {{temporary whose address is used}}
|
||||
|
||||
// Template specialization tests
|
||||
const auto ptrTSA = StringTemplateSpecA<char>().data(); // Declaration-only attribute // expected-warning {{temporary whose address is used}}
|
||||
const auto ptrTSB = StringTemplateSpecB<char>().data(); // Definition-only attribute // expected-warning {{temporary whose address is used}}
|
||||
const auto ptrTSC = StringTemplateSpecC<char>().data(); // Both have attribute // expected-warning {{temporary whose address is used}}
|
||||
}
|
||||
} // namespace GH175391
|
||||
|
||||
@ -1392,27 +1392,3 @@ void add(int c, MyObj* node) {
|
||||
arr[4] = node;
|
||||
}
|
||||
} // namespace CppCoverage
|
||||
|
||||
// Implicit this annotations with redecls.
|
||||
namespace GH172013 {
|
||||
// https://github.com/llvm/llvm-project/issues/62072
|
||||
// https://github.com/llvm/llvm-project/issues/172013
|
||||
struct S {
|
||||
View x() const [[clang::lifetimebound]];
|
||||
MyObj i;
|
||||
};
|
||||
|
||||
View S::x() const { return i; }
|
||||
|
||||
void bar() {
|
||||
View x;
|
||||
{
|
||||
S s;
|
||||
x = s.x(); // expected-warning {{object whose reference is captured does not live long enough}}
|
||||
View y = S().x(); // expected-warning {{object whose reference is captured does not live long enough}} \
|
||||
expected-note {{destroyed here}}
|
||||
(void)y; // expected-note {{later used here}}
|
||||
} // expected-note {{destroyed here}}
|
||||
(void)x; // expected-note {{used here}}
|
||||
}
|
||||
}
|
||||
|
||||
@ -75,27 +75,6 @@ namespace usage_ok {
|
||||
r = A(1); // expected-warning {{object backing the pointer 'r' will be destroyed at the end of the full-expression}}
|
||||
}
|
||||
|
||||
// Test that lifetimebound on implicit 'this' is propagated across redeclarations
|
||||
struct B {
|
||||
int *method() [[clang::lifetimebound]];
|
||||
int i;
|
||||
};
|
||||
int *B::method() { return &i; }
|
||||
|
||||
// Test that lifetimebound on implicit 'this' is propagated across redeclarations
|
||||
struct C {
|
||||
int *method();
|
||||
int i;
|
||||
};
|
||||
int *C::method() [[clang::lifetimebound]] { return &i; }
|
||||
|
||||
void test_lifetimebound_on_implicit_this() {
|
||||
int *t = B().method(); // expected-warning {{temporary whose address is used as value of local variable 't' will be destroyed at the end of the full-expression}}
|
||||
t = {B().method()}; // expected-warning {{object backing the pointer 't' will be destroyed at the end of the full-expression}}
|
||||
t = C().method(); // expected-warning {{object backing the pointer 't' will be destroyed at the end of the full-expression}}
|
||||
t = {C().method()}; // expected-warning {{object backing the pointer 't' will be destroyed at the end of the full-expression}}
|
||||
}
|
||||
|
||||
struct FieldCheck {
|
||||
struct Set {
|
||||
int a;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user