[Bug c/52991] New: attribute packed broken on mingw32?

[Bug c/52991] New: attribute packed broken on mingw32?

[sherpya at netfarm dot it]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

             Bug #: 52991
           Summary: attribute packed broken on mingw32?
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: sherpya@netfarm.it


I've switched form mingw gcc 4.6.2 to 4.7.0 and I've noticed that attribute
packed does not work anymore as expected, simple testcase:

#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>

typedef struct st_tag
{
    uint16_t head __attribute__ ((packed));
    uint8_t type;
    uint16_t flags __attribute__ ((packed));
} st;

int main(void)
{
    st x;
    printf("Structure packing got %u offset, expected 3\n",
(unsigned)((char*)&x.flags - (char*)&x));
    return 0;
}

the generated asm is:
        movl    $4, 4(%esp)
        movl    $LC0, (%esp)
        call    _printf

the value is 3 in gcc 4.6.2

Using built-in specs.
COLLECT_GCC=c:\mingw\bin\gcc-4.7.exe
COLLECT_LTO_WRAPPER=c:/mingw/bin/../libexec/gcc/i686-pc-mingw32/4.7.0/lto-wrapper.exe
Target: i686-pc-mingw32
Configured with: ../gcc-4.7.0/configure --prefix=/mingw --disable-bootstrap
--enable-lto --with-pkgversion=Sherpya --with-bugurl=http://oss.netfarm.it/
--target=i686-pc-mingw32 --enable-targets=i686-pc-mingw32 --with-tune=generic
--with-cpu=i686 --disable-win32-registry --disable-shared --program-suffix=-4.7
--enable-version-specific-runtime-libs --enable-languages=c,c++
--disable-werror --enable-threads=win32 --with-dwarf2 --disable-nls
Thread model: win32
gcc version 4.7.0 (Sherpya)

I don't think save-temps output may be relevant

[Bug c/52991] attribute packed broken on mingw32?

[daniel.c.klauer at web dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

daniel.c.klauer at web dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daniel.c.klauer at web dot
                   |                            |de

--- Comment #1 from daniel.c.klauer at web dot de 2012-08-18 03:37:01 UTC ---
I think I may have encountered this issue aswell. For example, given the
following example program:


#include <stdio.h>
#include <wchar.h>

struct A {
    short s;
    struct { int i; };
} __attribute__((__packed__));

struct B {
    short s;
    struct { int i; } __attribute__((__packed__));
} __attribute__((__packed__));

struct C {
    struct { int i; };
    short s;
} __attribute__((__packed__));

int main() {
    printf("sizeof(struct A) == %i\n", (int)sizeof(struct A));
    printf("sizeof(struct B) == %i\n", (int)sizeof(struct B));
    printf("sizeof(struct C) == %i\n", (int)sizeof(struct C));
    return 0;
}


with TDM-GCC's mingw32 gcc 4.6.1, all sizes are 6, and for comparison, with
Ubuntu's i686-linux-gnu gcc 4.6.3, all sizes are 6.

However, with mingw.org's gcc 4.7.0, sizeof(A) == 8. B and C still are 6, as
expected; A on the other hand apparently is not being packed, despite the
__attribute__((__packed__)).

[Bug c/52991] attribute packed broken on mingw32?

[daniel.c.klauer at web dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #2 from daniel.c.klauer at web dot de 2012-08-18 04:00:51 UTC ---
The -mms-bitfields option seems to trigger the wrong behaviour. As pointed out
by http://gcc.gnu.org/gcc-4.7/changes.html it is enabled by default since
4.7.0, and using -mms-bitfields when compiling with TDM-GCC 4.6.1 also causes
the wrong behaviour.

In contrast, compiling with -mno-ms-bitfields makes the example programs work
as expected under gcc 4.7.0.

[Bug c/52991] attribute packed broken on mingw32?

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #3 from Mikael Pettersson <mikpe at it dot uu.se> 2012-08-18 10:13:20 UTC ---
I see the same breakage with a self-built gcc-4.7.1 for x86_64-w64-mingw32.

[Bug c/52991] attribute packed broken on mingw32?

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> 2012-08-19 20:56:19 UTC ---
The problem started with the PR27942 fix in r114552:
http://gcc.gnu.org/ml/gcc-patches/2006-06/msg00569.html
http://gcc.gnu.org/ml/gcc-cvs/2006-06/msg00268.html
and it affects gcc-4.2.0 up to current trunk.

The problem isn't mingw-specific, it can be reproduced natively on e.g.
x86_64-linux.  Take this C test case, reduced from #c1:

struct A {
    short s;
    struct { int i; } x;
} __attribute__((__packed__));

struct C {
    struct { int i; } x;
    short s;
} __attribute__((__packed__));

int foo(void)
{
    return sizeof(struct A) == sizeof(struct C);
}

Compiling this with -mno-ms-bitfields makes foo() return 1, but compiling with
-mms-bitfields makes foo() return 0 because sizeof(struct A) then is 8 while
sizeof(struct C) still is 6.

[Bug c/52991] attribute packed broken on mingw32?

[mikpe at it dot uu.se]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #5 from Mikael Pettersson <mikpe at it dot uu.se> 2012-08-22 08:34:46 UTC ---
The following variation of the test case was compiled with both gcc-4.7.1 and
MS Visual Studio 2008:

#include <stdio.h>

#if defined(__GNUC__)
struct A1 {
    short s;
    struct { int i; } x;
} __attribute__((__packed__));
#endif

#pragma pack(1)
struct A2 {
    short s;
    struct { int i; } x;
};
#pragma pack()

int main(void)
{
#if defined(__GNUC__)
    printf("sizeof(struct A1) == %u\n", (unsigned)sizeof(struct A1));
#endif
    printf("sizeof(struct A2) == %u\n", (unsigned)sizeof(struct A2));
    return 0;
}

Note that structs A1 and A2 are identical, except A1 is packed (when compiled
by gcc) using the attribute syntax and A2 is packed using the #pragma syntax.

Compiling this with gcc-4.7.1 (target x86_64-w64-mingw32) with -mms-bitfields
(which is the default) gives:

sizeof(struct A1) == 8
sizeof(struct A2) == 6

proving that attribute packed doesn't work.  With -mno-ms-bitfields both
structs are reported as being 6 bytes.

Compiling with Visual Studio 2008 gives:

sizeof(struct A2) == 6

as expected.

[Bug c/52991] attribute packed broken on mingw32?

[daniel.c.klauer at web dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #6 from daniel.c.klauer at web dot de 2012-10-06 23:02:22 UTC ---
Using the ms_struct attribute instead of compiling with -mms-bitfields
reproduces the packing issue, while using the gcc_struct attribute prevents the
issue from showing up even under -mms-bitfields.

These struct examples show the packing issue without nested structs:

struct __attribute__((ms_struct, packed)) A {
    int   a;
    short b;
};
struct __attribute__((ms_struct, packed)) B {
    short a;
    int   b;
};

sizeof(struct A) == 6         as expected
sizeof(struct B) == 8         unexpected, expected 6 instead
offsetof(struct B, b) == 4    unexpected, expected 2 instead

A message [1] from the mingw-w64-public mailing list indicates this behaviour:
Only the outer struct is packed, causing padding bytes at its end to be removed
as expected, but not the fields: padding bytes in front of a field according to
the field's natural alignment are not removed, despite the packed attribute,
which is unexpected.

Explicitly packing the fields does not help:

struct __attribute__((ms_struct)) C {
    int   a __attribute__((packed, aligned(1)));
    short b __attribute__((packed, aligned(1)));
};
struct __attribute__((ms_struct)) D {
    short a __attribute__((packed, aligned(1)));
    int   b __attribute__((packed, aligned(1)));
};

sizeof(struct C) == 6         as expected
sizeof(struct D) == 8         unexpected, expected 6 instead
offsetof(struct D, b) == 4    unexpected, expected 2 instead

[1] http://sourceforge.net/mailarchive/message.php?msg_id=29650589

[Bug c/52991] attribute packed broken on mingw32?

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cgf at gcc dot gnu.org,
                   |                            |ktietz at gcc dot gnu.org

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-06 23:08:12 UTC ---
Let's add in CC target maintainers.

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-12-10
     Ever Confirmed|0                           |1

--- Comment #8 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-12-10 15:13:37 UTC ---
Yes, there is an issue about record/union attribute packed for ms_struct.

The following patch is fixing that for me, I just do regression-testing.

Index: stor-layout.c
===================================================================
--- stor-layout.c    (Revision 194356)
+++ stor-layout.c    (Arbeitskopie)
@@ -756,7 +756,10 @@ start_record_layout (tree t)
   /* If the type has a minimum specified alignment (via an attribute
      declaration, for example) use it -- otherwise, start with a
      one-byte alignment.  */
-  rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
+  if (TYPE_PACKED (t))
+    rli->record_align = BITS_PER_UNIT;
+  else
+    rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t));
   rli->unpacked_align = rli->record_align;
   rli->offset_align = MAX (rli->record_align, BIGGEST_ALIGNMENT);

@@ -952,15 +955,20 @@ update_alignment_for_field (record_layout_info rli
      meaningless.  */
   if (targetm.ms_bitfield_layout_p (rli->t))
     {
+      if (rli->t && TYPE_PACKED (rli->t)
+          && (is_bitfield || !DECL_PACKED (field)
+              || DECL_SIZE (field) == NULL_TREE
+              || !integer_zerop (DECL_SIZE (field))))
+        desired_align = BITS_PER_UNIT;
       /* Here, the alignment of the underlying type of a bitfield can
      affect the alignment of a record; even a zero-sized field
      can do this.  The alignment should be to the alignment of
      the type, except that for zero-size bitfields this only
      applies if there was an immediately prior, nonzero-size
      bitfield.  (That's the way it is, experimentally.) */
-      if ((!is_bitfield && !DECL_PACKED (field))
-      || ((DECL_SIZE (field) == NULL_TREE
-           || !integer_zerop (DECL_SIZE (field)))
+      else if ((!is_bitfield && !DECL_PACKED (field))
+           || ((DECL_SIZE (field) == NULL_TREE
+           || !integer_zerop (DECL_SIZE (field)))
           ? !DECL_PACKED (field)
           : (rli->prev_field
          && DECL_BIT_FIELD_TYPE (rli->prev_field)
@@ -1414,7 +1422,13 @@ place_field (record_layout_info rli, tree field)
         }

       /* Now align (conventionally) for the new type.  */
-      type_align = TYPE_ALIGN (TREE_TYPE (field));
+      if (!TYPE_PACKED (rli->t))
+        {
+          type_align = TYPE_ALIGN (TREE_TYPE (field));
+          if (DECL_PACKED (field))
+            type_align = MIN (type_align, BITS_PER_UNIT);
+          
+        }

       if (maximum_field_alignment != 0)
         type_align = MIN (type_align, maximum_field_alignment);

[Bug c/52991] attribute packed broken on mingw32?

[rth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Richard Henderson <rth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu.org

--- Comment #9 from Richard Henderson <rth at gcc dot gnu.org> 2012-12-18 17:14:37 UTC ---
It doesn't look to me that the first two hunks of the patch are needed.

The last hunk doesn't look like it should be considering either TYPE_PACKED
of the parent structure, or TYPE_ALIGN of the field type, but rather
using DECL_ALIGN of the field.  Which ought already have been assigned
by layout_decl via update_alignment_for_field.

The biggest problem to me appears to be the entire ms_bitfield_layout_p
block of code there, whose block comments talk about adjusting bitfields,
and yet is firing for structures that contain no bitfields at all.  That
tells me that the logic higher in the block (or elsewere wrt bitfield_layout_p)
is in error.

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #10 from Kai Tietz <ktietz at gcc dot gnu.org> 2012-12-18 21:35:27 UTC ---
(In reply to comment #9)
> It doesn't look to me that the first two hunks of the patch are needed.

Yes, that's right.

> The last hunk doesn't look like it should be considering either TYPE_PACKED
> of the parent structure, or TYPE_ALIGN of the field type, but rather
> using DECL_ALIGN of the field.  Which ought already have been assigned
> by layout_decl via update_alignment_for_field.

Well, nothing considers TYPE_PACKED of the parent structure for ms_struct.  The
field's type-alignment needs to be considered for checking, if type has
user-specified alignment AFAICS.

> The biggest problem to me appears to be the entire ms_bitfield_layout_p
> block of code there, whose block comments talk about adjusting bitfields,
> and yet is firing for structures that contain no bitfields at all.  That
> tells me that the logic higher in the block (or elsewere wrt bitfield_layout_p)
> is in error.
That isn't the fully story for ms_struct here, the ms_biffield_layout_p block
has  as internal packing of same-types - that's not related to aligned or
packed attributes at all - and needs to be done dependent to its offset, when
type's offset is known.   So there is no logical-error in this code, beside. 
Nevertheless I admit that the ms_struct code-path and the gnu_struct code-path
could win by separating them, and for both some documention about used schema
in source would be great.

In general it would be great to have somebody with a VC, who would be willing
to compile some test-cases by this VC.  The documention about structure-layout
on msdn it pretty poor and most stuff needs to be collected or done via
best-guess.  User-specified alignment within structures (not via #pragma pack)
seems to be introduced in recent VC versions, so tester should have a modern
version.

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #11 from Kai Tietz <ktietz at gcc dot gnu.org> 2013-01-09 12:30:50 UTC ---
Created attachment 29119
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29119
Testcase for bitfield and type-packing

This first testcase checking pack-pragma for x64/x86 MS-struct layout.
I got this testcase verified by a VC user.  It was verified with VC10, VC11,
and (Nov2012_CTP).
(Btw no regression with my suggested patch applied).

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #12 from Kai Tietz <ktietz at gcc dot gnu.org> 2013-01-09 13:34:07 UTC ---
Created attachment 29121
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29121
Testcase for bitfield and structure-aligning via attribute

This testcase use aligned/align attribute instead of #pragma pack.  With
suggested patch applied no regressions to VC results.

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #13 from Kai Tietz <ktietz at gcc dot gnu.org> 2013-01-09 13:37:13 UTC ---
Created attachment 29122
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29122
Testcase for bitfield and structure field-aligning by attribute

By this testcase we validate that structure's member-alignment set by attribute
is applied.  With applied patch this test shows no regressions to VC compiled
variant.

[Bug c/52991] attribute packed broken on mingw32?

[mikpelinux at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #14 from Mikael Pettersson <mikpelinux at gmail dot com> ---
Ping -- there's been a patch for this bug for almost 10 months now...

[Bug c/52991] attribute packed broken on mingw32?

[rogerdpack at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

roger pack <rogerdpack at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rogerdpack at gmail dot com

--- Comment #15 from roger pack <rogerdpack at gmail dot com> ---
So is the problem here that -mms-bitfields became the default, which caused
difficulties, or that -mms-bitfields is broken? (if the latter, does using
gcc_struct everywhere cause the right behavior?)  Sorry I'm a bit clueless
here.  Thanks!

[Bug c/52991] attribute packed broken on mingw32?

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

--- Comment #16 from Kai Tietz <ktietz at gcc dot gnu.org> ---
ms-bitfield is broken regarding pack-attribute and align-attribute.  Later is
the cause why suggested patch is just half of the story.

[Bug c/52991] attribute packed broken on mingw32?

[kugel at rockbox dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Thomas Martitz <kugel at rockbox dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kugel at rockbox dot org

--- Comment #17 from Thomas Martitz <kugel at rockbox dot org> ---
I'm affected by this as well. I was hit when cross-compiling Rockbox simulators
(www.rockbox.org) for win32 using i686-w64-mingw32-gcc, version 4.8.2

[Bug c/52991] attribute packed broken on mingw32?

[jackie.rosen at hushmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #18 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Marked for reference. Resolved as fixed @bugzilla.

[Bug target/52991] attribute packed broken on mingw32?

[ossman at cendio dot se]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Pierre Ossman <ossman at cendio dot se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ossman at cendio dot se

--- Comment #19 from Pierre Ossman <ossman at cendio dot se> ---
This bug report is now over three years old. Any progress? I'm still seeing it
in 5.1.0.

Should the summary also be updated to reflect that this is a regression?

And should the default be reverted until the code can be fixed?

[Bug target/52991] [6/7/8 Regression] attribute packed broken on mingw32?

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|6.5                         |8.0

[Bug target/52991] [6/7/8 Regression] attribute packed broken on mingw32?

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |steven.spark at gmail dot com

--- Comment #36 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 61813 has been marked as a duplicate of this bug. ***