On Thu, Dec 13, 2018 at 12:50 PM Jason Merrill wrote:
>
> On 9/25/18 11:46 AM, H.J. Lu wrote:
> > On Fri, Aug 31, 2018 at 2:04 PM, Jason Merrill wrote:
> >> On 07/23/2018 05:24 PM, H.J. Lu wrote:
> >>>
> >>> On Mon, Jun 18, 2018 at 12:26 PM, Joseph Myers
> >>> wrote:
>
> On Mon, 18 Jun 2018, Jason Merrill wrote:
>
> > On Mon, Jun 18, 2018 at 11:59 AM, Joseph Myers
> > wrote:
> >>
> >> On Mon, 18 Jun 2018, Jason Merrill wrote:
> >>
> + if (TREE_CODE (rhs) == COND_EXPR)
> +{
> + /* Check the THEN path first. */
> + tree op1 = TREE_OPERAND (rhs, 1);
> + context = check_address_of_packed_member (type, op1);
> >>>
> >>>
> >>> This should handle the GNU extension of re-using operand 0 if operand
> >>> 1 is omitted.
> >>
> >>
> >> Doesn't that just use a SAVE_EXPR?
> >
> >
> > Hmm, I suppose it does, but many places in the compiler seem to expect
> > that it produces a COND_EXPR with TREE_OPERAND 1 as NULL_TREE.
>
>
> Maybe that's used somewhere inside the C++ front end. For C a SAVE_EXPR
> is produced directly.
> >>>
> >>>
> >>> Here is the updated patch. Changes from the last one:
> >>>
> >>> 1. Handle COMPOUND_EXPR.
> >>> 2. Fixed typos in comments.
> >>> 3. Combined warn_for_pointer_of_packed_member and
> >>> warn_for_address_of_packed_member into
> >>> warn_for_address_or_pointer_of_packed_member.
> >>
> >>
> >>> c.i:4:33: warning: converting a packed ‘struct C *’ pointer increases the
> >>> alignment of ‘long int *’ pointer from 1 to 8 [-Waddress-of-packed-member]
> >>
> >>
> >> I think this would read better as
> >>
> >> c.i:4:33: warning: converting a packed ‘struct C *’ pointer (alignment 1)
> >> to
> >> ‘long int *’ (alignment 8) may result in an unaligned pointer value
> >> [-Waddress-of-packed-member]
> >
> > Fixed.
> >
> >>> + while (TREE_CODE (base) == ARRAY_REF)
> >>> + base = TREE_OPERAND (base, 0);
> >>> + if (TREE_CODE (base) != COMPONENT_REF)
> >>> + return NULL_TREE;
> >>
> >>
> >> Are you deliberately not handling the other handled_component_p cases? If
> >> so, there should be a comment.
> >
> > I changed it to
> >
> > while (handled_component_p (base))
> > {
> >enum tree_code code = TREE_CODE (base);
> >if (code == COMPONENT_REF)
> > break;
> >switch (code)
> > {
> > case ARRAY_REF:
> >base = TREE_OPERAND (base, 0);
> >break;
> > default:
> >/* FIXME: Can it ever happen? */
> >gcc_unreachable ();
> >break;
> > }
> > }
> >
> > Is there a testcase to trigger this ICE? I couldn't find one.
>
> You can take the address of an element of complex:
>
>__complex int i;
>int *p = &__real(i);
>
> You may get VIEW_CONVERT_EXPR with location wrappers.
Fixed. I replaced gcc_unreachable with return NULL_TREE;
> >>> + /* Check alignment of the object. */
> >>> + if (TREE_CODE (object) == COMPONENT_REF)
> >>> +{
> >>> + field = TREE_OPERAND (object, 1);
> >>> + if (TREE_CODE (field) == FIELD_DECL && DECL_PACKED (field))
> >>> + {
> >>> + type_align = TYPE_ALIGN (type);
> >>> + context = DECL_CONTEXT (field);
> >>> + record_align = TYPE_ALIGN (context);
> >>> + if ((record_align % type_align) != 0)
> >>> + return context;
> >>> + }
> >>> +}
> >>
> >>
> >> Why doesn't this recurse? What if you have a packed field three
> >> COMPONENT_REFs down?
> >
> > My patch works on
> > [hjl@gnu-cfl-1 pr51628-4]$ cat x.i
> > struct A { int i; } __attribute__ ((packed));
> > struct B { struct A a; };
> > struct C { struct B b; };
> >
> > extern struct C *p;
> >
> > int* g8 (void) { return >b.a.i; }
> > [hjl@gnu-cfl-1 pr51628-4]$ make x.s
> > /export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/xgcc
> > -B/export/build/gnu/tools-build/gcc-debug/build-x86_64-linux/gcc/ -O2
> > -S x.i
> > x.i: In function ‘g8’:
> > x.i:7:25: warning: taking address of packed member of ‘struct A’ may
> > result in an unaligned pointer value [-Waddress-of-packed-member]
> > 7 | int* g8 (void) { return >b.a.i; }
> >| ^
> > [hjl@gnu-cfl-1 pr51628-4]$
> >
> > If it isn't what you had in mind, can you give me a testcase?
>
> In that testcase, 'i' is the top COMPONENT_EXPR. What I was talking
> about would be more like
>
> struct A { int i; };
> struct B { struct A a; };
> struct C { struct B b __attribute__ ((packed)); };
>
> extern struct C *p;
>
> int* g8 (void) { return >b.a.i; }
>
Fixed with a recursive call:
tree context = check_alignment_of_packed_member (type, field);
if (context)
return context;
/* Check alignment of the object. */
while (TREE_CODE (object) == COMPONENT_REF)
{
