| Message ID | AM0PR07MB3874E57BD2C6B08AD2E37C07E4980@AM0PR07MB3874.eurprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refine -Waddress-of-packed-member once more | expand |
On Tue, Jan 22, 2019 at 6:10 AM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > > Hi, > > unfortunately there are some more issues with -Waddress-of-packed-member, that show that my previous > patch was not yet complete. > > That is a bogus warning, in C and C++ (see addition in test case 1), and a couple of missing warnings > in C++ (test case 2). > > The latter warning regressions were accidentally introduced by my previous patch, sorry. > > As a side note, the generic tree for an expression like struct.array is folded differently by > C and C++ FE. While C folds that to &struct.array, C++ folds it to struct.array, without the > ADDR_EXPR. That might help to explain why this needs to be done in this slightly confusing way. > > > Fixed the warning regression, and added some more test cases. While working on it, I noticed that clang didn't warn for many cases. Is that still true today? Thanks.
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. Jakub
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. > Okay, my guess it it will work with _Complex, but test cases would be straight forward, and no reason not to have at least a few. I have no idea what test case would be needed for VIEW_CONVERT_EXPR. >> @@ -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? > I will add that. > 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. > Porbably not. I tried your example, and find currently this is inconsistent between C and C++, C folds *& away, before the warning, and C++ does not and gets a warning. I feel like I want to fix that too. As a side note, your *&t0.c has a VIEW_CONVERT_EXPR in C++ but not in C: Breakpoint 1, check_address_or_pointer_of_packed_member (type=0x7ffff6eef9d8, rhs=0x7ffff702d760) at ../../gcc-trunk/gcc/c-family/c-warn.c:2727 2727 bool rvalue = true; (gdb) call debug(rhs) *&VIEW_CONVERT_EXPR<struct t>(t0).c ... but it does not affect the warning. > 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. > Ah, yes. That was not on my radar. So this works in C and C++: i1 = (0, (int*)&t0.c); But this is again inconsistent between C and C++: i1 = (int*) (0, &t0.c); C gets a warning, and C++ no warning. I agree and see that consistency betwenn C and C++ as important goal. So, I will try to fix that as well. Bernd.
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" } */ +}
On Thu, Jan 24, 2019 at 06:39:22AM +0000, Bernd Edlinger wrote: > --- 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; > } Given the unfolded *&, I wonder if the above actually shouldn't be a loop with indirection integral counter instead of a boolean and simply bump the indirection count on INDIRECT_REF_P and decrease on ADDR_EXPR, and if indirection count is > 0 after the loop return NULL_TREE? Say for *&*& or similar, and ***var.field always not warning etc. Otherwise the patch looks good. Though you might throw in a few test lines with the intermixed casts and compound exprs multiple times (why you've added correctly the loop in there). Jakub
On 1/24/19 8:18 AM, Jakub Jelinek wrote: > On Thu, Jan 24, 2019 at 06:39:22AM +0000, Bernd Edlinger wrote: >> --- 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; >> } > > Given the unfolded *&, I wonder if the above actually shouldn't be > a loop with indirection integral counter instead of a boolean > and simply bump the indirection count on INDIRECT_REF_P and decrease on > ADDR_EXPR, and if indirection count is > 0 after the loop return NULL_TREE? > Say for *&*& or similar, and ***var.field always not warning etc. > Okay, somehow the *&*&x is partially reduced, and arrives as *&x, so that works, also &*&x is also partially reduced, and arrives as &x, works too. Since in all test cases the INDIRECT_REFs are at the beginning, and only max. one ADDR_EXPR at the end, the counting seems unnecessary, since neither the second INDIRECT_REF nor the ADDR_EXPR are handled components, therefore there return value is always NULL_TREE, and no warning, as expected. Unfortunately **var.field should warn when field is a multi-dimensional array, and again that did not work in C++, because it happens that after the INDIRECT_REF there is a NOP_EXPR. So I would like to add a SKIP_NOP, to make the following additional test cases work: struct r { int b[10][10]; int ****i4; } i0 = *s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = *t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = &**s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = &**t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = **&s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = **&t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ i0 = ***s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ i0 = ***t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ i0 = ****&s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ i0 = ****&t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ i0 = &****s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ i0 = &****t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ > Otherwise the patch looks good. > > Though you might throw in a few test lines with the intermixed casts and > compound exprs multiple times (why you've added correctly the loop in > there). > So I've added one more, now have the following: i1 = (&t0.c, (int*) 0); /* { dg-bogus "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" } */ i1 = (int*)(0, 1, (void*)(2, 3, (int*)(4, 5, &t0.c)));/* { dg-warning "may result in an unaligned pointer value" } */ Bootstrapped and reg-tested on x86_64-pc-linux-gnu. Is it OK for trunk? Thanks Bernd. 2019-01-25 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-warn.c (check_address_or_pointer_of_packed_member): Handle the case when rhs is of array type correctly. Fix handling of nested structures. Fix handling of indirect_ref together with nop_expr and/or addr_expr. (check_and_warn_address_or_pointer_of_packed_member): Fix handling of type casts within nested compound expressions. testsuite: 2019-01-25 Bernd Edlinger <bernd.edlinger@hotmail.de> * c-c++-common/Waddress-of-packed-member-1.c: Extended test case. * c-c++-common/Waddress-of-packed-member-2.c: New test case. 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,19 @@ 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); + STRIP_NOPS (rhs); + indirect = true; + } if (TREE_CODE (rhs) == ADDR_EXPR) { rhs = TREE_OPERAND (rhs, 0); - rvalue = false; + rvalue = indirect; } if (!POINTER_TYPE_P (type)) @@ -2796,6 +2801,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 +2820,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,14 @@ 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" } */ + 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 +64,16 @@ 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" } */ + 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" } */ + i1 = (int*)(0, 1, (void*)(2, 3, (int*)(4, 5, &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,58 @@ +/* { dg-do compile } */ +/* { dg-options "-Waddress-of-packed-member" } */ + +struct r { + int a[10]; + int b[10][10]; + int ****i4; +}; + +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 = *s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = *t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = &**s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = &**t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = **&s0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = **&t0.p.b; /* { dg-warning "may result in an unaligned pointer value" } */ + 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.a; /* { dg-warning "may result in an unaligned pointer value" } */ + i0 = *&t0.p.a; /* { 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" } */ + i0 = *t0.u.b; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = &*t0.u.a; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = &**t0.u.b; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = ***s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = ***t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = ****&s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = ****&t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = &****s0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ + i0 = &****t0.p.i4; /* { dg-bogus "may result in an unaligned pointer value" } */ +}
On Fri, Jan 25, 2019 at 06:36:53AM +0000, Bernd Edlinger wrote: > 2019-01-25 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * c-warn.c (check_address_or_pointer_of_packed_member): Handle the case > when rhs is of array type correctly. Fix handling of nested structures. > Fix handling of indirect_ref together with nop_expr and/or addr_expr. > (check_and_warn_address_or_pointer_of_packed_member): Fix handling of > type casts within nested compound expressions. > > testsuite: > 2019-01-25 Bernd Edlinger <bernd.edlinger@hotmail.de> > > * c-c++-common/Waddress-of-packed-member-1.c: Extended test case. > * c-c++-common/Waddress-of-packed-member-2.c: New test case. Ok, thanks. Jakub
2019-01-22 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c/88928 * c-warn.c (check_address_or_pointer_of_packed_member): Handle the case when rhs is of array type correctly. Fix handling of nested structures. testsuite: 2019-01-22 Bernd Edlinger <bernd.edlinger@hotmail.de> PR c/88928 * c-c++-common/Waddress-of-packed-member-1.c: Extended test case. * c-c++-common/Waddress-of-packed-member-2.c: New test case. Index: gcc/c-family/c-warn.c =================================================================== --- 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; rhs = TREE_OPERAND (rhs, 0); } Index: gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c =================================================================== --- gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (revision 268119) +++ gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (working copy) @@ -6,6 +6,7 @@ struct t { int b; int *c; int d[10]; + int *e[1]; } __attribute__((packed)); struct t t0; @@ -40,6 +41,7 @@ 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" } */ 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 +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" } */ } 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" } */ +}