[Bug middle-end/39514] New: unreported change to packed bitfields

[Bug middle-end/39514] New: unreported change to packed bitfields

[janis at gcc dot gnu dot org]

Tests t007 and t177 from the C++ struct-layout-1 compat tests fail when using
GCC 4.3.3 as ALT compilers when testing trunk.  The failure for one, check 932
in test t007, is due to a change from

  r132614 | ebotcazou | 2008-02-25 09:55:26 +0000 (Mon, 25 Feb 2008) | 4 lines

        * stor-layout.c (layout_decl): Do not bump the alignment of a
        bit-field to more than byte alignment if it is packed.

Here's a smaller C testcase that shows the change:

------------------------------------------------------------------------
#include <stdio.h>

struct S932
{
  long int a;
  short int b;
  struct
  {
    __attribute__ ((aligned (2))) int d:32;
  } __attribute__ ((packed)) c;
  unsigned short int e;
};
struct S932 s932;

int
main ()
{
  void *p1, *p2;
  p1 = &s932;
  p2 = &s932.e;
  printf ("offset of e  = %lu\n", (unsigned long ) (p2 - p1));
  printf ("size of s932 = %lu\n", (unsigned long) sizeof (s932));
  return 0;
}
------------------------------------------------------------------------

For 4.3.3 the output is:

offset of e  = 12
size of s932 = 16

For trunk the output is:

offset of e  = 10
size of s932 = 12

I see this on powerpc64-unknown-linux-gnu with -m32 and -m64, but it looks as
if it would affect all targets.

Recently, warning option -Wpacked-bitfield-compat was added in

  r143718 | nemet | 2009-01-27 18:55:20 -0800 (Tue, 27 Jan 2009) | 7 lines

        * c.opt (Wpacked-bitfield-compat): Change init value to -1.
        * c-opts.c (c_common_post_options): If -W*packed-bitfield-compat
        was not supplied then set warn_packed_bitfield_compat to the
        default value of 1.
        * stor-layout.c (place_field): Check warn_packed_bitfield_compat
        against 1.

I'd like to see this message, which is on by default, for the change introduced
in r132614.  I haven't figured out how to determine if the offset for a field
has changed, just the alignment.  I'll attach my current patch, where the
message should be "Perhaps the alignment of packed bit-field <x> has changed in
GCC 4.4", which is not terribly useful.  The testsuite changes from that patch
will be needed if the message is used for t005 and t177.

I'll be out of touch for the next week and will be delighted if someone else
fixes this.


-- 
           Summary: unreported change to packed bitfields
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: janis at gcc dot gnu dot org


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

[Bug middle-end/39514] unreported change to packed bitfields

[janis at gcc dot gnu dot org]

------- Comment #1 from janis at gcc dot gnu dot org  2009-03-20 23:41 -------
Created an attachment (id=17505)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17505&action=view)
partial patch to compiler, good patch to testsuite


-- 


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

[Bug middle-end/39514] unreported change to packed bitfields

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2009-03-21 00:01 -------
http://gcc.gnu.org/ml/gcc-patches/2008-02/msg00686.html

In fact I think the trunk is more correct at least that is the way I read the
sizes and offsets.


-- 


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

[Bug middle-end/39514] unreported change to packed bitfields

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2009-03-21 00:02 -------
Also see the thread starting at
http://gcc.gnu.org/ml/gcc-patches/2007-09/msg00994.html .


-- 


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

[Bug middle-end/39514] unreported change to packed bitfields

[janis at gcc dot gnu dot org]

------- Comment #4 from janis at gcc dot gnu dot org  2009-03-21 00:22 -------
I didn't call it a bug or a regression, but even a deliberate change can
produce incompatibilities between objects compiled with different versions of
GCC.  That's why it would be good to get a suppressible warning/note about it.

I copied Eric because he might know where to issue the message so there aren't
tons of false positives.


-- 


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

[Bug middle-end/39514] [4.4 Regression] unreported change to packed bitfields

[rguenth at gcc dot gnu dot org]

------- Comment #5 from rguenth at gcc dot gnu dot org  2009-03-21 11:00 -------
I agree that we need to document this change.  And I agree that the new
behavior
is more reasonable.

Can we add this example to changes.html?  It looks different enough than
the existing one.

Did behavior change if you remove the aligned (2) attribute from d?

I'm marking this as a regression to be on the radar.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |ABI, diagnostic
   Last reconfirmed|0000-00-00 00:00:00         |2009-03-21 11:00:24
               date|                            |
            Summary|unreported change to packed |[4.4 Regression] unreported
                   |bitfields                   |change to packed bitfields
   Target Milestone|---                         |4.4.0


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

[Bug middle-end/39514] [4.4 Regression] unreported change to packed bitfields

[ebotcazou at gcc dot gnu dot org]

------- Comment #6 from ebotcazou at gcc dot gnu dot org  2009-03-23 11:25 -------
> Did behavior change if you remove the aligned (2) attribute from d?

No, it didn't change without the attribute, it was and still is (10, 12).


-- 


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

[Bug middle-end/39514] [4.4 Regression] unreported change to packed bitfields

[ebotcazou at gcc dot gnu dot org]

------- Comment #7 from ebotcazou at gcc dot gnu dot org  2009-03-23 11:47 -------
> I'd like to see this message, which is on by default, for the change
> introduced in r132614.  I haven't figured out how to determine if the offset
> for a field has changed, just the alignment.  I'll attach my current patch,
> where the message should be "Perhaps the alignment of packed bit-field <x>
> has changed in GCC 4.4", which is not terribly useful.

I think that's better (without the "perhaps", the alignment is always changed)
than "Offset" because this also can change the padding of the structure:

struct s
{
  struct
  {
    __attribute__ ((aligned (2))) int d:32;
  } __attribute__ ((packed)) c;
  unsigned short int e;
};

yields (4, 8) with 4.3.3 and (4, 6) with mainline.  This apparently will be so
uncommon in C/C++ (packed bit-field with alignment attribute) that I'm not sure
sure we need to tune it further.


-- 


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

[Bug middle-end/39514] [4.4 Regression] unreported change to packed bitfields

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2


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

[Bug middle-end/39514] [4.4/4.5 Regression] unreported change to packed bitfields

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.0                       |4.4.1


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

[Bug middle-end/39514] [4.4/4.5 Regression] unreported change to packed bitfields

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.1                       |4.4.2


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

[Bug middle-end/39514] [4.4/4.5 Regression] unreported change to packed bitfields

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.2                       |4.4.3


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

[Bug middle-end/39514] [4.4/4.5 Regression] unreported change to packed bitfields

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.3                       |4.4.4


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

[Bug middle-end/39514] [4.4/4.5/4.6 Regression] unreported change to packed bitfields

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.4                       |4.4.5


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