On 1/23/19 4:22 PM, Jakub Jelinek wrote:
> On Tue, Jan 22, 2019 at 02:10:38PM +0000, Bernd Edlinger wrote:
>> --- gcc/c-family/c-warn.c (revision 268119)
>> +++ gcc/c-family/c-warn.c (working copy)
>> @@ -2796,6 +2796,10 @@ check_address_or_pointer_of_packed_membe
>> if (context)
>> break;
>> }
>> + if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
>> + rvalue = false;
>> + if (rvalue)
>> + return NULL_TREE;
>
> That looks like ARRAY_REF specific stuff, isn't it? We have various other
> handled components, does e.g. REALPART_EXPR/IMAGPART_EXPR need that?
> What about VIEW_CONVERT_EXPR? Or is that something you want to do for
> all of them? In any case, there should be testcases with _Complex and
> __real__/__imag__, address of that as well as value.
>
>> @@ -52,4 +54,6 @@ void foo (void)
>> i1 = t0.d; /* { dg-warning "may result in an unaligned
>> pointer value" } */
>> i1 = t1->d; /* { dg-warning "may result in an unaligned
>> pointer value" } */
>> i1 = t10[0].d; /* { dg-warning "may result in an unaligned
>> pointer value" } */
>> + i1 = (int*) &t10[0].e[0]; /* { dg-warning "may result in an unaligned
>> pointer value" } */
>> + i1 = (int*) t10[0].e; /* { dg-warning "may result in an unaligned
>> pointer value" } */
>
> Why not also i1 = &t10[0].e[0];
> and i1 = t10[0].e; tests?
>
> Unrelated to this patch, I'm worried e.g. about
> if (INDIRECT_REF_P (rhs))
> rhs = TREE_OPERAND (rhs, 0);
>
> at the start of the check_address_or_pointer_of_packed_member
> function, what if we have:
> i1 = *&t0.c;
> Do we want to warn when in the end we don't care if the address is unaligned
> or not, this is folded eventually into t0.c.
>
> Plus as I said earlier to H.J., I think
> bool nop_p;
>
> while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
>
> tree orig_rhs = rhs;
> STRIP_NOPS (rhs);
> nop_p = orig_rhs != rhs;
> should be really:
> bool nop_p;
> tree orig_rhs = rhs;
> STRIP_NOPS (rhs);
> nop_p = orig_rhs != rhs;
>
> while (TREE_CODE (rhs) == COMPOUND_EXPR)
> rhs = TREE_OPERAND (rhs, 1);
>
> orig_rhs = rhs;
> STRIP_NOPS (rhs);
> nop_p |= orig_rhs != rhs;
>
> or similar, because if there are casts around COMPOUND_EXPR, it doesn't than
> look through those COMPOUND_EXPRs, and if e.g. in the COMPOUND_EXPR rhs
> there is then COND_EXPR or similar, it should handle that case.
>
Okay, I updated the patch to address your comments.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
Index: gcc/c-family/c-warn.c
===================================================================
--- gcc/c-family/c-warn.c (revision 268195)
+++ gcc/c-family/c-warn.c (working copy)
@@ -2725,14 +2725,18 @@ static tree
check_address_or_pointer_of_packed_member (tree type, tree rhs)
{
bool rvalue = true;
+ bool indirect = false;
if (INDIRECT_REF_P (rhs))
- rhs = TREE_OPERAND (rhs, 0);
+ {
+ rhs = TREE_OPERAND (rhs, 0);
+ indirect = true;
+ }
if (TREE_CODE (rhs) == ADDR_EXPR)
{
rhs = TREE_OPERAND (rhs, 0);
- rvalue = false;
+ rvalue = indirect;
}
if (!POINTER_TYPE_P (type))
@@ -2796,6 +2800,10 @@ check_address_or_pointer_of_packed_member (tree ty
if (context)
break;
}
+ if (TREE_CODE (TREE_TYPE (rhs)) == ARRAY_TYPE)
+ rvalue = false;
+ if (rvalue)
+ return NULL_TREE;
rhs = TREE_OPERAND (rhs, 0);
}
@@ -2811,15 +2819,19 @@ check_address_or_pointer_of_packed_member (tree ty
static void
check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
{
- bool nop_p;
+ bool nop_p = false;
+ tree orig_rhs;
- while (TREE_CODE (rhs) == COMPOUND_EXPR)
- rhs = TREE_OPERAND (rhs, 1);
+ do
+ {
+ while (TREE_CODE (rhs) == COMPOUND_EXPR)
+ rhs = TREE_OPERAND (rhs, 1);
+ orig_rhs = rhs;
+ STRIP_NOPS (rhs);
+ nop_p |= orig_rhs != rhs;
+ }
+ while (orig_rhs != rhs);
- tree orig_rhs = rhs;
- STRIP_NOPS (rhs);
- nop_p = orig_rhs != rhs;
-
if (TREE_CODE (rhs) == COND_EXPR)
{
/* Check the THEN path. */
Index: gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
===================================================================
--- gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (revision 268195)
+++ gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (working copy)
@@ -6,6 +6,8 @@ struct t {
int b;
int *c;
int d[10];
+ int *e[1];
+ _Complex float f;
} __attribute__((packed));
struct t t0;
@@ -17,6 +19,8 @@ struct t *bar();
struct t (*baz())[10];
struct t (*bazz())[10][10];
int *i1;
+int **i2;
+float f0, *f1;
__UINTPTR_TYPE__ u1;
__UINTPTR_TYPE__ baa();
@@ -40,6 +44,13 @@ void foo (void)
i1 = t10[0].c; /* { dg-bogus "may result in an unaligned pointer value" } */
u1 = (__UINTPTR_TYPE__) &t0; /* { dg-bogus "may result in an unaligned pointer value" } */
u1 = (__UINTPTR_TYPE__) t1; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = t10[0].e[0]; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = *&t0.c; /* { dg-bogus "may result in an unaligned pointer value" } */
+ f0 = __real__ t0.f; /* { dg-bogus "may result in an unaligned pointer value" } */
+ f0 = __imag__ t0.f; /* { dg-bogus "may result in an unaligned pointer value" } */
+ f0 = *&__real__ t0.f; /* { dg-bogus "may result in an unaligned pointer value" } */
+ f0 = *&__imag__ t0.f; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = (&t0.c, (int*) 0); /* { dg-bogus "may result in an unaligned pointer value" } */
t2 = (struct t**) t10; /* { dg-warning "may result in an unaligned pointer value" } */
t2 = (struct t**) t100; /* { dg-warning "may result in an unaligned pointer value" } */
t2 = (struct t**) t1; /* { dg-warning "may result in an unaligned pointer value" } */
@@ -52,4 +63,14 @@ void foo (void)
i1 = t0.d; /* { dg-warning "may result in an unaligned pointer value" } */
i1 = t1->d; /* { dg-warning "may result in an unaligned pointer value" } */
i1 = t10[0].d; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = (int*) &t10[0].e[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = (int*) t10[0].e; /* { dg-warning "may result in an unaligned pointer value" } */
+ i2 = &t10[0].e[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i2 = t10[0].e; /* { dg-warning "may result in an unaligned pointer value" } */
+ i2 = &*&t0.c; /* { dg-warning "may result in an unaligned pointer value" } */
+ f1 = &__real__ t0.f; /* { dg-warning "may result in an unaligned pointer value" } */
+ f1 = &__imag__ t0.f; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = (0, (int*) &t0.c); /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = (int*) (0, &t0.c); /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = (0, (int*)(0, &t0.c));/* { dg-warning "may result in an unaligned pointer value" } */
}
Index: gcc/testsuite/c-c++-common/Waddress-of-packed-member-2.c
===================================================================
--- gcc/testsuite/c-c++-common/Waddress-of-packed-member-2.c (revision 0)
+++ gcc/testsuite/c-c++-common/Waddress-of-packed-member-2.c (working copy)
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-Waddress-of-packed-member" } */
+
+struct r {
+ int a[10];
+ int b[10][10];
+};
+
+struct s {
+ char c;
+ struct r p;
+} __attribute__((packed));
+
+struct t {
+ char c;
+ struct r p __attribute__((packed));
+ struct r u;
+};
+
+struct s s0;
+struct t t0;
+int *i0;
+
+void foo (void)
+{
+ i0 = s0.p.a; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = t0.p.a; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = s0.p.b[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = t0.p.b[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = &s0.p.a[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = &t0.p.a[0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = &s0.p.b[0][0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = &t0.p.b[0][0]; /* { dg-warning "may result in an unaligned pointer value" } */
+ i0 = t0.u.a; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i0 = t0.u.b[0]; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i0 = &t0.u.a[0]; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i0 = &t0.u.b[0][0]; /* { dg-bogus "may result in an unaligned pointer value" } */
+}
