| Message ID | 20190117045725.GA1119@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | V2 [PATCH] c-family: Update unaligned adress of packed member check | expand |
On Wed, Jan 16, 2019 at 8:57 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Wed, Jan 16, 2019 at 01:28:26PM +0100, Jakub Jelinek wrote: > > On Wed, Jan 16, 2019 at 04:11:44AM -0800, H.J. Lu wrote: > > > > Why? What is so special about C and (implicit?) casts where the rhs isn't > > > > ADDR_EXPR? Aren't all casts (explicit or implicit) from one pointer type > > > > to another pointer and satisfy the rules something that should be warned > > > > > > -Wincompatible-pointer-types is C only. In C++, incompatible pointer types > > > aren't allowed at all. > > > > How so? You can certainly: > > struct B { int i; }; > > struct C { struct B b; } __attribute__ ((packed)); > > > > extern struct C *p; > > long* g8 (void) { return (long *)p; } > > > > and similarly for C. So, why is explicit cast something that shouldn't > > be warned about in this case and implicit cast should get a warning, > > especially when it already does get one (and one even enabled by default, > > -Wincompatible-pointer-types)? > > Either such casts are problematic and then it should treat explicit and > > implicit casts the same, or they aren't, and then > > -Wincompatible-pointer-types is all you need. > > > > I am testing this patch. > There is no regression. > H.J. > --- > 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. > --- > gcc/c-family/c-common.h | 2 +- > gcc/c-family/c-warn.c | 154 +++++++++++++----------- > gcc/c/c-typeck.c | 6 +- > gcc/cp/call.c | 2 +- > gcc/cp/typeck.c | 2 +- > gcc/testsuite/c-c++-common/pr51628-33.c | 19 +++ > gcc/testsuite/c-c++-common/pr51628-35.c | 15 +++ > gcc/testsuite/c-c++-common/pr88664-1.c | 20 +++ > gcc/testsuite/c-c++-common/pr88664-2.c | 22 ++++ > gcc/testsuite/gcc.dg/pr51628-34.c | 25 ++++ > 10 files changed, 190 insertions(+), 77 deletions(-) > create mode 100644 gcc/testsuite/c-c++-common/pr51628-33.c > create mode 100644 gcc/testsuite/c-c++-common/pr51628-35.c > create mode 100644 gcc/testsuite/c-c++-common/pr88664-1.c > create mode 100644 gcc/testsuite/c-c++-common/pr88664-2.c > create mode 100644 gcc/testsuite/gcc.dg/pr51628-34.c > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h > index db16ae94b64..460954fefd8 100644 > --- a/gcc/c-family/c-common.h > +++ b/gcc/c-family/c-common.h > @@ -1284,7 +1284,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool, > bool); > extern void warn_for_omitted_condop (location_t, tree); > extern bool warn_for_restrict (unsigned, tree *, unsigned); > -extern void warn_for_address_or_pointer_of_packed_member (bool, tree, tree); > +extern void warn_for_address_or_pointer_of_packed_member (tree, tree); > > /* Places where an lvalue, or modifiable lvalue, may be required. > Used to select diagnostic messages in lvalue_error and > diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c > index 79b2d8ad449..5df17ba2e1b 100644 > --- a/gcc/c-family/c-warn.c > +++ b/gcc/c-family/c-warn.c > @@ -2713,12 +2713,14 @@ check_alignment_of_packed_member (tree type, tree field) > return NULL_TREE; > } > > -/* Return struct or union type if the right hand value, RHS, takes the > - unaligned address of packed member of struct or union when assigning > - to TYPE. Otherwise, return NULL_TREE. */ > +/* Return struct or union type if the right hand value, RHS: > + 1. Is a pointer value which isn't aligned to a pointer type TYPE. > + 2. Is an address which takes the unaligned address of packed member > + of struct or union when assigning to TYPE. > + Otherwise, return NULL_TREE. */ > > static tree > -check_address_of_packed_member (tree type, tree rhs) > +check_address_or_pointer_of_packed_member (tree type, tree rhs) > { > if (INDIRECT_REF_P (rhs)) > rhs = TREE_OPERAND (rhs, 0); > @@ -2726,6 +2728,36 @@ check_address_of_packed_member (tree type, tree rhs) > if (TREE_CODE (rhs) == ADDR_EXPR) > rhs = TREE_OPERAND (rhs, 0); > > + if (!TYPE_P (type) || POINTER_TYPE_P (type)) > + type = TREE_TYPE (type); > + > + if (TREE_CODE (rhs) == PARM_DECL || TREE_CODE (rhs) == VAR_DECL) > + { > + tree rhstype = TREE_TYPE (rhs); > + if ((POINTER_TYPE_P (rhstype) > + || TREE_CODE (rhstype) == ARRAY_TYPE) > + && TYPE_PACKED (TREE_TYPE (rhstype))) > + { > + unsigned int type_align = TYPE_ALIGN_UNIT (type); > + unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype)); > + if ((rhs_align % type_align) != 0) > + { > + location_t location = EXPR_LOC_OR_LOC (rhs, input_location); > + warning_at (location, OPT_Waddress_of_packed_member, > + "converting a packed %qT pointer (alignment %d) " > + "to %qT (alignment %d) may result in an " > + "unaligned pointer value", > + rhstype, rhs_align, type, type_align); > + tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype)); > + inform (DECL_SOURCE_LOCATION (decl), "defined here"); > + decl = TYPE_STUB_DECL (type); > + if (decl) > + inform (DECL_SOURCE_LOCATION (decl), "defined here"); > + } > + } > + return NULL_TREE; > + } > + > tree context = NULL_TREE; > > /* Check alignment of the object. */ > @@ -2744,18 +2776,53 @@ check_address_of_packed_member (tree type, tree rhs) > return context; > } > > -/* Check and warn if the right hand value, RHS, takes the unaligned > - address of packed member of struct or union when assigning to TYPE. */ > +/* Check and warn if the right hand value, RHS: > + 1. Is a pointer value which isn't aligned to a pointer type TYPE. > + 2. Is an address which takes the unaligned address of packed member > + of struct or union when assigning to TYPE. > + */ > > static void > -check_and_warn_address_of_packed_member (tree type, tree rhs) > +check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs) > { > + bool nop_p; > + > + if (TREE_CODE (rhs) == NOP_EXPR) > + { > + STRIP_NOPS (rhs); > + nop_p = true; > + } > + else > + nop_p = false; > + > if (TREE_CODE (rhs) != COND_EXPR) > { > while (TREE_CODE (rhs) == COMPOUND_EXPR) > rhs = TREE_OPERAND (rhs, 1); > > - tree context = check_address_of_packed_member (type, rhs); > + if (TREE_CODE (rhs) == NOP_EXPR) > + { > + STRIP_NOPS (rhs); > + nop_p = true; > + } > + > + if (nop_p) > + { > + switch (TREE_CODE (rhs)) > + { > + case ADDR_EXPR: > + /* Address is taken. */ > + case PARM_DECL: > + case VAR_DECL: > + /* Pointer conversion. */ > + break; > + default: > + return; > + } > + } > + > + tree context > + = check_address_or_pointer_of_packed_member (type, rhs); > if (context) > { > location_t loc = EXPR_LOC_OR_LOC (rhs, input_location); > @@ -2768,22 +2835,22 @@ check_and_warn_address_of_packed_member (tree type, tree rhs) > } > > /* Check the THEN path. */ > - check_and_warn_address_of_packed_member (type, TREE_OPERAND (rhs, 1)); > + check_and_warn_address_or_pointer_of_packed_member (type, > + TREE_OPERAND (rhs, 1)); > > /* Check the ELSE path. */ > - check_and_warn_address_of_packed_member (type, TREE_OPERAND (rhs, 2)); > + check_and_warn_address_or_pointer_of_packed_member (type, > + TREE_OPERAND (rhs, 2)); > } > > /* Warn if the right hand value, RHS: > - 1. For CONVERT_P == true, is a pointer value which isn't aligned to a > - pointer type TYPE. > - 2. For CONVERT_P == false, is an address which takes the unaligned > - address of packed member of struct or union when assigning to TYPE. > + 1. Is a pointer value which isn't aligned to a pointer type TYPE. > + 2. Is an address which takes the unaligned address of packed member > + of struct or union when assigning to TYPE. > */ > > void > -warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type, > - tree rhs) > +warn_for_address_or_pointer_of_packed_member (tree type, tree rhs) > { > if (!warn_address_of_packed_member) > return; > @@ -2795,58 +2862,5 @@ warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type, > while (TREE_CODE (rhs) == COMPOUND_EXPR) > rhs = TREE_OPERAND (rhs, 1); > > - if (convert_p) > - { > - bool rhspointer_p; > - tree rhstype; > - > - /* Check the original type of RHS. */ > - switch (TREE_CODE (rhs)) > - { > - case PARM_DECL: > - case VAR_DECL: > - rhstype = TREE_TYPE (rhs); > - rhspointer_p = POINTER_TYPE_P (rhstype); > - break; > - case NOP_EXPR: > - rhs = TREE_OPERAND (rhs, 0); > - if (TREE_CODE (rhs) == ADDR_EXPR) > - rhs = TREE_OPERAND (rhs, 0); > - rhstype = TREE_TYPE (rhs); > - rhspointer_p = TREE_CODE (rhstype) == ARRAY_TYPE; > - break; > - default: > - return; > - } > - > - if (rhspointer_p && TYPE_PACKED (TREE_TYPE (rhstype))) > - { > - unsigned int type_align = TYPE_ALIGN_UNIT (TREE_TYPE (type)); > - unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype)); > - if ((rhs_align % type_align) != 0) > - { > - location_t location = EXPR_LOC_OR_LOC (rhs, input_location); > - warning_at (location, OPT_Waddress_of_packed_member, > - "converting a packed %qT pointer (alignment %d) " > - "to %qT (alignment %d) may result in an " > - "unaligned pointer value", > - rhstype, rhs_align, type, type_align); > - tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype)); > - inform (DECL_SOURCE_LOCATION (decl), "defined here"); > - decl = TYPE_STUB_DECL (TREE_TYPE (type)); > - if (decl) > - inform (DECL_SOURCE_LOCATION (decl), "defined here"); > - } > - } > - } > - else > - { > - /* Get the type of the pointer pointing to. */ > - type = TREE_TYPE (type); > - > - if (TREE_CODE (rhs) == NOP_EXPR) > - rhs = TREE_OPERAND (rhs, 0); > - > - check_and_warn_address_of_packed_member (type, rhs); > - } > + check_and_warn_address_or_pointer_of_packed_member (type, rhs); > } > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c > index 63d177f7a6f..05e171e4bda 100644 > --- a/gcc/c/c-typeck.c > +++ b/gcc/c/c-typeck.c > @@ -6725,8 +6725,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, > > if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (rhstype)) > { > - warn_for_address_or_pointer_of_packed_member (false, type, > - orig_rhs); > + warn_for_address_or_pointer_of_packed_member (type, orig_rhs); > return rhs; > } > > @@ -7285,8 +7284,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, > > /* If RHS isn't an address, check pointer or array of packed > struct or union. */ > - warn_for_address_or_pointer_of_packed_member > - (TREE_CODE (orig_rhs) != ADDR_EXPR, type, orig_rhs); > + warn_for_address_or_pointer_of_packed_member (type, orig_rhs); > > return convert (type, rhs); > } > diff --git a/gcc/cp/call.c b/gcc/cp/call.c > index 8bc8566e8d6..3b937d059d0 100644 > --- a/gcc/cp/call.c > +++ b/gcc/cp/call.c > @@ -7631,7 +7631,7 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain) > } > > if (complain & tf_warning) > - warn_for_address_or_pointer_of_packed_member (false, type, val); > + warn_for_address_or_pointer_of_packed_member (type, val); > > return val; > } > diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c > index 43d2899a3c4..9f5b2ec77e9 100644 > --- a/gcc/cp/typeck.c > +++ b/gcc/cp/typeck.c > @@ -9074,7 +9074,7 @@ convert_for_assignment (tree type, tree rhs, > } > > if (complain & tf_warning) > - warn_for_address_or_pointer_of_packed_member (false, type, rhs); > + warn_for_address_or_pointer_of_packed_member (type, rhs); > > return perform_implicit_conversion_flags (strip_top_quals (type), rhs, > complain, flags); > diff --git a/gcc/testsuite/c-c++-common/pr51628-33.c b/gcc/testsuite/c-c++-common/pr51628-33.c > new file mode 100644 > index 00000000000..0092f32202f > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr51628-33.c > @@ -0,0 +1,19 @@ > +/* PR c/51628. */ > +/* { dg-do compile } */ > +/* { dg-options "-O" } */ > + > +struct pair_t > +{ > + char x; > + int i[4]; > +} __attribute__ ((packed, aligned (4))); > + > +extern struct pair_t p; > +extern void bar (int *); > + > +void > +foo (struct pair_t *p) > +{ > + bar (p ? p->i : (int *) 0); > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c b/gcc/testsuite/c-c++-common/pr51628-35.c > new file mode 100644 > index 00000000000..597f1b7d15f > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr51628-35.c > @@ -0,0 +1,15 @@ > +/* PR c/51628. */ > +/* { dg-do compile } */ > +/* { dg-options "-O" } */ > + > +struct B { int i; }; > +struct C { struct B b; } __attribute__ ((packed)); > + > +extern struct C *p; > + > +long * > +foo (void) > +{ > + return (long *)p; > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > diff --git a/gcc/testsuite/c-c++-common/pr88664-1.c b/gcc/testsuite/c-c++-common/pr88664-1.c > new file mode 100644 > index 00000000000..5e680b9ae90 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr88664-1.c > @@ -0,0 +1,20 @@ > +/* PR c/88664. */ > +/* { dg-do compile } */ > +/* { dg-options "-O" } */ > + > +struct data > +{ > + void *ptr; > +} __attribute__((packed)); > + > +int * > +fun1 (struct data *p) > +{ > + return (int *) p->ptr; > +} > + > +int * > +fun2 (struct data *p, int *x) > +{ > + return x ? (*x = 1, (int *) p->ptr) : (int *) 0; > +} > diff --git a/gcc/testsuite/c-c++-common/pr88664-2.c b/gcc/testsuite/c-c++-common/pr88664-2.c > new file mode 100644 > index 00000000000..d2d880a66d7 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/pr88664-2.c > @@ -0,0 +1,22 @@ > +/* PR c/88664. */ > +/* { dg-do compile } */ > +/* { dg-options "-O" } */ > + > +struct data > +{ > + void *ptr; > +} __attribute__((packed)); > + > +void ** > +fun1 (struct data *p) > +{ > + return &p->ptr; > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > + > +int * > +fun2 (struct data *p, int *x) > +{ > + return p ? (*x = 1, (int *) &p->ptr) : (int *) 0; > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > diff --git a/gcc/testsuite/gcc.dg/pr51628-34.c b/gcc/testsuite/gcc.dg/pr51628-34.c > new file mode 100644 > index 00000000000..51d4b26a114 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr51628-34.c > @@ -0,0 +1,25 @@ > +/* PR c/51628. */ > +/* { dg-do compile } */ > +/* { dg-options "-O -Wno-incompatible-pointer-types" } */ > + > +struct __attribute__((packed)) S { char p; int a, b, c; }; > + > +short * > +baz (int x, struct S *p) > +{ > + return (x > + ? &p->a > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > + : &p->b); > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > + > +short * > +qux (int x, struct S *p) > +{ > + return (short *) (x > + ? &p->a > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > + : &p->b); > +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ > +} > -- > 2.20.1 >
On Wed, Jan 16, 2019 at 08:57:25PM -0800, H.J. Lu wrote: > Check unaligned pointer conversion and strip NOPS. > -check_address_of_packed_member (tree type, tree rhs) > +check_address_or_pointer_of_packed_member (tree type, tree rhs) > { > if (INDIRECT_REF_P (rhs)) > rhs = TREE_OPERAND (rhs, 0); > @@ -2726,6 +2728,36 @@ check_address_of_packed_member (tree type, tree rhs) > if (TREE_CODE (rhs) == ADDR_EXPR) > rhs = TREE_OPERAND (rhs, 0); > > + if (!TYPE_P (type) || POINTER_TYPE_P (type)) > + type = TREE_TYPE (type); Bad formatting. Plus, when would you pass around a non-type here? And, isn't type always a POINTER_TYPE_P here anyway? If not, whether you use TREE_TYPE on it or not shouldn't depend on whether it is a pointer, but on some other conditions, because a field can have pointer type too, so if you come in through sometimes type being the address of the var and sometimes the type of its value, the bug is in allowing that. > + > + if (TREE_CODE (rhs) == PARM_DECL || TREE_CODE (rhs) == VAR_DECL) VAR_P (rhs) instead of TREE_CODE (rhs) == VAR_DECL. What about RESULT_DECL? > static void > -check_and_warn_address_of_packed_member (tree type, tree rhs) > +check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs) > { > + bool nop_p; > + > + if (TREE_CODE (rhs) == NOP_EXPR) This should be probably if (CONVERT_EXPR_P (rhs)) or maybe just do tree orig_rhs = rhs; STRIP_NOPS (rhs); nop_p = orig_rhs != rhs; ? I must say I don't fully understand the nop_p stuff, why you handle it differently if there were any nops vs. if there were none. And, e.g. if you have NOP_EXPR around COND_EXPR, that outer nop_p isn't taken into account. So, again, what exactly do you want to achieve, why do you care if there are any conversions in between or not. Isn't all that matters what the innermost ADDR_EXPR is and what the outermost type is? > if (TREE_CODE (rhs) != COND_EXPR) I think it would be more readable to do: if (TREE_CODE (rhs) == COND_EXPR) { recurse; recurse; return; } and handle the remaining code (longer) normally indented below that. Another thing is, the NOP_EXPRs/CONVERT_EXPRs, COMPOUND_EXPRs and COND_EXPRs can be arbitrarily nested, while you handle only a subset of those cases. You could e.g. move the while (TREE_CODE (rhs) == COMPOUND_EXPR) rhs = TREE_OPERAND (rhs, 1); before the if (TREE_CODE (rhs) == COND_EXPR) check and stick another STRIP_NOPS in between. > @@ -2795,58 +2862,5 @@ warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type, > while (TREE_CODE (rhs) == COMPOUND_EXPR) > rhs = TREE_OPERAND (rhs, 1); and then it would be pointless to do this here. Jakub
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index db16ae94b64..460954fefd8 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1284,7 +1284,7 @@ extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool, bool); extern void warn_for_omitted_condop (location_t, tree); extern bool warn_for_restrict (unsigned, tree *, unsigned); -extern void warn_for_address_or_pointer_of_packed_member (bool, tree, tree); +extern void warn_for_address_or_pointer_of_packed_member (tree, tree); /* Places where an lvalue, or modifiable lvalue, may be required. Used to select diagnostic messages in lvalue_error and diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c index 79b2d8ad449..5df17ba2e1b 100644 --- a/gcc/c-family/c-warn.c +++ b/gcc/c-family/c-warn.c @@ -2713,12 +2713,14 @@ check_alignment_of_packed_member (tree type, tree field) return NULL_TREE; } -/* Return struct or union type if the right hand value, RHS, takes the - unaligned address of packed member of struct or union when assigning - to TYPE. Otherwise, return NULL_TREE. */ +/* Return struct or union type if the right hand value, RHS: + 1. Is a pointer value which isn't aligned to a pointer type TYPE. + 2. Is an address which takes the unaligned address of packed member + of struct or union when assigning to TYPE. + Otherwise, return NULL_TREE. */ static tree -check_address_of_packed_member (tree type, tree rhs) +check_address_or_pointer_of_packed_member (tree type, tree rhs) { if (INDIRECT_REF_P (rhs)) rhs = TREE_OPERAND (rhs, 0); @@ -2726,6 +2728,36 @@ check_address_of_packed_member (tree type, tree rhs) if (TREE_CODE (rhs) == ADDR_EXPR) rhs = TREE_OPERAND (rhs, 0); + if (!TYPE_P (type) || POINTER_TYPE_P (type)) + type = TREE_TYPE (type); + + if (TREE_CODE (rhs) == PARM_DECL || TREE_CODE (rhs) == VAR_DECL) + { + tree rhstype = TREE_TYPE (rhs); + if ((POINTER_TYPE_P (rhstype) + || TREE_CODE (rhstype) == ARRAY_TYPE) + && TYPE_PACKED (TREE_TYPE (rhstype))) + { + unsigned int type_align = TYPE_ALIGN_UNIT (type); + unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype)); + if ((rhs_align % type_align) != 0) + { + location_t location = EXPR_LOC_OR_LOC (rhs, input_location); + warning_at (location, OPT_Waddress_of_packed_member, + "converting a packed %qT pointer (alignment %d) " + "to %qT (alignment %d) may result in an " + "unaligned pointer value", + rhstype, rhs_align, type, type_align); + tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype)); + inform (DECL_SOURCE_LOCATION (decl), "defined here"); + decl = TYPE_STUB_DECL (type); + if (decl) + inform (DECL_SOURCE_LOCATION (decl), "defined here"); + } + } + return NULL_TREE; + } + tree context = NULL_TREE; /* Check alignment of the object. */ @@ -2744,18 +2776,53 @@ check_address_of_packed_member (tree type, tree rhs) return context; } -/* Check and warn if the right hand value, RHS, takes the unaligned - address of packed member of struct or union when assigning to TYPE. */ +/* Check and warn if the right hand value, RHS: + 1. Is a pointer value which isn't aligned to a pointer type TYPE. + 2. Is an address which takes the unaligned address of packed member + of struct or union when assigning to TYPE. + */ static void -check_and_warn_address_of_packed_member (tree type, tree rhs) +check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs) { + bool nop_p; + + if (TREE_CODE (rhs) == NOP_EXPR) + { + STRIP_NOPS (rhs); + nop_p = true; + } + else + nop_p = false; + if (TREE_CODE (rhs) != COND_EXPR) { while (TREE_CODE (rhs) == COMPOUND_EXPR) rhs = TREE_OPERAND (rhs, 1); - tree context = check_address_of_packed_member (type, rhs); + if (TREE_CODE (rhs) == NOP_EXPR) + { + STRIP_NOPS (rhs); + nop_p = true; + } + + if (nop_p) + { + switch (TREE_CODE (rhs)) + { + case ADDR_EXPR: + /* Address is taken. */ + case PARM_DECL: + case VAR_DECL: + /* Pointer conversion. */ + break; + default: + return; + } + } + + tree context + = check_address_or_pointer_of_packed_member (type, rhs); if (context) { location_t loc = EXPR_LOC_OR_LOC (rhs, input_location); @@ -2768,22 +2835,22 @@ check_and_warn_address_of_packed_member (tree type, tree rhs) } /* Check the THEN path. */ - check_and_warn_address_of_packed_member (type, TREE_OPERAND (rhs, 1)); + check_and_warn_address_or_pointer_of_packed_member (type, + TREE_OPERAND (rhs, 1)); /* Check the ELSE path. */ - check_and_warn_address_of_packed_member (type, TREE_OPERAND (rhs, 2)); + check_and_warn_address_or_pointer_of_packed_member (type, + TREE_OPERAND (rhs, 2)); } /* Warn if the right hand value, RHS: - 1. For CONVERT_P == true, is a pointer value which isn't aligned to a - pointer type TYPE. - 2. For CONVERT_P == false, is an address which takes the unaligned - address of packed member of struct or union when assigning to TYPE. + 1. Is a pointer value which isn't aligned to a pointer type TYPE. + 2. Is an address which takes the unaligned address of packed member + of struct or union when assigning to TYPE. */ void -warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type, - tree rhs) +warn_for_address_or_pointer_of_packed_member (tree type, tree rhs) { if (!warn_address_of_packed_member) return; @@ -2795,58 +2862,5 @@ warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type, while (TREE_CODE (rhs) == COMPOUND_EXPR) rhs = TREE_OPERAND (rhs, 1); - if (convert_p) - { - bool rhspointer_p; - tree rhstype; - - /* Check the original type of RHS. */ - switch (TREE_CODE (rhs)) - { - case PARM_DECL: - case VAR_DECL: - rhstype = TREE_TYPE (rhs); - rhspointer_p = POINTER_TYPE_P (rhstype); - break; - case NOP_EXPR: - rhs = TREE_OPERAND (rhs, 0); - if (TREE_CODE (rhs) == ADDR_EXPR) - rhs = TREE_OPERAND (rhs, 0); - rhstype = TREE_TYPE (rhs); - rhspointer_p = TREE_CODE (rhstype) == ARRAY_TYPE; - break; - default: - return; - } - - if (rhspointer_p && TYPE_PACKED (TREE_TYPE (rhstype))) - { - unsigned int type_align = TYPE_ALIGN_UNIT (TREE_TYPE (type)); - unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype)); - if ((rhs_align % type_align) != 0) - { - location_t location = EXPR_LOC_OR_LOC (rhs, input_location); - warning_at (location, OPT_Waddress_of_packed_member, - "converting a packed %qT pointer (alignment %d) " - "to %qT (alignment %d) may result in an " - "unaligned pointer value", - rhstype, rhs_align, type, type_align); - tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype)); - inform (DECL_SOURCE_LOCATION (decl), "defined here"); - decl = TYPE_STUB_DECL (TREE_TYPE (type)); - if (decl) - inform (DECL_SOURCE_LOCATION (decl), "defined here"); - } - } - } - else - { - /* Get the type of the pointer pointing to. */ - type = TREE_TYPE (type); - - if (TREE_CODE (rhs) == NOP_EXPR) - rhs = TREE_OPERAND (rhs, 0); - - check_and_warn_address_of_packed_member (type, rhs); - } + check_and_warn_address_or_pointer_of_packed_member (type, rhs); } diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 63d177f7a6f..05e171e4bda 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -6725,8 +6725,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (rhstype)) { - warn_for_address_or_pointer_of_packed_member (false, type, - orig_rhs); + warn_for_address_or_pointer_of_packed_member (type, orig_rhs); return rhs; } @@ -7285,8 +7284,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type, /* If RHS isn't an address, check pointer or array of packed struct or union. */ - warn_for_address_or_pointer_of_packed_member - (TREE_CODE (orig_rhs) != ADDR_EXPR, type, orig_rhs); + warn_for_address_or_pointer_of_packed_member (type, orig_rhs); return convert (type, rhs); } diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 8bc8566e8d6..3b937d059d0 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -7631,7 +7631,7 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain) } if (complain & tf_warning) - warn_for_address_or_pointer_of_packed_member (false, type, val); + warn_for_address_or_pointer_of_packed_member (type, val); return val; } diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 43d2899a3c4..9f5b2ec77e9 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -9074,7 +9074,7 @@ convert_for_assignment (tree type, tree rhs, } if (complain & tf_warning) - warn_for_address_or_pointer_of_packed_member (false, type, rhs); + warn_for_address_or_pointer_of_packed_member (type, rhs); return perform_implicit_conversion_flags (strip_top_quals (type), rhs, complain, flags); diff --git a/gcc/testsuite/c-c++-common/pr51628-33.c b/gcc/testsuite/c-c++-common/pr51628-33.c new file mode 100644 index 00000000000..0092f32202f --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr51628-33.c @@ -0,0 +1,19 @@ +/* PR c/51628. */ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +struct pair_t +{ + char x; + int i[4]; +} __attribute__ ((packed, aligned (4))); + +extern struct pair_t p; +extern void bar (int *); + +void +foo (struct pair_t *p) +{ + bar (p ? p->i : (int *) 0); +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c b/gcc/testsuite/c-c++-common/pr51628-35.c new file mode 100644 index 00000000000..597f1b7d15f --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr51628-35.c @@ -0,0 +1,15 @@ +/* PR c/51628. */ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +struct B { int i; }; +struct C { struct B b; } __attribute__ ((packed)); + +extern struct C *p; + +long * +foo (void) +{ + return (long *)p; +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/c-c++-common/pr88664-1.c b/gcc/testsuite/c-c++-common/pr88664-1.c new file mode 100644 index 00000000000..5e680b9ae90 --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr88664-1.c @@ -0,0 +1,20 @@ +/* PR c/88664. */ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +struct data +{ + void *ptr; +} __attribute__((packed)); + +int * +fun1 (struct data *p) +{ + return (int *) p->ptr; +} + +int * +fun2 (struct data *p, int *x) +{ + return x ? (*x = 1, (int *) p->ptr) : (int *) 0; +} diff --git a/gcc/testsuite/c-c++-common/pr88664-2.c b/gcc/testsuite/c-c++-common/pr88664-2.c new file mode 100644 index 00000000000..d2d880a66d7 --- /dev/null +++ b/gcc/testsuite/c-c++-common/pr88664-2.c @@ -0,0 +1,22 @@ +/* PR c/88664. */ +/* { dg-do compile } */ +/* { dg-options "-O" } */ + +struct data +{ + void *ptr; +} __attribute__((packed)); + +void ** +fun1 (struct data *p) +{ + return &p->ptr; +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +} + +int * +fun2 (struct data *p, int *x) +{ + return p ? (*x = 1, (int *) &p->ptr) : (int *) 0; +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/pr51628-34.c b/gcc/testsuite/gcc.dg/pr51628-34.c new file mode 100644 index 00000000000..51d4b26a114 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr51628-34.c @@ -0,0 +1,25 @@ +/* PR c/51628. */ +/* { dg-do compile } */ +/* { dg-options "-O -Wno-incompatible-pointer-types" } */ + +struct __attribute__((packed)) S { char p; int a, b, c; }; + +short * +baz (int x, struct S *p) +{ + return (x + ? &p->a +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ + : &p->b); +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +} + +short * +qux (int x, struct S *p) +{ + return (short *) (x + ? &p->a +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ + : &p->b); +/* { dg-warning "may result in an unaligned pointer value" "" { target *-*-* } .-1 } */ +}