[Bug target/110044] New: #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[Bug target/110044] New: #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[vital.had at gmail dot com]

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

            Bug ID: 110044
           Summary: #pragma pack(push, 1) may not force packing, while
                    __attribute__((packed, aligned(1))) works
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vital.had at gmail dot com
                CC: iains at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc-apple-darwin

Problem: #pragma pack(push, 1) may not work correctly on ppc (32-bit); seems to
be present across GCC versions, confirmed to affect gcc7, gcc11 and gcc12.
Old Apple GCC 4.2 is not affected, at the same time.


Test code:

#include <iostream>
#include <stdint.h>

#pragma pack(push, 1)

/* struct from OpenEXR; should be packed with the pragma directive  */
typedef struct
{
    uint32_t x_size;
    uint32_t y_size;
    uint8_t  level_and_round;
} exr_attr_tiledesc_t;

/* same struct but reordered */
typedef struct
{
    uint8_t  level_and_round;
    uint32_t x_size;
    uint32_t y_size;
} new1_exr_attr_tiledesc_t;

/* same as first struct but with packed forced */
typedef struct
{
    uint32_t x_size;
    uint32_t y_size;
    uint8_t  level_and_round;
} __attribute__((packed, aligned(1))) new2_exr_attr_tiledesc_t;

#pragma pack(pop)

int main() {
    std::cout << sizeof(exr_attr_tiledesc_t) << " "
              << sizeof(new1_exr_attr_tiledesc_t) << " "
              << sizeof(new2_exr_attr_tiledesc_t) << "\n";

    return 0;
}


On Mac OS X Leopart (10.5 PowerPC):
    `g++-mp-7 main.cxx && ./a.out` gives: 12 9 9
    `g++ main.cxx && ./a.out`      gives: 9  9 9
    `g++* -arch ppc64 && ./a.out`  gives: 9 9 9

On Mac OS X Snow Leopard (10A190 PowerPC):
    `g++-mp-11 main.cxx && ./a.out` gives: 12 9 9
    `g++-mp-12 main.cxx && ./a.out` gives: 12 9 9
    `g++ main.cxx && ./a.out`       gives: 9 9 9

where g++ stands for Xcode gcc-4.2.

Discussion in: https://github.com/macports/macports-ports/pull/18872
Also see: https://trac.macports.org/ticket/63490

[Bug target/110044] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ABI, wrong-code

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect the issue is inside darwin_rs6000_special_round_type_align .
But I can't seem to figure out just by looking at the code.

[Bug target/110044] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[egallager at gcc dot gnu.org]

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

Eric Gallager <egallager at gcc dot gnu.org> changed:

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

--- Comment #2 from Eric Gallager <egallager at gcc dot gnu.org> ---
possible dup of either bug 60972 and/or bug 68160?

[Bug target/110044] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[vital.had at gmail dot com]

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

--- Comment #3 from Sergey Fedorov <vital.had at gmail dot com> ---
(In reply to Eric Gallager from comment #2)
> possible dup of either bug 60972 and/or bug 68160?

From those topics it looks that the bug, if identical, has never been addressed
since GCC 4.9. Would it be helpful to compare against Apple gcc code, which
seems to handle the issue correctly?

[Bug target/110044] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |iains at gcc dot gnu.org
   Last reconfirmed|                            |2023-05-31
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Eric Gallager from comment #2)
> possible dup of either bug 60972 and/or bug 68160?

Are those bugs still current?

If so, I suspect that they are not DUPs because:

They do not indicate the target - but the code that Sergey has attached passes
everywhere (on Darwin, at least) except 32b powerpc (which makes it quite
likely related to some mistake in darwin_rs6000_special_round_type_align() as
Andrew suggests).

[Bug target/110044] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[iains at gcc dot gnu.org]

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

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Sergey Fedorov from comment #3)
> (In reply to Eric Gallager from comment #2)
> > possible dup of either bug 60972 and/or bug 68160?
> 
> From those topics it looks that the bug, if identical, has never been
> addressed since GCC 4.9. Would it be helpful to compare against Apple gcc
> code, which seems to handle the issue correctly?

maybe - but the darwin_rs6000_special_round_type_align() has had to change from
the original (at least recently) to cater for the new C++ rules (which did not
exist when gcc-4.2.1 was current).

.. it's on my TODO to take a look...

[Bug target/110044] [10, 11, 12, 13, 14 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.5

[Bug target/110044] [10, 11, 12, 13, 14 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[iains at gcc dot gnu.org]

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

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
I'm going to test the following (which will take some time since the hardware
is needed for testing releases too).

The test for AGGREGATE_TYPE_P() could actually be changed to
RECORD_OR_UNION_TYPE_P () - since the case that we might have an array is
handled for non-empty structs (but we do need to guard the empty struct case). 
The problem seems to be we were ignoring that the field type could be packed or
that there was a cap on the max alignment.

If it works as expected then I will apply to the open branches (hopefully
before 10.5 is spun).

----

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 5b3b8b52e7e..e1c038da305 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -8209,7 +8209,8 @@ darwin_rs6000_special_round_type_align (tree type,
unsigned int computed,
       type = TREE_TYPE (type);
   } while (AGGREGATE_TYPE_P (type));

-  if (! AGGREGATE_TYPE_P (type) && type != error_mark_node)
+  if (type != error_mark_node && ! AGGREGATE_TYPE_P (type)
+      ! TYPE_PACKED(type) && maximum_field_alignment == 0)
     align = MAX (align, TYPE_ALIGN (type));

   return align;

[Bug target/110044] [10, 11, 12, 13, 14 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[vital.had at gmail dot com]

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

--- Comment #7 from Sergey Fedorov <vital.had at gmail dot com> ---
(In reply to Iain Sandoe from comment #6)
> I'm going to test the following (which will take some time since the
> hardware is needed for testing releases too).
> 
> The test for AGGREGATE_TYPE_P() could actually be changed to
> RECORD_OR_UNION_TYPE_P () - since the case that we might have an array is
> handled for non-empty structs (but we do need to guard the empty struct
> case).  The problem seems to be we were ignoring that the field type could
> be packed or that there was a cap on the max alignment.
> 
> If it works as expected then I will apply to the open branches (hopefully
> before 10.5 is spun).
> 
> ----
> 
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 5b3b8b52e7e..e1c038da305 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -8209,7 +8209,8 @@ darwin_rs6000_special_round_type_align (tree type,
> unsigned int computed,
>        type = TREE_TYPE (type);
>    } while (AGGREGATE_TYPE_P (type));
>  
> -  if (! AGGREGATE_TYPE_P (type) && type != error_mark_node)
> +  if (type != error_mark_node && ! AGGREGATE_TYPE_P (type)
> +      ! TYPE_PACKED(type) && maximum_field_alignment == 0)
>      align = MAX (align, TYPE_ALIGN (type));
>  
>    return align;

Thank you for dealing with this!

[Bug target/110044] [10/11/12/13/14 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:84d080a29a780973bef47171ba708ae2f7b4ee47

commit r14-1506-g84d080a29a780973bef47171ba708ae2f7b4ee47
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jun 1 13:43:35 2023 +0100

    Darwin, PPC: Fix struct layout with pragma pack [PR110044].

    This bug was essentially that darwin_rs6000_special_round_type_align()
    was ignoring externally-imposed capping of field alignment.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR target/110044

    gcc/ChangeLog:

            * config/rs6000/rs6000.cc (darwin_rs6000_special_round_type_align):
            Make sure that we do not have a cap on field alignment before
altering
            the struct layout based on the type alignment of the first entry.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/darwin-abi-13-0.c: New test.
            * gcc.target/powerpc/darwin-abi-13-1.c: New test.
            * gcc.target/powerpc/darwin-abi-13-2.c: New test.
            * gcc.target/powerpc/darwin-structs-0.h: New test.

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:46e585c5c3f8ea81c163e1d7db044a972bc29321

commit r13-7432-g46e585c5c3f8ea81c163e1d7db044a972bc29321
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jun 1 13:43:35 2023 +0100

    Darwin, PPC: Fix struct layout with pragma pack [PR110044].

    This bug was essentially that darwin_rs6000_special_round_type_align()
    was ignoring externally-imposed capping of field alignment.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR target/110044

    gcc/ChangeLog:

            * config/rs6000/rs6000.cc (darwin_rs6000_special_round_type_align):
            Make sure that we do not have a cap on field alignment before
altering
            the struct layout based on the type alignment of the first entry.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/darwin-abi-13-0.c: New test.
            * gcc.target/powerpc/darwin-abi-13-1.c: New test.
            * gcc.target/powerpc/darwin-abi-13-2.c: New test.
            * gcc.target/powerpc/darwin-structs-0.h: New test.

    (cherry picked from commit 84d080a29a780973bef47171ba708ae2f7b4ee47)

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:8d64b55db0bec62873503739fabc2a4d3219ad58

commit r12-9688-g8d64b55db0bec62873503739fabc2a4d3219ad58
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jun 1 13:43:35 2023 +0100

    Darwin, PPC: Fix struct layout with pragma pack [PR110044].

    This bug was essentially that darwin_rs6000_special_round_type_align()
    was ignoring externally-imposed capping of field alignment.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR target/110044

    gcc/ChangeLog:

            * config/rs6000/rs6000.cc (darwin_rs6000_special_round_type_align):
            Make sure that we do not have a cap on field alignment before
altering
            the struct layout based on the type alignment of the first entry.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/darwin-abi-13-0.c: New test.
            * gcc.target/powerpc/darwin-abi-13-1.c: New test.
            * gcc.target/powerpc/darwin-abi-13-2.c: New test.
            * gcc.target/powerpc/darwin-structs-0.h: New test.

    (cherry picked from commit 84d080a29a780973bef47171ba708ae2f7b4ee47)

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:92293d71135f8a4ad4097e971122ce3153de73a8

commit r11-10851-g92293d71135f8a4ad4097e971122ce3153de73a8
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jun 1 13:43:35 2023 +0100

    Darwin, PPC: Fix struct layout with pragma pack [PR110044].

    This bug was essentially that darwin_rs6000_special_round_type_align()
    was ignoring externally-imposed capping of field alignment.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR target/110044

    gcc/ChangeLog:

            * config/rs6000/rs6000.c (darwin_rs6000_special_round_type_align):
            Make sure that we do not have a cap on field alignment before
altering
            the struct layout based on the type alignment of the first entry.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/darwin-abi-13-0.c: New test.
            * gcc.target/powerpc/darwin-abi-13-1.c: New test.
            * gcc.target/powerpc/darwin-abi-13-2.c: New test.
            * gcc.target/powerpc/darwin-structs-0.h: New test.

    (cherry picked from commit 84d080a29a780973bef47171ba708ae2f7b4ee47)

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Iain D Sandoe
<iains@gcc.gnu.org>:

https://gcc.gnu.org/g:dfbf62f87824ac2bd40222bd9c1cbc8d65875a9c

commit r10-11442-gdfbf62f87824ac2bd40222bd9c1cbc8d65875a9c
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Thu Jun 1 13:43:35 2023 +0100

    Darwin, PPC: Fix struct layout with pragma pack [PR110044].

    This bug was essentially that darwin_rs6000_special_round_type_align()
    was ignoring externally-imposed capping of field alignment.

    Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>

            PR target/110044

    gcc/ChangeLog:

            * config/rs6000/rs6000.c (darwin_rs6000_special_round_type_align):
            Make sure that we do not have a cap on field alignment before
altering
            the struct layout based on the type alignment of the first entry.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/darwin-abi-13-0.c: New test.
            * gcc.target/powerpc/darwin-abi-13-1.c: New test.
            * gcc.target/powerpc/darwin-abi-13-2.c: New test.
            * gcc.target/powerpc/darwin-structs-0.h: New test.

    (cherry picked from commit 84d080a29a780973bef47171ba708ae2f7b4ee47)

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[iains at gcc dot gnu.org]

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

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #13 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed on open branches (should be back-portable to earlier if anyone cares).

[Bug target/110044] [10/11/12/13 Regression] #pragma pack(push, 1) may not force packing, while __attribute__((packed, aligned(1))) works

[vital.had at gmail dot com]

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

--- Comment #14 from Sergey Fedorov <vital.had at gmail dot com> ---
(In reply to Iain Sandoe from comment #13)
> fixed on open branches (should be back-portable to earlier if anyone cares).

Awesome, thanks!