Created attachment 45316 [details] extracted from code using epoll_event structure nathans@devvm2452:29>./cc1plus -Waddress-of-packed-member ../../../packed.cc -quiet ../../../packed.cc: In function 'void foo(epoll_event*)': ../../../packed.cc:10:49: warning: taking address of packed member of 'epoll_event' may result in an unaligned pointer value [-Waddress-of-packed-member] 10 | int *actionable = static_cast <int *> (event->ptr); | ~~~~~~~^~~ But we're not taking the address of the packed field, we're converting the value the field holds to a different pointer. epoll_event is packed, and I imagine more code than mine squirrels away data pointers in its data field. I suppose the FE could internally be taking a reference to preserve lvalueness ...
Author: nathan Date: Wed Jan 2 15:23:56 2019 New Revision: 267516 URL: https://gcc.gnu.org/viewcvs?rev=267516&root=gcc&view=rev Log: gcc/cp/ * cxx-mapper.cc (server): Workaround PR c++/88664. Modified: branches/c++-modules/ChangeLog.modules branches/c++-modules/gcc/cp/cxx-mapper.cc
C is also affected. int* fun() { struct data { void *ptr; } __attribute__((packed)) var; return (int*)(var.ptr); } With today's gcc, git master branch: test.c: In function ‘fun’: test.c:7:22: warning: taking address of packed member of ‘struct data’ may result in an unaligned pointer value [-Waddress-of-packed-member] 7 | return (int*)(var.ptr); | ~~~~^~~~~
I'd use just struct S { short s; void *p; } __attribute__ ((__packed__)); int * foo (struct S *x) { return (int *) (x->p); } for both languages. Despite quite many tests added in the -Waddress-of-packed-member commit, I really don't see any testsuite coverage of the warning_at (location, OPT_Waddress_of_packed_member, "converting a packed %qT pointer (alignment %d) " "to %qT (alignment %d) may may result in an " "unaligned pointer value", rhstype, rhs_align, type, type_align); warning (and note the "may may" bug in the wording there), is that covered by anything? As for the other path, it doesn't care whether address is taken or not: if (INDIRECT_REF_P (rhs)) rhs = TREE_OPERAND (rhs, 0); if (TREE_CODE (rhs) == ADDR_EXPR) rhs = TREE_OPERAND (rhs, 0); while it is significant. We shouldn't warn if we are reading content from the packed structure, but should warn if we are taking address (and for that needs to take into account the array to pointer conversions, whether they happen before or after this warning is reported).
H.J., can you please look into this? Plus, I guess we need to reevaluate all the spots where -Wno-address-of-packed-member has been added, whether we were warning there correctly or not.
(In reply to Jakub Jelinek from comment #3) > I'd use just > struct S { short s; void *p; } __attribute__ ((__packed__)); > > int * > foo (struct S *x) > { > return (int *) (x->p); > } > for both languages. > > Despite quite many tests added in the -Waddress-of-packed-member commit, I > really don't see any testsuite coverage of the > warning_at (location, OPT_Waddress_of_packed_member, > "converting a packed %qT pointer (alignment %d) " > "to %qT (alignment %d) may may result in an " > "unaligned pointer value", > rhstype, rhs_align, type, type_align); > warning (and note the "may may" bug in the wording there), is that covered > by anything? This is covered by gcc.dg/pr51628-20.c, gcc.dg/pr51628-21.c and gcc.dg/pr51628-25.c. > As for the other path, it doesn't care whether address is taken or not: > if (INDIRECT_REF_P (rhs)) > rhs = TREE_OPERAND (rhs, 0); > > if (TREE_CODE (rhs) == ADDR_EXPR) > rhs = TREE_OPERAND (rhs, 0); > while it is significant. We shouldn't warn if we are reading content from > the packed structure, but should warn if we are taking address (and for that > needs to take into account the array to pointer conversions, whether they > happen before or after this warning is reported). I am testing: diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 79b2d8ad449..c937c016889 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2752,9 +2752,15 @@ check_and_warn_address_of_packed_member (tree type, tree rhs) { if (TREE_CODE (rhs) != COND_EXPR) { + if (TREE_CODE (rhs) == NOP_EXPR) + rhs = TREE_OPERAND (rhs, 0); + while (TREE_CODE (rhs) == COMPOUND_EXPR) rhs = TREE_OPERAND (rhs, 1); + if (TREE_CODE (rhs) != ADDR_EXPR) + return; + tree context = check_address_of_packed_member (type, rhs); if (context) { (
There's STRIP_NOPS, isn't there?
(In reply to Nathan Sidwell from comment #6) > There's STRIP_NOPS, isn't there? The updated patch is posted at https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00732.html
Author: hjl Date: Fri Jan 18 13:05:18 2019 New Revision: 268075 URL: https://gcc.gnu.org/viewcvs?rev=268075&root=gcc&view=rev Log: c-family: Update unaligned adress of packed member check Check unaligned pointer conversion and strip NOPS. gcc/c-family/ PR c/51628 PR c/88664 * c-common.h (warn_for_address_or_pointer_of_packed_member): Remove the boolean argument. * c-warn.c (check_address_of_packed_member): Renamed to ... (check_address_or_pointer_of_packed_member): This. Also warn pointer conversion. (check_and_warn_address_of_packed_member): Renamed to ... (check_and_warn_address_or_pointer_of_packed_member): This. Also warn pointer conversion. (warn_for_address_or_pointer_of_packed_member): Remove the boolean argument. Don't check pointer conversion here. gcc/c PR c/51628 PR c/88664 * c-typeck.c (convert_for_assignment): Upate the warn_for_address_or_pointer_of_packed_member call. gcc/cp PR c/51628 PR c/88664 * call.c (convert_for_arg_passing): Upate the warn_for_address_or_pointer_of_packed_member call. * typeck.c (convert_for_assignment): Likewise. gcc/testsuite/ PR c/51628 PR c/88664 * c-c++-common/pr51628-33.c: New test. * c-c++-common/pr51628-35.c: New test. * c-c++-common/pr88664-1.c: Likewise. * c-c++-common/pr88664-2.c: Likewise. * gcc.dg/pr51628-34.c: Likewise. Added: trunk/gcc/testsuite/c-c++-common/pr51628-33.c trunk/gcc/testsuite/c-c++-common/pr51628-35.c trunk/gcc/testsuite/c-c++-common/pr88664-1.c trunk/gcc/testsuite/c-c++-common/pr88664-2.c trunk/gcc/testsuite/gcc.dg/pr51628-34.c Modified: trunk/gcc/c-family/ChangeLog trunk/gcc/c-family/c-common.h trunk/gcc/c-family/c-warn.c trunk/gcc/c/ChangeLog trunk/gcc/c/c-typeck.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/typeck.c trunk/gcc/testsuite/ChangeLog
Fixed.