Compiling this with -Wall: ----- struct A { A(); } __attribute__((packed)); struct B { A a[2]; } __attribute__((packed)); ----- gives: test.cpp:6: warning: ignoring packed attribute on unpacked non-POD field `A B::a[2]' A non-array packed non-POD member does not produce this warning. There is also no warning if the packed attribute is removed from B and added to the array declaration inside, even though declaring it on a struct is meant to be equivalent to declaring it on each member. May be related to bug 13983.
Nathan could you comment on this bug? Confirmed.
Postponed until GCC 3.4.4.
I want to say the warning is correct but should be given more than it is currently which would rate this as a progression instead of a regression.
There's certainly a good case for warning about packing that's likely not to have the desired results - we've been bitten by that before. But that doesn't really apply to all non-POD; you can define a subset safe-non-POD (broadly speaking, no virtual functions/bases and (recursively) no unsafe non-static data members) where everything still has reasonably predictable representations and you don't need extra space for polymorphism magic. So if you do go in that direction, please consider separate warnings for those non-POD types that can nevertheless safely be packed and those that can't. Classes made non-POD through fairly innoccuous constructions - e.g. constructors, simple inheritance - are often useful in abstracting hardware registers, data formats, etc. We'd be swamped in warnings if we got one for every packed non-POD, but we definitely do want to know if there's something that's actually going to mess things up.
The killer with packing in C++ is that it is so easy to silently take the address of a field (pass by reference). Until we actually reflect unalignedness in the type system *no* packing is really safe in C++. That said, I don't see why the warning would be different for a packed non-array member.
That's orthogonal though - and presumably it only applies to a packed object or member *whose type is not packed by definition*, because you can so easily end up passing a pointer or reference to something that doesn't know about the post facto packing. Where we use packing it's always specified on the struct definition, which ought to be safe in that respect. (Although I can see that potentially some work may be required to be able to issue appropriate warnings for the unsafe uses.)
Moving to 4.0.2 pre Mark.
Leaving as P2; we should at least try to figure out whether we really want to give a warning here. (I suspect we may end up closing this PR as INVALID, because the non-PODness does affect things -- like whether the value is passed by reference or not when calling a function. But, I haven't thought hard enough to say for sure.)
(In reply to comment #4) > There's certainly a good case for warning about packing that's likely not to > have the desired results - we've been bitten by that before. But that doesn't > really apply to all non-POD; you can define a subset safe-non-POD (broadly > speaking, no virtual functions/bases and (recursively) no unsafe non-static data > members) where everything still has reasonably predictable representations and > you don't need extra space for polymorphism magic. All the more reason to warn if we don't pack these members now, but we might fix that in the future and thus change the data layout. I.e. if the packed attribute is applied (individually or as part of an entire struct / class) to a member, but ignored, and this is relevant for the size or alignment of that member, we should warn. That obviously applies for the target we are compiling for, but I think if possible, we should warn if the ignoring of the packed attribute is relevant for any possible gcc target. > > So if you do go in that direction, please consider separate warnings for those > non-POD types that can nevertheless safely be packed and those that can't. > Classes made non-POD through fairly innoccuous constructions - e.g. > constructors, simple inheritance - are often useful in abstracting hardware > registers, data formats, etc. We'd be swamped in warnings if we got one for > every packed non-POD, but we definitely do want to know if there's something > that's actually going to mess things up.
(In reply to comment #9) > (In reply to comment #4) [snip] > All the more reason to warn if we don't pack these members now, but we might > fix that in the future and thus change the data layout. > I.e. if the packed attribute is applied (individually or as part of an entire > struct / class) to a member, but ignored, and this is relevant for the size or > alignment of that member, we should warn. That obviously applies for the > target > we are compiling for, but I think if possible, we should warn if the ignoring > of the packed attribute is relevant for any possible gcc target. I've got non-POD objects that abstract endiannes and use them in arrays that represent data-packets. The non-POD objects are quite simple, they don't use inheritance or virtual methods. I use assert statements to verify the size of the data-packets and notice that packing hasn't changed. Nevertheless, I get this warning when changing from gcc 3.3 to 4.0. Is this warning given because there are other targets where the packing will be different, or because the packing might change in the future? In the first case: I don't need my program to be portable to every possible GCC target. I think it's my responsibility to ensure that it works on the relevant targets and would only want to get a warning if the packed attribute is actually ignored. In the second case: how likely is it that the packing will change in the future? I don't see any reason why my objects can't be packed but I have no knowledge of the GCC internals. In any case, can this warning be suppressed or is there a simple workaround? Since I use the asserts I'm not realy worried about missing the case when the packet attribute is ignored. Forgive me for using this bugtracking system as a helpdesk but I hope my questions contribute to the discussion. Pepijn.
Is this still an issue in 4.1/4.2. IIUC I've cleaned up the points where this warning was/was not being emitted.
(In reply to comment #11) > Is this still an issue in 4.1/4.2. IIUC I've cleaned up the points where this > warning was/was not being emitted. > Ah. I've got 4.0.3. I'll check 4.1/4.2.
This issue will not be resolved in GCC 4.1.0; retargeted at GCC 4.1.1.
Will not be fixed in 4.1.1; adjust target milestone to 4.1.2.
Subject: Bug 17519 Author: jason Date: Thu Jul 6 02:09:02 2006 New Revision: 115217 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115217 Log: PR c++/13983 PR c++/17519 * stor-layout.c (finish_record_layout): Copy TYPE_PACKED to variants. * c-common.c (handle_packed_attribute): So don't copy it here. * c-decl.c (finish_struct): Don't copy TYPE_ALIGN. * cp/class.c (check_field_decls): Check TYPE_PACKED after stripping array types. (finish_struct_bits): Don't copy TYPE_SIZE here. Added: trunk/gcc/testsuite/g++.dg/ext/packed10.C Modified: trunk/gcc/ChangeLog trunk/gcc/c-common.c trunk/gcc/c-decl.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/class.c trunk/gcc/stor-layout.c
Subject: Bug 17519 Author: jason Date: Thu Jul 6 02:21:17 2006 New Revision: 115219 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115219 Log: PR c++/13983 PR c++/17519 * stor-layout.c (finish_record_layout): Copy TYPE_PACKED to variants. * cp/class.c (check_field_decls): Check TYPE_PACKED after stripping array types. Added: branches/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/packed10.C - copied unchanged from r115217, trunk/gcc/testsuite/g++.dg/ext/packed10.C Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/cp/ChangeLog branches/gcc-4_1-branch/gcc/cp/class.c branches/gcc-4_1-branch/gcc/stor-layout.c
Subject: Bug 17519 Author: jason Date: Thu Jul 6 03:33:20 2006 New Revision: 115220 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115220 Log: PR c++/13983 PR c++/17519 * stor-layout.c (finish_record_layout): Copy TYPE_PACKED to variants. * cp/class.c (check_field_decls): Check TYPE_PACKED after stripping array types. Added: branches/gcc-4_0-branch/gcc/testsuite/g++.dg/ext/packed10.C - copied unchanged from r115219, branches/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/packed10.C Modified: branches/gcc-4_0-branch/gcc/ChangeLog branches/gcc-4_0-branch/gcc/cp/ChangeLog branches/gcc-4_0-branch/gcc/cp/class.c branches/gcc-4_0-branch/gcc/stor-layout.c
f1x0r'd
(We finally got round to throwing the switch on our next release from gcc-3.3 to gcc-4.2. It works great - the compiled code gets us significantly higher benchmark numbers and we're appreciating improved error reporting from the compiler. This PR had been the killer obstacle, so many, if belated, thanks to Jason for sorting this out.)