[clang] 543e5f5 - Fix crash with align_value diagnostic reporting (#135013)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 9 09:39:07 PDT 2025
Author: Aaron Ballman
Date: 2025-04-09T12:39:03-04:00
New Revision: 543e5f5842fe576bb8752f04345b2cc995e5bb50
URL: https://github.com/llvm/llvm-project/commit/543e5f5842fe576bb8752f04345b2cc995e5bb50
DIFF: https://github.com/llvm/llvm-project/commit/543e5f5842fe576bb8752f04345b2cc995e5bb50.diff
LOG: Fix crash with align_value diagnostic reporting (#135013)
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
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaCXX/align_value.cpp
clang/test/SemaCXX/alloc-align-attr.cpp
clang/test/SemaCXX/builtin-assume-aligned-tmpl.cpp
Removed:
################################################################################
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/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 d76afe9d6464d..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;
}
@@ -4383,7 +4377,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 +4390,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 at +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
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
More information about the cfe-commits
mailing list