http://d.puremagic.com/issues/show_bug.cgi?id=3398

           Summary: Version block inside a union screws data alignment
           Product: D
           Version: 2.031
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: 2korden@gmail.com


--- Comment #0 from Koroskin Denis <2korden@gmail.com> 2009-10-14 07:23:40 PDT ---
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)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3398


Don <clugdbug@yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug@yahoo.com.au
            Summary|Version block inside a      |Attributes inside a union
                   |union screws data alignment |screws data alignment
           Severity|normal                      |critical


--- Comment #1 from Don <clugdbug@yahoo.com.au> 2010-04-21 01:14:12 PDT ---
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;
-------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3398



--- Comment #2 from Don <clugdbug@yahoo.com.au> 2010-04-22 06:28:52 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3398



--- Comment #3 from Don <clugdbug@yahoo.com.au> 2010-06-09 00:31:56 PDT ---
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");

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=3398


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla@digitalmars.com
         Resolution|                            |FIXED


--- Comment #4 from Walter Bright <bugzilla@digitalmars.com> 2010-06-09 16:37:24 PDT ---
http://www.dsource.org/projects/dmd/changeset/529

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------