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.
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
