On Sun, Jan 13, 2019 at 5:20 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> On Sun, Jan 13, 2019 at 04:48:38AM -0800, H.J. Lu wrote:
> > When checking alignment of packed member, we should only check the
> > non-pointer data member.
> >
> > gcc/c-family/
> >
> > PR c++/88664
> > * c-family/c-warn.c (check_alignment_of_packed_member): Only
> > check the non-pointer data member.
>
> Doesn't that mean we'd stop warning about:
> struct S { short s; void *p; } __attribute__ ((__packed__));
>
> void **
> foo (struct S *x)
> {
> return static_cast <void **> (&x->p);
> }
> where we do want to warn?
> What always matters is whether we take address of a packed structure
> field/non-static data member or whether we just read that field.
> The former should be warned about, the latter not.
>
How about this patch? It checks if address is taken with NOP.
--
H.J.
From 2ffb941641a5eb9ce02136b3afbff35a433b3edf Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Sat, 12 Jan 2019 21:03:50 -0800
Subject: [PATCH] Don't warn address of packed member if address isn't taken
Properly strip NOPS and don't warn address of packed member if address
isn't taken with NOPS.
gcc/c-family/
PR c/51628
PR c/88664
* c-warn.c (warn_for_address_or_pointer_of_packed_member): Move
NOP_EXPR check to ...
(check_and_warn_address_of_packed_member): Here. Don't warn if
address isn't taken.
gcc/testsuite/
PR c/51628
PR c/88664
* c-c++-common/pr51628-33.c: New test.
* c-c++-common/pr88664-1.c: Likewise.
* c-c++-common/pr88664-2.c: Likewise.
---
gcc/c-family/c-warn.c | 11 ++++++++---
gcc/testsuite/c-c++-common/pr51628-33.c | 19 +++++++++++++++++++
gcc/testsuite/c-c++-common/pr88664-1.c | 20 ++++++++++++++++++++
gcc/testsuite/c-c++-common/pr88664-2.c | 22 ++++++++++++++++++++++
4 files changed, 69 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/pr51628-33.c
create mode 100644 gcc/testsuite/c-c++-common/pr88664-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr88664-2.c
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 79b2d8ad449..61e5b76bba4 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2755,6 +2755,14 @@ check_and_warn_address_of_packed_member (tree type, tree rhs)
while (TREE_CODE (rhs) == COMPOUND_EXPR)
rhs = TREE_OPERAND (rhs, 1);
+ if (TREE_CODE (rhs) == NOP_EXPR)
+ {
+ STRIP_NOPS (rhs);
+ /* For NOP_EXPR, address is taken only with ADDR_EXPR. */
+ if (TREE_CODE (rhs) != ADDR_EXPR)
+ return;
+ }
+
tree context = check_address_of_packed_member (type, rhs);
if (context)
{
@@ -2844,9 +2852,6 @@ warn_for_address_or_pointer_of_packed_member (bool convert_p, tree type,
/* 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);
}
}
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/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 } */
+}
--
2.20.1
