From 7dd0506aaf94473b30684b5b5816ab69570744bf Mon Sep 17 00:00:00 2001 From: drpriver Date: Tue, 8 Apr 2025 19:59:26 -0700 Subject: [PATCH] Fix #21150 - ImportC: alignment value expected, not _Alignof (#21180) The gnu attribute aligned() allows specifying the alignment of an entire struct, mostly as a syntatic convenience. This attribute allows compile-time integer expressions, but the parser was trying to evaluate them ahead of time by checking for an integer literal. Instead we need to preserve the expression and defer it to a later semantic stage. Accomplish this by emulating the behavior by specifying the alignment of the first member of the struct. I didn't change how __declspec(align(#)) parses as from the documentation it seems to only allow integer literals. Some light testing with cl.exe gives syntax errors when trying to use _Alignof() in that position. --- compiler/src/dmd/cparse.d | 86 +++++++++++++------- compiler/src/dmd/dsymbol.d | 1 + compiler/src/dmd/frontend.h | 2 + compiler/test/compilable/test21150.c | 43 ++++++++++ compiler/test/fail_compilation/alignedext.i | 7 +- compiler/test/fail_compilation/alignedext2.i | 9 ++ 6 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 compiler/test/compilable/test21150.c create mode 100644 compiler/test/fail_compilation/alignedext2.i diff --git a/compiler/src/dmd/cparse.d b/compiler/src/dmd/cparse.d index c7650c6c51..3ad7ed093c 100644 --- a/compiler/src/dmd/cparse.d +++ b/compiler/src/dmd/cparse.d @@ -1913,10 +1913,14 @@ final class CParser(AST) : Parser!AST break; } - if (specifier.alignExps && dt.isTypeFunction()) - error("no alignment-specifier for function declaration"); // C11 6.7.5-2 - if (specifier.alignExps && specifier.scw == SCW.xregister) - error("no alignment-specifier for `register` storage class"); // C11 6.7.5-2 + + // Check alignasExp and not alignExps so that gnu + // __atribute__((aligned())) is silently allowed, matching the + // behavior of other compilers. + if (specifier.alignasExp && dt.isTypeFunction()) + error(specifier.alignasExp.loc, "no alignment-specifier for function declaration"); // C11 6.7.5-2 + if (specifier.alignasExp && specifier.scw == SCW.xregister) + error(specifier.alignasExp.loc, "no alignment-specifier for `register` storage class"); // C11 6.7.5-2 /* C11 6.9.1 Function Definitions * function-definition: @@ -1960,8 +1964,8 @@ final class CParser(AST) : Parser!AST { if (token.value == TOK.assign) error("no initializer for typedef declaration"); - if (specifier.alignExps) - error("no alignment-specifier for typedef declaration"); // C11 6.7.5-2 + if (specifier.alignasExp) + error(specifier.alignasExp.loc, "no alignment-specifier for typedef declaration"); // C11 6.7.5-2 if (specifier.vector_size) { @@ -2474,7 +2478,7 @@ final class CParser(AST) : Parser!AST else break; } - t = cparseStruct(sloc, structOrUnion, tagSpecifier.packalign, symbols); + t = cparseStruct(sloc, structOrUnion, tagSpecifier, symbols); tkwx = TKW.xtag; break; } @@ -2536,6 +2540,7 @@ final class CParser(AST) : Parser!AST if (!specifier.alignExps) specifier.alignExps = new AST.Expressions(0); specifier.alignExps.push(exp); + specifier.alignasExp = exp; check(TOK.rightParenthesis); break; @@ -3620,26 +3625,18 @@ final class CParser(AST) : Parser!AST if (token.value == TOK.leftParenthesis) { nextToken(); - if (token.value == TOK.int32Literal) - { - const n = token.unsvalue; - if (n < 1 || n & (n - 1) || ushort.max < n) - error("__attribute__((aligned(%lld))) must be an integer positive power of 2 and be <= 32,768", cast(ulong)n); - specifier.packalign.set(cast(uint)n); - specifier.packalign.setPack(true); - nextToken(); - } - else - { - error("alignment value expected, not `%s`", token.toChars()); - nextToken(); - } - + AST.Expression exp = cparseConstantExp(); + if (!specifier.alignExps) + specifier.alignExps = new AST.Expressions(0); + specifier.alignExps.push(exp); check(TOK.rightParenthesis); } - /* ignore __attribute__((aligned)), which sets the alignment to the largest value for any data - * type on the target machine. It's the opposite of __attribute__((packed)) - */ + else + { + /* ignore __attribute__((aligned)), which sets the alignment to the largest value for any data + * type on the target machine. It's the opposite of __attribute__((packed)) + */ + } } else if (token.ident == Id.packed) { @@ -3978,9 +3975,10 @@ final class CParser(AST) : Parser!AST * Returns: * type of the struct */ - private AST.Type cparseStruct(Loc loc, TOK structOrUnion, structalign_t packalign, ref AST.Dsymbols* symbols) + private AST.Type cparseStruct(Loc loc, TOK structOrUnion, ref Specifier specifier, ref AST.Dsymbols* symbols) { Identifier tag; + auto packalign = specifier.packalign; if (token.value == TOK.identifier) { @@ -4013,7 +4011,7 @@ final class CParser(AST) : Parser!AST /* GNU Extensions * Parse the postfix gnu-attributes (opt) */ - Specifier specifier; + specifier.packalign = structalign_t(); if (token.value == TOK.__attribute__) cparseGnuAttributes(specifier); if (!specifier.packalign.isUnknown) @@ -4022,11 +4020,40 @@ final class CParser(AST) : Parser!AST packalign.setPack(specifier.packalign.isPack()); foreach (ref d; (*members)[]) { + // skip possible static assert declarations + if (d.isStaticAssert()) continue; + auto decls = new AST.Dsymbols(1); (*decls)[0] = d; d = new AST.AlignDeclaration(d.loc, specifier.packalign, decls); } } + if (specifier.alignExps && specifier.alignExps.length) + { + // Align the entire struct by aligning the first member. + if (members) + { + foreach (ref d; (*members)[]) + { + // skip possible static assert declarations + if (d.isStaticAssert()) continue; + + if (AST.AlignDeclaration ad = d.isAlignDeclaration()) + { + foreach (exp; *specifier.alignExps) + ad.exps.push(exp); + break; + } + else + { + auto decls = new AST.Dsymbols(1); + (*decls)[0] = d; + d = new AST.AlignDeclaration(d.loc, specifier.alignExps, decls); + break; + } + } + } + } } else if (!tag) error("missing tag `identifier` after `%s`", Token.toChars(structOrUnion)); @@ -4178,8 +4205,8 @@ final class CParser(AST) : Parser!AST error("specifier-qualifier-list required"); else if (width) { - if (specifier.alignExps) - error("no alignment-specifier for bit field declaration"); // C11 6.7.5-2 + if (specifier.alignasExp) + error(specifier.alignasExp.loc, "no alignment-specifier for bit field declaration"); // C11 6.7.5-2 auto s = new AST.BitFieldDeclaration(width.loc, dt, id, width); members.push(s); } @@ -5155,6 +5182,7 @@ final class CParser(AST) : Parser!AST SCW scw; /// storage-class specifiers MOD mod; /// type qualifiers AST.Expressions* alignExps; /// alignment + AST.Expression alignasExp; /// Last _Alignas() for errors structalign_t packalign; /// #pragma pack alignment value } diff --git a/compiler/src/dmd/dsymbol.d b/compiler/src/dmd/dsymbol.d index 1b584eafaf..d2c1f8b308 100644 --- a/compiler/src/dmd/dsymbol.d +++ b/compiler/src/dmd/dsymbol.d @@ -1226,6 +1226,7 @@ extern (C++) class Dsymbol : ASTNode return null; } } + inout(AlignDeclaration) isAlignDeclaration() inout { return dsym == DSYM.alignDeclaration ? cast(inout(AlignDeclaration)) cast(void*) this : null; } inout(AnonDeclaration) isAnonDeclaration() inout { return dsym == DSYM.anonDeclaration ? cast(inout(AnonDeclaration)) cast(void*) this : null; } inout(CPPNamespaceDeclaration) isCPPNamespaceDeclaration() inout { return dsym == DSYM.cppNamespaceDeclaration ? cast(inout(CPPNamespaceDeclaration)) cast(void*) this : null; } inout(VisibilityDeclaration) isVisibilityDeclaration() inout { return dsym == DSYM.visibilityDeclaration ? cast(inout(VisibilityDeclaration)) cast(void*) this : null; } diff --git a/compiler/src/dmd/frontend.h b/compiler/src/dmd/frontend.h index ce1e3d37ff..44a370492c 100644 --- a/compiler/src/dmd/frontend.h +++ b/compiler/src/dmd/frontend.h @@ -96,6 +96,7 @@ class Import; class EnumDeclaration; class SymbolDeclaration; class AttribDeclaration; +class AlignDeclaration; class AnonDeclaration; class VisibilityDeclaration; class OverloadSet; @@ -674,6 +675,7 @@ public: EnumDeclaration* isEnumDeclaration(); SymbolDeclaration* isSymbolDeclaration(); AttribDeclaration* isAttribDeclaration(); + AlignDeclaration* isAlignDeclaration(); AnonDeclaration* isAnonDeclaration(); CPPNamespaceDeclaration* isCPPNamespaceDeclaration(); VisibilityDeclaration* isVisibilityDeclaration(); diff --git a/compiler/test/compilable/test21150.c b/compiler/test/compilable/test21150.c new file mode 100644 index 0000000000..7f193b528f --- /dev/null +++ b/compiler/test/compilable/test21150.c @@ -0,0 +1,43 @@ +// https://github.com/dlang/dmd/issues/21150 + +struct S +{ + char z[16][256]; +} __attribute__((aligned(_Alignof(unsigned int)))); +_Static_assert(_Alignof(struct S) == _Alignof(unsigned int), "S"); + +struct T { + _Static_assert(1, ""); + char x; +} __attribute__((aligned(_Alignof(struct S)))); + +_Static_assert(_Alignof(struct T) == _Alignof(unsigned int), "T"); + + +struct __attribute__ ((aligned(8))) Q { + short f[3]; +}; +_Static_assert(sizeof(struct Q) == 8, "Q1"); +_Static_assert(_Alignof(struct Q) == 8, "Q2"); + + +struct __attribute__ ((aligned(8))) R { + short f[3]; + char c; +}; +_Static_assert(sizeof(struct R) == 8, "R1"); +_Static_assert(_Alignof(struct R) == 8, "R2"); + +struct C { + unsigned _Alignas(2) _Alignas(4) char c; +} +__attribute__((aligned(8))); + +_Static_assert(_Alignof(struct C)==8, "C"); + +struct __attribute__((aligned(4))) D { + unsigned char c; +} +__attribute__((aligned(8))); + +_Static_assert(_Alignof(struct D)==8, "D"); diff --git a/compiler/test/fail_compilation/alignedext.i b/compiler/test/fail_compilation/alignedext.i index eae3137ce4..55c8477e0b 100644 --- a/compiler/test/fail_compilation/alignedext.i +++ b/compiler/test/fail_compilation/alignedext.i @@ -2,13 +2,10 @@ --- fail_compilation/alignedext.i(10): Error: __decspec(align(123)) must be an integer positive power of 2 and be <= 8,192 fail_compilation/alignedext.i(11): Error: __decspec(align(16384)) must be an integer positive power of 2 and be <= 8,192 -fail_compilation/alignedext.i(13): Error: __attribute__((aligned(123))) must be an integer positive power of 2 and be <= 32,768 -fail_compilation/alignedext.i(14): Error: __attribute__((aligned(65536))) must be an integer positive power of 2 and be <= 32,768 + + --- */ typedef struct __declspec(align(123)) S { int a; } S; struct __declspec(align(16384)) T { int a; }; - -typedef struct __attribute__((aligned(123))) U { int a; } S; -struct __attribute__((aligned(65536))) V { int a; }; diff --git a/compiler/test/fail_compilation/alignedext2.i b/compiler/test/fail_compilation/alignedext2.i new file mode 100644 index 0000000000..a0950ca9fe --- /dev/null +++ b/compiler/test/fail_compilation/alignedext2.i @@ -0,0 +1,9 @@ +/* TEST_OUTPUT: +--- +fail_compilation/alignedext2.i(8): Error: alignment must be an integer positive power of 2, not 0x7b +fail_compilation/alignedext2.i(9): Error: alignment must be an integer positive power of 2, not 0x10000 +--- +*/ + +typedef struct __attribute__((aligned(123))) U { int a; } S; +struct __attribute__((aligned(65536))) V { int a; };