ImportC: Fix interaction of aligned and packed structs (#21204)

Previous iteration of this did not properly account for the interaction
of aligned and packed and would even segfault on a null access in such
a case. This version properly handles that interaction by aligning the
struct itself instead of the first member and not forcing the struct
alignment to 1 if it is packed.

compiler/src/dmd/astbase.d

@@ -3775,13 +3775,14 @@ struct ASTBase
         TOK tok;
         Identifier id;
         structalign_t packalign;
+        Expressions* alignExps;
         Dsymbols* members;
         Type base;
 
         Type resolved;
         MOD mod;
 
-        extern (D) this(Loc loc, TOK tok, Identifier id, structalign_t packalign, Type base, Dsymbols* members)
+        extern (D) this(Loc loc, TOK tok, Identifier id, structalign_t packalign, Expressions* alignExps, Type base, Dsymbols* members)
         {
             //printf("TypeTag %p\n", this);
             super(Ttag);
@@ -3789,6 +3790,7 @@ struct ASTBase
             this.tok = tok;
             this.id = id;
             this.packalign = packalign;
+            this.alignExps = alignExps;
             this.base = base;
             this.members = members;
             this.mod = 0;

compiler/src/dmd/cparse.d

@@ -1775,7 +1775,7 @@ final class CParser(AST) : Parser!AST
             auto stag = (tt.tok == TOK.struct_) ? new AST.StructDeclaration(tt.loc, tt.id, false) :
                         (tt.tok == TOK.union_)  ? new AST.UnionDeclaration(tt.loc, tt.id) :
                                                   new AST.EnumDeclaration(tt.loc, tt.id, tt.base);
-            if (!tt.packalign.isUnknown())
+            if (!tt.alignExps && !tt.packalign.isUnknown())
             {
                 // saw `struct __declspec(align(N)) Tag ...`
                 auto st = stag.isStructDeclaration();
@@ -1785,7 +1785,13 @@ final class CParser(AST) : Parser!AST
             tt.members = null;
             if (!symbols)
                 symbols = new AST.Dsymbols();
-            auto stags = applySpecifier(stag, specifier);
+            AST.Dsymbol stags = stag;
+            if (tt.alignExps)
+            {
+                auto decls = new AST.Dsymbols(1);
+                (*decls)[0] = stags;
+                stags = new AST.AlignDeclaration(stags.loc, tt.alignExps, decls);
+            }
             symbols.push(stags);
             return stags;
         }
@@ -2452,25 +2458,7 @@ final class CParser(AST) : Parser!AST
                     const sloc = token.loc;
                     nextToken();
 
-                    Specifier tagSpecifier;
+                    t = cparseStruct(sloc, structOrUnion, symbols);
-
-                    /* GNU Extensions
-                     * struct-or-union-specifier:
-                     *    struct-or-union gnu-attributes (opt) identifier (opt) { struct-declaration-list } gnu-attributes (opt)
-                     *    struct-or-union gnu-attribute (opt) identifier
-                     */
-                    while (1)
-                    {
-                        if (token.value == TOK.__attribute__)
-                            cparseGnuAttributes(tagSpecifier);
-                        else if (token.value == TOK.__declspec)
-                            cparseDeclspec(tagSpecifier);
-                        else if (token.value == TOK.__pragma)
-                            uupragmaDirective(sloc);
-                        else
-                            break;
-                    }
-                    t = cparseStruct(sloc, structOrUnion, tagSpecifier, symbols);
                     tkwx = TKW.xtag;
                     break;
                 }
@@ -3940,7 +3928,7 @@ final class CParser(AST) : Parser!AST
          * redeclaration, or reference to existing declaration.
          * Defer to the semantic() pass with a TypeTag.
          */
-        return new AST.TypeTag(loc, TOK.enum_, tag, structalign_t.init, base, members);
+        return new AST.TypeTag(loc, TOK.enum_, tag, structalign_t.init, null, base, members);
     }
 
     /*************************************
@@ -3967,10 +3955,26 @@ final class CParser(AST) : Parser!AST
      * Returns:
      *  type of the struct
      */
-    private AST.Type cparseStruct(Loc loc, TOK structOrUnion, ref Specifier specifier, ref AST.Dsymbols* symbols)
+    private AST.Type cparseStruct(Loc loc, TOK structOrUnion, ref AST.Dsymbols* symbols)
     {
+        /* GNU Extensions
+         * struct-or-union-specifier:
+         *    struct-or-union gnu-attributes (opt) identifier (opt) { struct-declaration-list } gnu-attributes (opt)
+         *    struct-or-union gnu-attribute (opt) identifier
+         */
+        Specifier tagSpecifier;
+        while (1)
+        {
+            if (token.value == TOK.__attribute__)
+                cparseGnuAttributes(tagSpecifier);
+            else if (token.value == TOK.__declspec)
+                cparseDeclspec(tagSpecifier);
+            else if (token.value == TOK.__pragma)
+                uupragmaDirective(loc);
+            else
+                break;
+        }
         Identifier tag;
-        auto packalign = specifier.packalign;
 
         if (token.value == TOK.identifier)
         {
@@ -3985,7 +3989,7 @@ final class CParser(AST) : Parser!AST
             members = new AST.Dsymbols();          // so `members` will be non-null even with 0 members
             while (token.value != TOK.rightCurly)
             {
-                cparseStructDeclaration(members, packalign);
+                cparseStructDeclaration(members, tagSpecifier.packalign);
 
                 if (token.value == TOK.endOfFile)
                     break;
@@ -4003,13 +4007,10 @@ final class CParser(AST) : Parser!AST
             /* GNU Extensions
              * Parse the postfix gnu-attributes (opt)
              */
-            specifier.packalign = structalign_t();
             if (token.value == TOK.__attribute__)
-                cparseGnuAttributes(specifier);
+                cparseGnuAttributes(tagSpecifier);
-            if (!specifier.packalign.isUnknown)
+            if (!tagSpecifier.packalign.isUnknown)
             {
-                packalign.set(specifier.packalign.get());
-                packalign.setPack(specifier.packalign.isPack());
                 foreach (ref d; (*members)[])
                 {
                     // skip possible static assert declarations
@@ -4017,33 +4018,7 @@ final class CParser(AST) : Parser!AST
 
                     auto decls = new AST.Dsymbols(1);
                     (*decls)[0] = d;
-                    d = new AST.AlignDeclaration(d.loc, specifier.packalign, decls);
+                    d = new AST.AlignDeclaration(d.loc, tagSpecifier.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;
-                        }
-                    }
                 }
             }
         }
@@ -4051,14 +4026,14 @@ final class CParser(AST) : Parser!AST
             error("missing tag `identifier` after `%s`", Token.toChars(structOrUnion));
 
         // many ways and places to declare alignment
-        if (packalign.isUnknown() && !this.packalign.isUnknown())
+        if (tagSpecifier.packalign.isUnknown() && !this.packalign.isUnknown())
-            packalign.set(this.packalign.get());
+            tagSpecifier.packalign.set(this.packalign.get());
 
         /* Need semantic information to determine if this is a declaration,
          * redeclaration, or reference to existing declaration.
          * Defer to the semantic() pass with a TypeTag.
          */
-        return new AST.TypeTag(loc, structOrUnion, tag, packalign, null, members);
+        return new AST.TypeTag(loc, structOrUnion, tag, tagSpecifier.packalign, tagSpecifier.alignExps, null, members);
     }
 
     /*************************************

compiler/src/dmd/frontend.h

@@ -4850,6 +4850,7 @@ public:
     Loc loc;
     TOK tok;
     structalign_t packalign;
+    Array<Expression* >* alignExps;
     Identifier* id;
     Type* base;
     Array<Dsymbol* >* members;

compiler/src/dmd/mtype.d

@@ -3761,6 +3761,7 @@ extern (C++) final class TypeTag : Type
     Loc loc;                /// location of declaration
     TOK tok;                /// TOK.struct_, TOK.union_, TOK.enum_
     structalign_t packalign; /// alignment of struct/union fields
+    Expressions* alignExps; /// alignment of struct itself
     Identifier id;          /// tag name identifier
     Type base;              /// base type for enums otherwise null
     Dsymbols* members;      /// members of struct, null if none
@@ -3770,7 +3771,7 @@ extern (C++) final class TypeTag : Type
                             ///   struct S { int a; } s1, *s2;
     MOD mod;                /// modifiers to apply after type is resolved (only MODFlags.const_ at the moment)
 
-    extern (D) this(Loc loc, TOK tok, Identifier id, structalign_t packalign, Type base, Dsymbols* members) @safe
+    extern (D) this(Loc loc, TOK tok, Identifier id, structalign_t packalign, Expressions* alignExps, Type base, Dsymbols* members) @safe
     {
         //printf("TypeTag ctor %s %p\n", id ? id.toChars() : "null".ptr, this);
         super(Ttag);
@@ -3778,6 +3779,7 @@ extern (C++) final class TypeTag : Type
         this.tok = tok;
         this.id = id;
         this.packalign = packalign;
+        this.alignExps = alignExps;
         this.base = base;
         this.members = members;
         this.mod = 0;

compiler/test/compilable/test21150.c

@@ -41,3 +41,37 @@ struct __attribute__((aligned(4))) D {
 __attribute__((aligned(8)));
 
 _Static_assert(_Alignof(struct D)==8, "D");
+//
+// Interaction of aligned() and packed
+//
+#include <stddef.h>
+ struct Spacked {
+     unsigned a;
+     unsigned long long b;
+ } __attribute__((aligned(4), packed));
+_Static_assert(_Alignof(struct Spacked) == 4, "Spacked");
+_Static_assert(_Alignof(struct Spacked) == 4, "Spacked");
+_Static_assert(offsetof(struct Spacked, a) == 0, "Spacked.a");
+_Static_assert(offsetof(struct Spacked, b) == sizeof(unsigned), "Spacked.b");
+_Static_assert(sizeof(struct Spacked) == sizeof(unsigned) + sizeof(unsigned long long), "sizeof(Spacked)");
+
+struct __attribute__((aligned(4))) Spacked2 {
+    unsigned a;
+    unsigned long long b;
+} __attribute__((packed));
+_Static_assert(_Alignof(struct Spacked2) == 4, "Spacked2");
+_Static_assert(offsetof(struct Spacked2, a) == 0, "Spacked2.a");
+_Static_assert(offsetof(struct Spacked2, b) == sizeof(unsigned), "Spacked2.b");
+_Static_assert(sizeof(struct Spacked2) == sizeof(unsigned) + sizeof(unsigned long long), "sizeof(Spacked2)");
+
+#pragma pack(push)
+#pragma pack(1)
+struct __attribute__((aligned(4))) Spacked3 {
+    unsigned a;
+    unsigned long long b;
+};
+#pragma pack(pop)
+_Static_assert(_Alignof(struct Spacked3) == 4, "Spacked3");
+_Static_assert(offsetof(struct Spacked3, a) == 0, "Spacked3.a");
+_Static_assert(offsetof(struct Spacked3, b) == sizeof(unsigned), "Spacked3.b");
+_Static_assert(sizeof(struct Spacked3) == sizeof(unsigned) + sizeof(unsigned long long), "sizeof(Spacked3)");