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
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.
possible dup of either bug 60972 and/or bug 68160?
(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?
(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).
(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...
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;
(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!
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.
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)
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)
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)
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)
fixed on open branches (should be back-portable to earlier if anyone cares).
(In reply to Iain Sandoe from comment #13) > fixed on open branches (should be back-portable to earlier if anyone cares). Awesome, thanks!