[clang] Fix a regression with alignas on structure members in C (PR #98642)
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 16 12:43:11 PDT 2024
https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/98642
>From 41b7064dce91fdb1cf73d0e7508ad83a1edbb42a Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Fri, 12 Jul 2024 10:38:09 -0400
Subject: [PATCH 1/2] Fix a regression with alignas on structure members in C
This was a 19.x regression and thus has no release note.
Fixes #95032
---
clang/include/clang/Sema/DeclSpec.h | 3 ++-
clang/lib/Parse/ParseDecl.cpp | 10 ++++++++++
clang/lib/Sema/ParsedAttr.cpp | 2 +-
clang/test/C/C2y/n3254.c | 4 ++--
clang/test/Sema/alignas.c | 19 ++++++++++++++++---
5 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 9c22c35535ede..1dd9781bba5ea 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -2042,7 +2042,8 @@ class Declarator {
assert(llvm::all_of(DeclarationAttrs,
[](const ParsedAttr &AL) {
return (AL.isStandardAttributeSyntax() ||
- AL.isRegularKeywordAttribute());
+ AL.isRegularKeywordAttribute() ||
+ AL.isAlignas());
}) &&
"DeclarationAttrs may only contain [[]] and keyword attributes");
}
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7ce9a9cea1c7a..ae1801f8a572a 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4919,6 +4919,16 @@ void Parser::ParseStructDeclaration(
// Parse the common specifier-qualifiers-list piece.
ParseSpecifierQualifierList(DS);
+ // FIXME: _Alignas has special handling that really shouldn't be necessary.
+ // Loop over all the attributes on the specifier qualifier, find any that
+ // are alignment attributes, and shift those off the specifier qualifier and
+ // onto the declarator.
+ ParsedAttributes &DSAttrs = DS.getAttributes();
+ for (auto &PAttr : DSAttrs) {
+ if (PAttr.isAlignas())
+ Attrs.takeOneFrom(DSAttrs, &PAttr);
+ }
+
// If there are no declarators, this is a free-standing declaration
// specifier. Let the actions module cope with it.
if (Tok.is(tok::semi)) {
diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index 6abc90336c994..2109494aa5889 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -225,7 +225,7 @@ bool ParsedAttr::slidesFromDeclToDeclSpecLegacyBehavior() const {
// atributes.
return false;
- assert(isStandardAttributeSyntax());
+ assert(isStandardAttributeSyntax() || isAlignas());
// We have historically allowed some type attributes with standard attribute
// syntax to slide to the decl-specifier-seq, so we have to keep supporting
diff --git a/clang/test/C/C2y/n3254.c b/clang/test/C/C2y/n3254.c
index e08659cf377aa..347360a87faed 100644
--- a/clang/test/C/C2y/n3254.c
+++ b/clang/test/C/C2y/n3254.c
@@ -80,9 +80,9 @@ struct T {
// CHECK-LABEL: define dso_local signext i8 @quux(
// CHECK-SAME: ) #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[T:%.*]] = alloca [[STRUCT_T:%.*]], align 1
+// CHECK-NEXT: [[T:%.*]] = alloca [[STRUCT_T:%.*]], align 4
// CHECK-NEXT: [[S_PTR:%.*]] = alloca ptr, align 8
-// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 1 [[T]], i8 0, i64 12, i1 false)
+// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[T]], i8 0, i64 12, i1 false)
// CHECK-NEXT: [[BUFFER:%.*]] = getelementptr inbounds [[STRUCT_T]], ptr [[T]], i32 0, i32 0
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: store ptr [[ARRAYDECAY]], ptr [[S_PTR]], align 8
diff --git a/clang/test/Sema/alignas.c b/clang/test/Sema/alignas.c
index 020eff6a141c0..391553bc540ec 100644
--- a/clang/test/Sema/alignas.c
+++ b/clang/test/Sema/alignas.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -Dalignof=__alignof %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -Dalignof=_Alignof -DUSING_C11_SYNTAX %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c23 -DUSING_C11_SYNTAX %s
_Alignas(3) int align_illegal; //expected-error {{requested alignment is not a power of 2}}
_Alignas(int) char align_big;
@@ -18,12 +19,24 @@ void f(_Alignas(1) char c) { // expected-error {{'_Alignas' attribute cannot be
}
#ifdef USING_C11_SYNTAX
-// expected-warning at +4{{'_Alignof' applied to an expression is a GNU extension}}
-// expected-warning at +4{{'_Alignof' applied to an expression is a GNU extension}}
-// expected-warning at +4{{'_Alignof' applied to an expression is a GNU extension}}
+// expected-warning-re at +4{{'{{(_A|a)}}lignof' applied to an expression is a GNU extension}}
+// expected-warning-re at +4{{'{{(_A|a)}}lignof' applied to an expression is a GNU extension}}
+// expected-warning-re at +4{{'{{(_A|a)}}lignof' applied to an expression is a GNU extension}}
#endif
_Static_assert(alignof(align_big) == alignof(int), "k's alignment is wrong");
_Static_assert(alignof(align_small) == 1, "j's alignment is wrong");
_Static_assert(alignof(align_multiple) == 8, "l's alignment is wrong");
_Static_assert(alignof(struct align_member) == 8, "quuux's alignment is wrong");
_Static_assert(sizeof(struct align_member) == 8, "quuux's size is wrong");
+
+struct GH95032_1 {
+ _Alignas(16) char bytes[16];
+};
+_Static_assert(_Alignof(struct GH95032_1) == 16, "");
+
+#if __STDC_VERSION__ >= 202311L
+struct GH95032_2 {
+ alignas(16) char bytes[16];
+};
+static_assert(alignof(struct GH95032_2) == 16);
+#endif
>From 57d41bc1096e236df92359cfdbe6fe5585637e17 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 16 Jul 2024 15:41:45 -0400
Subject: [PATCH 2/2] Rework the approach to solving the issue
This needed some extra help to ensure that matrix_type continues to
work and that we don't introduce failing assertions in the process.
It also fixes an issue with the alignment tests in n3254.c.
---
clang/lib/Parse/ParseDecl.cpp | 10 ------
clang/lib/Sema/SemaDeclAttr.cpp | 56 ++++++++++++++++++++-------------
clang/test/C/C2y/n3254.c | 24 +++++++-------
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index ae1801f8a572a..7ce9a9cea1c7a 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4919,16 +4919,6 @@ void Parser::ParseStructDeclaration(
// Parse the common specifier-qualifiers-list piece.
ParseSpecifierQualifierList(DS);
- // FIXME: _Alignas has special handling that really shouldn't be necessary.
- // Loop over all the attributes on the specifier qualifier, find any that
- // are alignment attributes, and shift those off the specifier qualifier and
- // onto the declarator.
- ParsedAttributes &DSAttrs = DS.getAttributes();
- for (auto &PAttr : DSAttrs) {
- if (PAttr.isAlignas())
- Attrs.takeOneFrom(DSAttrs, &PAttr);
- }
-
// If there are no declarators, this is a free-standing declaration
// specifier. Let the actions module cope with it.
if (Tok.is(tok::semi)) {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 20f46c003a464..41295bfb3b94f 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6435,8 +6435,14 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
return;
// Ignore C++11 attributes on declarator chunks: they appertain to the type
- // instead.
- if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
+ // instead. Note, isCXX11Attribute() will look at whether the attribute is
+ // [[]] or alignas, while isC23Attribute() will only look at [[]]. This is
+ // important for ensuring that alignas in C23 is properly handled on a
+ // structure member declaration because it is a type-specifier-qualifier in
+ // C but still applies to the declaration rather than the type.
+ if ((S.getLangOpts().CPlusPlus ? AL.isCXX11Attribute()
+ : AL.isC23Attribute()) &&
+ !Options.IncludeCXX11Attributes)
return;
// Unknown attributes are automatically warned on. Target-specific attributes
@@ -7500,29 +7506,37 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {
// Ordering of attributes can be important, so we take care to process
// attributes in the order in which they appeared in the source code.
+ auto ProcessAttributesWithSliding =
+ [&](const ParsedAttributesView &Src,
+ const ProcessDeclAttributeOptions &Options) {
+ ParsedAttributesView NonSlidingAttrs;
+ for (ParsedAttr &AL : Src) {
+ // FIXME: this sliding is specific to standard attributes and should
+ // eventually be deprecated and removed as those are not intended to
+ // slide to anything.
+ if ((AL.isStandardAttributeSyntax() || AL.isAlignas()) &&
+ AL.slidesFromDeclToDeclSpecLegacyBehavior()) {
+ // Skip processing the attribute, but do check if it appertains to
+ // the declaration. This is needed for the `MatrixType` attribute,
+ // which, despite being a type attribute, defines a `SubjectList`
+ // that only allows it to be used on typedef declarations.
+ AL.diagnoseAppertainsTo(*this, D);
+ } else {
+ NonSlidingAttrs.addAtEnd(&AL);
+ }
+ }
+ ProcessDeclAttributeList(S, D, NonSlidingAttrs, Options);
+ };
+
// First, process attributes that appeared on the declaration itself (but
// only if they don't have the legacy behavior of "sliding" to the DeclSepc).
- ParsedAttributesView NonSlidingAttrs;
- for (ParsedAttr &AL : PD.getDeclarationAttributes()) {
- if (AL.slidesFromDeclToDeclSpecLegacyBehavior()) {
- // Skip processing the attribute, but do check if it appertains to the
- // declaration. This is needed for the `MatrixType` attribute, which,
- // despite being a type attribute, defines a `SubjectList` that only
- // allows it to be used on typedef declarations.
- AL.diagnoseAppertainsTo(*this, D);
- } else {
- NonSlidingAttrs.addAtEnd(&AL);
- }
- }
- ProcessDeclAttributeList(S, D, NonSlidingAttrs);
+ ProcessAttributesWithSliding(PD.getDeclarationAttributes(), {});
// Apply decl attributes from the DeclSpec if present.
- if (!PD.getDeclSpec().getAttributes().empty()) {
- ProcessDeclAttributeList(S, D, PD.getDeclSpec().getAttributes(),
- ProcessDeclAttributeOptions()
- .WithIncludeCXX11Attributes(false)
- .WithIgnoreTypeAttributes(true));
- }
+ ProcessAttributesWithSliding(PD.getDeclSpec().getAttributes(),
+ ProcessDeclAttributeOptions()
+ .WithIncludeCXX11Attributes(false)
+ .WithIgnoreTypeAttributes(true));
// Walk the declarator structure, applying decl attributes that were in a type
// position to the decl itself. This handles cases like:
diff --git a/clang/test/C/C2y/n3254.c b/clang/test/C/C2y/n3254.c
index 347360a87faed..60d068cf9980b 100644
--- a/clang/test/C/C2y/n3254.c
+++ b/clang/test/C/C2y/n3254.c
@@ -21,9 +21,9 @@ struct S {
// CHECK-LABEL: define dso_local i32 @foo(
// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 1
+// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 4
// CHECK-NEXT: [[S_PTR:%.*]] = alloca ptr, align 8
-// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 1 [[BUFFER]], i8 0, i64 12, i1 false)
+// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[BUFFER]], i8 0, i64 12, i1 false)
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: store ptr [[ARRAYDECAY]], ptr [[S_PTR]], align 8
// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[S_PTR]], align 8
@@ -40,13 +40,13 @@ int foo() {
// CHECK-LABEL: define dso_local signext i8 @bar(
// CHECK-SAME: ) #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 1
+// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 4
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[C:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[ARRAYDECAY]], i32 0, i32 1
-// CHECK-NEXT: store i8 97, ptr [[C]], align 1
+// CHECK-NEXT: store i8 97, ptr [[C]], align 4
// CHECK-NEXT: [[ARRAYDECAY1:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[C2:%.*]] = getelementptr inbounds [[STRUCT_S]], ptr [[ARRAYDECAY1]], i32 0, i32 1
-// CHECK-NEXT: [[TMP0:%.*]] = load i8, ptr [[C2]], align 1
+// CHECK-NEXT: [[TMP0:%.*]] = load i8, ptr [[C2]], align 4
// CHECK-NEXT: ret i8 [[TMP0]]
//
char bar() {
@@ -58,13 +58,13 @@ char bar() {
// CHECK-LABEL: define dso_local float @baz(
// CHECK-SAME: ) #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 1
+// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 4
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[F:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[ARRAYDECAY]], i32 0, i32 2
-// CHECK-NEXT: store float 3.000000e+00, ptr [[F]], align 1
+// CHECK-NEXT: store float 3.000000e+00, ptr [[F]], align 4
// CHECK-NEXT: [[ARRAYDECAY1:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[F2:%.*]] = getelementptr inbounds [[STRUCT_S]], ptr [[ARRAYDECAY1]], i32 0, i32 2
-// CHECK-NEXT: [[TMP0:%.*]] = load float, ptr [[F2]], align 1
+// CHECK-NEXT: [[TMP0:%.*]] = load float, ptr [[F2]], align 4
// CHECK-NEXT: ret float [[TMP0]]
//
float baz() {
@@ -100,10 +100,10 @@ char quux() {
// CHECK-LABEL: define dso_local float @quibble(
// CHECK-SAME: ) #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 1
+// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 4
// CHECK-NEXT: [[T_PTR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: [[S_PTR:%.*]] = alloca ptr, align 8
-// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 1 [[BUFFER]], i8 0, i64 12, i1 false)
+// CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[BUFFER]], i8 0, i64 12, i1 false)
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: store ptr [[ARRAYDECAY]], ptr [[T_PTR]], align 8
// CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[T_PTR]], align 8
@@ -125,13 +125,13 @@ float quibble() {
// CHECK-LABEL: define dso_local i32 @quorble(
// CHECK-SAME: ) #[[ATTR0]] {
// CHECK-NEXT: [[ENTRY:.*:]]
-// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 1
+// CHECK-NEXT: [[BUFFER:%.*]] = alloca [12 x i8], align 4
// CHECK-NEXT: [[S_PTR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: [[ARRAYDECAY:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[BUFFER1:%.*]] = getelementptr inbounds [[STRUCT_T:%.*]], ptr [[ARRAYDECAY]], i32 0, i32 0
// CHECK-NEXT: [[ARRAYDECAY2:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER1]], i64 0, i64 0
// CHECK-NEXT: [[X:%.*]] = getelementptr inbounds [[STRUCT_S:%.*]], ptr [[ARRAYDECAY2]], i32 0, i32 0
-// CHECK-NEXT: store i32 12, ptr [[X]], align 1
+// CHECK-NEXT: store i32 12, ptr [[X]], align 4
// CHECK-NEXT: [[ARRAYDECAY3:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER]], i64 0, i64 0
// CHECK-NEXT: [[BUFFER4:%.*]] = getelementptr inbounds [[STRUCT_T]], ptr [[ARRAYDECAY3]], i32 0, i32 0
// CHECK-NEXT: [[ARRAYDECAY5:%.*]] = getelementptr inbounds [12 x i8], ptr [[BUFFER4]], i64 0, i64 0
More information about the cfe-commits
mailing list