Bug 3398 – Attributes inside a union screws data alignment

Here is a test case: union Foo1 { int a; float b; } version = Test; union Foo2 { version (Test) { int a; float b; } } pragma(msg, Foo1.sizeof.stringof); // 4u (okay) pragma(msg, Foo2.sizeof.stringof); // 8u (wrong)

This applies not just to version declarations; it applies to any attribute. union Foo1 { double[2] a; double[2] b; } union Foo2 { public: double[2] a; double[2] b; } static assert(Foo1.sizeof == Foo2.sizeof); --- bug.d(32): Error: static assert (16u == 32u) is false Subtle and disastrous bug (especially when dealing with C libraries), escalating severity. Here's a mitigation to create an error message, rather than generate wrong code. MITIGATION: struct.c StructDeclaration::semantic() line 384. The problem is that AttribDeclaration::semantic has it's own loop for doing the semantic on members, which does NOT include setting the offset to 0 if it's a union. Fixing this would not be difficult, but would require a minor structural change in the compiler. --- for (int i = 0; i < members_dim; i++) { Dsymbol *s = (Dsymbol *)members->data[i]; + if (isUnionDeclaration() && s->isAttribDeclaration()) + error("Attributes don't yet work inside unions, see Bugzilla 3398"); s->semantic(sc2); if (isUnionDeclaration()) sc2->offset = 0; -------

One instance where wrong code is currently generated is in std.c.windows.winsock.d, in_addr and in6_addr. My mitigation would break existing code, though, in cases where the AttribDeclaration is actually an AnonDeclaration.

PATCH: Currently, after processing every field, sc->offset gets changed, and StructDeclaration::semantic() sets the offset back to zero if it was a union. This patch moves the processing to AggregateDeclaration::addField() instead. If it's a union, don't change the offset in the first place. Index: struct.c =================================================================== --- struct.c (revision 527) +++ struct.c (working copy) @@ -194,20 +194,23 @@ sizeok = 2; // cannot finish; flag as forward referenced return; } + size_t ofs = sc->offset; memsize = t->size(loc); memalignsize = t->alignsize(); xalign = t->memalign(sc->structalign); - alignmember(xalign, memalignsize, &sc->offset); - v->offset = sc->offset; - sc->offset += memsize; - if (sc->offset > structsize) - structsize = sc->offset; + alignmember(xalign, memalignsize, &ofs); + v->offset = ofs; + ofs += memsize; + if (ofs > structsize) + structsize = ofs; if (sc->structalign < memalignsize) memalignsize = sc->structalign; if (alignsize < memalignsize) alignsize = memalignsize; //printf("\talignsize = %d\n", alignsize); + if (!isUnionDeclaration()) + sc->offset = ofs; v->storage_class |= STCfield; //printf(" addField '%s' to '%s' at offset %d, size = %d\n", v->toChars(), toChars(), v->offset, memsize); @@ -386,8 +389,6 @@ { Dsymbol *s = (Dsymbol *)members->data[i]; s->semantic(sc2); - if (isUnionDeclaration()) - sc2->offset = 0; #if 0 if (sizeok == 2) { //printf("forward reference\n");