https://github.com/AaronBallman updated
https://github.com/llvm/llvm-project/pull/135013
>From 99190952174f334642ec237596969f9b0c846411 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Wed, 9 Apr 2025 09:51:49 -0400
Subject: [PATCH 1/3] Fix crash with align_value diagnostic reporting
We were passing the address of a local variable to a call to Diag()
which then tried to use the object after its lifetime ended, resulting
in crashes. We no longer pass the temporary object any longer.
Fixes #26612
---
clang/docs/ReleaseNotes.rst | 4 ++++
clang/lib/Sema/SemaDeclAttr.cpp | 3 +--
clang/test/SemaCXX/align_value.cpp | 14 ++++++++++++++
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5b702b56038f7..cd16641c25ed8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -386,6 +386,10 @@ Bug Fixes to Attribute Support
or too few attribute argument indicies for the specified callback function.
(#GH47451)
+- No longer crashing on ``__attribute__((align_value(N)))`` during template
+ instantiation when the function parameter type is not a pointer or reference.
+ (#GH26612)
+
Bug Fixes to C++ Support
^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d76afe9d6464d..b3c0367ed51b2 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4383,7 +4383,6 @@ static void handleAlignValueAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
}
void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) {
- AlignValueAttr TmpAttr(Context, CI, E);
SourceLocation AttrLoc = CI.getLoc();
QualType T;
@@ -4397,7 +4396,7 @@ void Sema::AddAlignValueAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E) {
if (!T->isDependentType() && !T->isAnyPointerType() &&
!T->isReferenceType() && !T->isMemberPointerType()) {
Diag(AttrLoc, diag::warn_attribute_pointer_or_reference_only)
- << &TmpAttr << T << D->getSourceRange();
+ << CI << T << D->getSourceRange();
return;
}
diff --git a/clang/test/SemaCXX/align_value.cpp
b/clang/test/SemaCXX/align_value.cpp
index 519868201f994..ad89791902e7f 100644
--- a/clang/test/SemaCXX/align_value.cpp
+++ b/clang/test/SemaCXX/align_value.cpp
@@ -24,3 +24,17 @@ struct nope {
// expected-note@+1 {{in instantiation of template class 'nope<long double,
4>' requested here}}
nope<long double, 4> y2;
+namespace GH26612 {
+// This used to crash while issuing the diagnostic about only applying to a
+// pointer or reference type.
+// FIXME: it would be ideal to only diagnose once rather than twice. We get one
+// diagnostic from explicit template arguments and another one for deduced
+// template arguments, which seems silly.
+template <class T>
+void f(T __attribute__((align_value(4))) x) {} // expected-warning 2
{{'align_value' attribute only applies to a pointer or reference ('int' is
invalid)}}
+
+void foo() {
+ f<int>(0); // expected-note {{while substituting explicitly-specified
template arguments into function template 'f'}} \
+ expected-note {{while substituting deduced template arguments
into function template 'f' [with T = int]}}
+}
+} // namespace GH26612
>From beff38c9e0399277c4d0b8e67b2c4c8574353cfe Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Wed, 9 Apr 2025 10:09:55 -0400
Subject: [PATCH 2/3] Fix formatting; NFC
---
clang/lib/Sema/SemaDeclAttr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index b3c0367ed51b2..62a8e0f742bc6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4396,7 +4396,7 @@ void Sema::AddAlignValueAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E) {
if (!T->isDependentType() && !T->isAnyPointerType() &&
!T->isReferenceType() && !T->isMemberPointerType()) {
Diag(AttrLoc, diag::warn_attribute_pointer_or_reference_only)
- << CI << T << D->getSourceRange();
+ << CI << T << D->getSourceRange();
return;
}
>From c39150fa0308bca58d724282b0b39c1dfaa9c5aa Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aa...@aaronballman.com>
Date: Wed, 9 Apr 2025 10:58:22 -0400
Subject: [PATCH 3/3] Hit other attributes based on review feedback
---
clang/include/clang/Sema/Sema.h | 2 +-
clang/lib/Sema/SemaDeclAttr.cpp | 28 ++++++++-----------
clang/test/SemaCXX/alloc-align-attr.cpp | 12 ++++++++
.../SemaCXX/builtin-assume-aligned-tmpl.cpp | 11 ++++++++
4 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 7bd77d33a1f3d..917f49bd9bee3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4510,7 +4510,7 @@ class Sema final : public SemaBase {
getAttrLoc(const AttrInfo &AL) {
return AL.getLocation();
}
- SourceLocation getAttrLoc(const ParsedAttr &AL);
+ SourceLocation getAttrLoc(const AttributeCommonInfo &CI);
/// If Expr is a valid integer constant, get the value of the integer
/// expression and return success or failure. May output an error.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 62a8e0f742bc6..20ea38b7e05db 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -86,7 +86,9 @@ static unsigned getNumAttributeArgs(const ParsedAttr &AL) {
return AL.getNumArgs() + AL.hasParsedType();
}
-SourceLocation Sema::getAttrLoc(const ParsedAttr &AL) { return AL.getLoc(); }
+SourceLocation Sema::getAttrLoc(const AttributeCommonInfo &CI) {
+ return CI.getLoc();
+}
/// Wrapper around checkUInt32Argument, with an extra check to be sure
/// that the result will fit into a regular (signed) int. All args have the
same
@@ -1415,13 +1417,11 @@ void Sema::AddAssumeAlignedAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E,
Expr *OE) {
QualType ResultType = getFunctionOrMethodResultType(D);
SourceRange SR = getFunctionOrMethodResultSourceRange(D);
-
- AssumeAlignedAttr TmpAttr(Context, CI, E, OE);
- SourceLocation AttrLoc = TmpAttr.getLocation();
+ SourceLocation AttrLoc = CI.getLoc();
if (!isValidPointerAttrType(ResultType, /* RefOkay */ true)) {
Diag(AttrLoc, diag::warn_attribute_return_pointers_refs_only)
- << &TmpAttr << TmpAttr.getRange() << SR;
+ << CI << CI.getRange() << SR;
return;
}
@@ -1430,12 +1430,10 @@ void Sema::AddAssumeAlignedAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E,
if (!(I = E->getIntegerConstantExpr(Context))) {
if (OE)
Diag(AttrLoc, diag::err_attribute_argument_n_type)
- << &TmpAttr << 1 << AANT_ArgumentIntegerConstant
- << E->getSourceRange();
+ << CI << 1 << AANT_ArgumentIntegerConstant << E->getSourceRange();
else
Diag(AttrLoc, diag::err_attribute_argument_type)
- << &TmpAttr << AANT_ArgumentIntegerConstant
- << E->getSourceRange();
+ << CI << AANT_ArgumentIntegerConstant << E->getSourceRange();
return;
}
@@ -1452,8 +1450,7 @@ void Sema::AddAssumeAlignedAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E,
if (OE && !OE->isValueDependent() && !OE->isIntegerConstantExpr(Context)) {
Diag(AttrLoc, diag::err_attribute_argument_n_type)
- << &TmpAttr << 2 << AANT_ArgumentIntegerConstant
- << OE->getSourceRange();
+ << CI << 2 << AANT_ArgumentIntegerConstant << OE->getSourceRange();
return;
}
@@ -1463,19 +1460,17 @@ void Sema::AddAssumeAlignedAttr(Decl *D, const
AttributeCommonInfo &CI, Expr *E,
void Sema::AddAllocAlignAttr(Decl *D, const AttributeCommonInfo &CI,
Expr *ParamExpr) {
QualType ResultType = getFunctionOrMethodResultType(D);
-
- AllocAlignAttr TmpAttr(Context, CI, ParamIdx());
SourceLocation AttrLoc = CI.getLoc();
if (!isValidPointerAttrType(ResultType, /* RefOkay */ true)) {
Diag(AttrLoc, diag::warn_attribute_return_pointers_refs_only)
- << &TmpAttr << CI.getRange() <<
getFunctionOrMethodResultSourceRange(D);
+ << CI << CI.getRange() << getFunctionOrMethodResultSourceRange(D);
return;
}
ParamIdx Idx;
const auto *FuncDecl = cast<FunctionDecl>(D);
- if (!checkFunctionOrMethodParameterIndex(FuncDecl, TmpAttr,
+ if (!checkFunctionOrMethodParameterIndex(FuncDecl, CI,
/*AttrArgNum=*/1, ParamExpr, Idx))
return;
@@ -1483,8 +1478,7 @@ void Sema::AddAllocAlignAttr(Decl *D, const
AttributeCommonInfo &CI,
if (!Ty->isDependentType() && !Ty->isIntegralType(Context) &&
!Ty->isAlignValT()) {
Diag(ParamExpr->getBeginLoc(), diag::err_attribute_integers_only)
- << &TmpAttr
- << FuncDecl->getParamDecl(Idx.getASTIndex())->getSourceRange();
+ << CI << FuncDecl->getParamDecl(Idx.getASTIndex())->getSourceRange();
return;
}
diff --git a/clang/test/SemaCXX/alloc-align-attr.cpp
b/clang/test/SemaCXX/alloc-align-attr.cpp
index 5a40e8d8fb6b5..ccc75369b8cfb 100644
--- a/clang/test/SemaCXX/alloc-align-attr.cpp
+++ b/clang/test/SemaCXX/alloc-align-attr.cpp
@@ -47,3 +47,15 @@ void dependent_impl(int align) {
dependent_param_func<int>(1);
dependent_param_func<float>(1); // expected-note {{in instantiation of
function template specialization 'dependent_param_func<float>' requested here}}
}
+
+namespace GH26612 {
+// This issue was about the align_value attribute, but alloc_align has the
+// same problematic code pattern, so is being fixed at the same time despite
+// not having the same crashing behavior.
+template <class T>
+__attribute__((alloc_align(1))) T f(T x); // expected-warning {{'alloc_align'
attribute only applies to return values that are pointers or references}}
+
+void foo() {
+ f<int>(0); // expected-note {{in instantiation of function template
specialization 'GH26612::f<int>' requested here}}
+}
+} // namespace GH26612
diff --git a/clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
b/clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
index e709c936735c7..ea1c940b6232f 100644
--- a/clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
+++ b/clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
@@ -91,3 +91,14 @@ void test24() {
atest5<s3>(); // expected-note {{in instantiation of function template
specialization 'atest5<s3>' requested here}}
}
+namespace GH26612 {
+// This issue was about the align_value attribute, but assume_aligned has the
+// same problematic code pattern, so is being fixed at the same time despite
+// not having the same crashing behavior.
+template <class T>
+__attribute__((assume_aligned(4))) T f(T x); // expected-warning
{{'assume_aligned' attribute only applies to return values that are pointers or
references}}
+
+void foo() {
+ f<int>(0); // expected-note {{in instantiation of function template
specialization 'GH26612::f<int>' requested here}}
+}
+} // namespace GH26612
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
