Bug 51628 - __attribute__((packed)) is unsafe in some cases (i.e. add -Waddress-of-packed-member, etc.)
Summary: __attribute__((packed)) is unsafe in some cases (i.e. add -Waddress-of-packed...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.5.1
: P3 normal
Target Milestone: 9.0
Assignee: Not yet assigned to anyone
URL: https://gcc.gnu.org/ml/gcc-patches/20...
Keywords: diagnostic, patch, wrong-code
Depends on:
Blocks: new-warning
  Show dependency treegraph
 
Reported: 2011-12-19 22:58 UTC by Keith Thompson
Modified: 2019-01-18 13:05 UTC (History)
14 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2011-12-20 00:00:00


Attachments
packed.c test case with output in a comment (407 bytes, text/plain)
2011-12-19 22:58 UTC, Keith Thompson
Details
Add -Waddress-of-packed-member to C/C++ FE (2.64 KB, patch)
2018-01-13 16:16 UTC, H.J. Lu
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Thompson 2011-12-19 22:58:07 UTC
Created attachment 26147 [details]
packed.c test case with output in a comment

I've seen this problem with gcc 4.5.1 on SPARC Solaris 9.  I presume it will affect other versions of gcc on other platforms with strict alignment requirements (unlike x86, which supports misaligned accesses in hardware as I understand it).  I think it also applies to "#pragma pack".

The gcc extension __attribute__(packed), which applied to a struct, has the following semantics (quoting the gcc documentation for 4.5.2):

     The `packed' attribute specifies that a variable or structure field
     should have the smallest possible alignment--one byte for a
     variable, and one bit for a field, unless you specify a larger
     value with the `aligned' attribute.

When a program accesses a misaligned member of a packed struct, the compiler generates whatever code is necessary to read or write the correct value.

If the address of a misaligned member is stored in a pointer object, dereferencing that pointer doesn't give the compiler an opportunity to generate that extra code.

The attached program demonstrates the problem, and includes (as a comment) the output I get on Ubuntu x86 (ok) and Solaris 9 SPARC (bus error).

See also http://stackoverflow.com/questions/8568432/is-gccs-attribute-packed-pragma-pack-unsafe/

I don't believe it would be practical to fix this (though there might be some clever solution I haven't thought of).  But at least I suggest mentioning this issue in the documentation.
Comment 1 Keith Thompson 2011-12-19 23:05:50 UTC
A commenter on stackoverflow points out that a possible fix would be to permit the address of a member of a packed structure to be assigned only to a pointer object that is itself declared "packed".
Comment 2 Andrew Pinski 2011-12-19 23:27:24 UTC
I think this is just undefined at runtime which means we can only warn about it.  Which we do with -Walign
Comment 3 Keith Thompson 2011-12-20 00:36:52 UTC
I see no "-Walign" option, either in the versions of gcc I'm using or in the online documentation.  Were you thinking of a different option?

What I'm suggesting, primarily, is that the issue should be mentioned in the documentation of the "packed" attribute (and probably of "pragma #pack" as well).
Comment 4 Richard Biener 2011-12-20 10:20:19 UTC
The point is that even if you use sth like

typedef int myint __attribute__((aligned(1)));

to capture the misaligned pointer to the packed structure element:

myint *p = &s->i;

then accesses like '*p' will still assume an _aligned_ int at 'p' for
STRICT_ALIGNMENT targets.

That's a long-long-long-standing bug and a cause of major headache for
more modern GCCs even ...

The testcase with using a 'int *' pointer is indeed invalid though.
Comment 5 Eric Botcazou 2011-12-20 10:52:19 UTC
> The point is that even if you use sth like
> 
> typedef int myint __attribute__((aligned(1)));
> 
> to capture the misaligned pointer to the packed structure element:
> 
> myint *p = &s->i;
> 
> then accesses like '*p' will still assume an _aligned_ int at 'p' for
> STRICT_ALIGNMENT targets.
> 
> That's a long-long-long-standing bug and a cause of major headache for
> more modern GCCs even ...

That's a limitation rather than a bug.  Clearly, on strict-alignment targets, you must know what you're doing when you start to misalign things.  As for

 typedef int myint __attribute__((aligned(1)));

that's an abomination I don't even want to know of ;-)
Comment 6 rguenther@suse.de 2011-12-20 11:04:37 UTC
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 10:52:19 UTC ---
> > The point is that even if you use sth like
> > 
> > typedef int myint __attribute__((aligned(1)));
> > 
> > to capture the misaligned pointer to the packed structure element:
> > 
> > myint *p = &s->i;
> > 
> > then accesses like '*p' will still assume an _aligned_ int at 'p' for
> > STRICT_ALIGNMENT targets.
> > 
> > That's a long-long-long-standing bug and a cause of major headache for
> > more modern GCCs even ...
> 
> That's a limitation rather than a bug.  Clearly, on strict-alignment targets,
> you must know what you're doing when you start to misalign things.  As for
> 
>  typedef int myint __attribute__((aligned(1)));
> 
> that's an abomination I don't even want to know of ;-)

Huh, it's not.  It's the same as a packed struct or enum type.  Why
can't you strict-align people simply fix this case?

Richard.
Comment 7 Eric Botcazou 2011-12-20 11:18:24 UTC
> Huh, it's not.  It's the same as a packed struct or enum type.

No, it isn't, the mode is integral instead of BLKmode.  In Ada we do support misaligned integers, but we simply wrap them in a BLKmode record.

> Why can't you strict-align people simply fix this case?

Because integral modes are naturally aligned.  The only reasonable way to support the aforementioned abomination is to use the Ada approach.
Comment 8 rguenther@suse.de 2011-12-20 11:23:48 UTC
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 11:18:24 UTC ---
> > Huh, it's not.  It's the same as a packed struct or enum type.
> 
> No, it isn't, the mode is integral instead of BLKmode.  In Ada we do support
> misaligned integers, but we simply wrap them in a BLKmode record.
> 
> > Why can't you strict-align people simply fix this case?
> 
> Because integral modes are naturally aligned.  The only reasonable way to
> support the aforementioned abomination is to use the Ada approach.

You mean that handling the TYPE_ALIGN != MODE_ALIGN case when
expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work?
Why not?  You'd simply have to emit the same RTL as when expanding
that wrapped struct case.

Richard.
Comment 9 Eric Botcazou 2011-12-20 11:34:00 UTC
> You mean that handling the TYPE_ALIGN != MODE_ALIGN case when
> expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work?

But you cannot have TYPE_ALIGN < MODE_ALIGN (TYPE_MODE).
Comment 10 rguenther@suse.de 2011-12-20 11:56:22 UTC
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 11:34:00 UTC ---
> > You mean that handling the TYPE_ALIGN != MODE_ALIGN case when
> > expanding a MEM_REF (thus, INDIRECT_REF on old branches) won't work?
> 
> But you cannot have TYPE_ALIGN < MODE_ALIGN (TYPE_MODE).

You can.  Just check what you get with that aligned(1) int typedef.

For vector types you can anyways (by design) to allow unaligned
vector moves.

Is it documented anywhere that you can't take the address of
an unaligned structure member (given the struct is packed) on
STRICT_ALIGNMENT targets (or, when it's a vector component even
on non-STRICT_ALIGNMENT targets)?  Why does the C frontend not
warn for these cases (unconditionally?)?

I don't see at all that we can't support expanding such moves
properly.  After all we can extract an INT mode value from
unaligned memory (we are never generating a mem:BLK with
INT mode size in that case anyway, or a MEM:SI with MEM_ALIGN
less than SImode align anyway)
Comment 11 Eric Botcazou 2011-12-20 12:25:13 UTC
> You can.  Just check what you get with that aligned(1) int typedef.

Well, we're going in circles as this example precisely doesn't work.

> Is it documented anywhere that you can't take the address of
> an unaligned structure member (given the struct is packed) on
> STRICT_ALIGNMENT targets (or, when it's a vector component even
> on non-STRICT_ALIGNMENT targets)?  Why does the C frontend not
> warn for these cases (unconditionally?)?

Good question, but for a C maintainer.  The C front-end would have implemented something for a long time if it had cared about the issue, but apparently not.
In Ada we do care since Ada 2005, so we have implemented the necessary support.
Comment 12 Jakub Jelinek 2011-12-20 12:35:18 UTC
What Ada does looks just like a workaround for what should be done properly in the expander.  So no, IMHO we shouldn't be changing all other FEs and the middle-end (when it wants to generate them e.g. for memcpy) to do what Ada does, but we should change the expander.  It has all the information to perform the unaligned reads/writes, it just doesn't use them.
Comment 13 rguenther@suse.de 2011-12-20 13:21:02 UTC
On Tue, 20 Dec 2011, ebotcazou at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-12-20 12:25:13 UTC ---
> > You can.  Just check what you get with that aligned(1) int typedef.
> 
> Well, we're going in circles as this example precisely doesn't work.
> 
> > Is it documented anywhere that you can't take the address of
> > an unaligned structure member (given the struct is packed) on
> > STRICT_ALIGNMENT targets (or, when it's a vector component even
> > on non-STRICT_ALIGNMENT targets)?  Why does the C frontend not
> > warn for these cases (unconditionally?)?
> 
> Good question, but for a C maintainer.  The C front-end would have implemented
> something for a long time if it had cared about the issue, but apparently not.
> In Ada we do care since Ada 2005, so we have implemented the necessary support.

Fact is, the middle-end needs a way to support this (well, or "wants").
Otherwise stripping off component-refs does not work even for the
"long time working cases".  "Fixes" to avoid stripping them away
are not really fixes but workarounds around fixing this long-time issue.

But I see you are not going to work on fixing the expansion side
(which I hoped, since you have plenty of experience in this area
and strict-alignment targets)
Comment 14 Eric Botcazou 2011-12-20 17:43:07 UTC
> What Ada does looks just like a workaround for what should be done properly in
> the expander.  So no, IMHO we shouldn't be changing all other FEs and the
> middle-end (when it wants to generate them e.g. for memcpy) to do what Ada
> does, but we should change the expander.

Of course I have the exactly opposite viewpoint, since I think that we should keep the invariants we have: integral modes are naturally aligned and TYPE_ALIGN >= MODE_ALIGN (TYPE_MODE).  Breaking them for a few pathological cases that the C compiler has shun for years doesn't seem the best course of action.
Comment 15 incrediball 2013-04-02 00:58:40 UTC
I believe the discussion here is missing the point. Currently (at least with version 4.5 and ARM, which I am currently using) the situation is that the compiler generates broken code WITHOUT COMMENT when the address of something like an integer in a packed structure is assigned to a int* . Since the pointer type does not provide any information about the actual alignment of the integer, it is obviously impractical to fix when accessing the integer and we certainly do not want to have extra code inserted for handling the possibility that the int may not be aligned correctly. Therefore the compiler MUST see this as a type conflict and at least warn that the address of the member in a packed struct is incompatible with the pointer type.

I have several projects which require the use of packed structures and I simply cannot rule out that that I have not at some point accidentally assigned the address of a member variable to a pointer. It would therefore be very handy if the compiler could point this out. -Wcast-align certainly doesn't do this.
Comment 16 Richard Biener 2013-04-02 08:30:16 UTC
(In reply to comment #14)
> > What Ada does looks just like a workaround for what should be done properly in
> > the expander.  So no, IMHO we shouldn't be changing all other FEs and the
> > middle-end (when it wants to generate them e.g. for memcpy) to do what Ada
> > does, but we should change the expander.
> 
> Of course I have the exactly opposite viewpoint, since I think that we should
> keep the invariants we have: integral modes are naturally aligned and
> TYPE_ALIGN >= MODE_ALIGN (TYPE_MODE).  Breaking them for a few pathological
> cases that the C compiler has shun for years doesn't seem the best course of
> action.

Well.  I'm fine with forcing BLKmode for unaligned accesses, but not sure
if that is less invasive.  You'd basically have to change modes whenever
more precise knowledge about alignment appears ...
Comment 17 Richard Biener 2013-04-02 08:30:54 UTC
(In reply to comment #15)
> I believe the discussion here is missing the point. Currently (at least with
> version 4.5 and ARM, which I am currently using) the situation is that the
> compiler generates broken code WITHOUT COMMENT when the address of something
> like an integer in a packed structure is assigned to a int* . Since the pointer
> type does not provide any information about the actual alignment of the
> integer, it is obviously impractical to fix when accessing the integer and we
> certainly do not want to have extra code inserted for handling the possibility
> that the int may not be aligned correctly. Therefore the compiler MUST see this
> as a type conflict and at least warn that the address of the member in a packed
> struct is incompatible with the pointer type.
> 
> I have several projects which require the use of packed structures and I simply
> cannot rule out that that I have not at some point accidentally assigned the
> address of a member variable to a pointer. It would therefore be very handy if
> the compiler could point this out. -Wcast-align certainly doesn't do this.

Because there is no cast involved.  And that's because the types of the fields
are "wrong" as well.
Comment 18 incrediball 2013-04-02 20:21:06 UTC
Not sure if I can agree with (or understand) this comment. If we use my example of the address of an int in a packed structure being assigned to an int* then one could argue that there is a kind of implicit cast happening. The pointer is to an integer WITH the default alignment which would be 4 on the ARM system I work with. The int in the struct has an alignment of 1 and so the two types need to be regarded as being incompatible and there should be an unconditional warning or at least -Wcast-align should enable such a warning.

I don't know what BLKmode is but if you're speaking of making an int* type generally able to access the int regardless of its alignment, I think this is the wrong way to go because code size will explode and efficiency will go out the window. However if the int* would be declared with some special attribute then, sure, generate whatever code that is necessary to access the int and the above mentioned warning can be suppressed in this case (wouldn't the code generated be much the same code that is used to directly access the int in the packed struct anyway?) However this would be the second (optional) step, the first thing is that the compiler needs to emit a warning explaining that the code that it is generating is broken.
Comment 19 Eric Botcazou 2013-04-03 07:29:51 UTC
> Not sure if I can agree with (or understand) this comment. If we use my example
> of the address of an int in a packed structure being assigned to an int* then
> one could argue that there is a kind of implicit cast happening. The pointer is
> to an integer WITH the default alignment which would be 4 on the ARM system I
> work with. The int in the struct has an alignment of 1 and so the two types
> need to be regarded as being incompatible and there should be an unconditional
> warning or at least -Wcast-align should enable such a warning.

Yes, a warning should probably be implemented in the C family of compilers, but this is a very old issue and nobody is really interested.  Note that taking the address of the field is OK, the problem is dereferencing it.
Comment 20 rguenther@suse.de 2013-04-03 07:55:34 UTC
On Wed, 3 Apr 2013, ebotcazou at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #19 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-04-03 07:29:51 UTC ---
> > Not sure if I can agree with (or understand) this comment. If we use my example
> > of the address of an int in a packed structure being assigned to an int* then
> > one could argue that there is a kind of implicit cast happening. The pointer is
> > to an integer WITH the default alignment which would be 4 on the ARM system I
> > work with. The int in the struct has an alignment of 1 and so the two types
> > need to be regarded as being incompatible and there should be an unconditional
> > warning or at least -Wcast-align should enable such a warning.
> 
> Yes, a warning should probably be implemented in the C family of compilers, but
> this is a very old issue and nobody is really interested.  Note that taking the
> address of the field is OK, the problem is dereferencing it.

One of the C frontend issues is that the type of the address of
the field of the packed struct is int *, not int attribute((aligned(1))) 
*.  And this is so because nothing adjusts the type of the FIELD_DECL
to be a less aligned type.  That is, we have

 <field_decl 0x7ffff6d245f0 i
    type <integer_type 0x7ffff6d175e8 int public SI
        size <integer_cst 0x7ffff6d1a0c0 constant 32>
        unit size <integer_cst 0x7ffff6d1a0e0 constant 4>
        align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 
precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst 
0x7ffff6d1a080 2147483647>
        pointer_to_this <pointer_type 0x7ffff6d1f2a0>>
    packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> 
unit size <integer_cst 0x7ffff6d1a0e0 4>
    align 8 offset_align 128
    offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 
sizetype> constant 0>
    bit offset <integer_cst 0x7ffff6d02e00 type <integer_type 
0x7ffff6d170a8 bitsizetype> constant 0> context <record_type 
0x7ffff6e1c3f0 Foo>>

and alignof (x.i) and alignof (*&x.i) would not agree (if the
latter were not folded to x.i) for

struct Foo {
    int i;
} __attribute__((packed)) ;
struct Foo x;

for example (surprisingly) __alignof__ (*(char *)&x.i) is 4
while __alignof__ (*&x.i) is 1 (due to folding) and
__alignof__ (x.i) is 1 as well.

I'm quite sure that "fixing" this will have loads of fallout
throughout the frontend(s if 'fixed' in stor-layout.c).  Which
is why this issue is so old and unfixed.
Comment 21 Eric Botcazou 2013-04-03 08:51:44 UTC
> One of the C frontend issues is that the type of the address of
> the field of the packed struct is int *, not int attribute((aligned(1))) 
> *.  And this is so because nothing adjusts the type of the FIELD_DECL
> to be a less aligned type.  That is, we have
> 
>  <field_decl 0x7ffff6d245f0 i
>     type <integer_type 0x7ffff6d175e8 int public SI
>         size <integer_cst 0x7ffff6d1a0c0 constant 32>
>         unit size <integer_cst 0x7ffff6d1a0e0 constant 4>
>         align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 
> precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst 
> 0x7ffff6d1a080 2147483647>
>         pointer_to_this <pointer_type 0x7ffff6d1f2a0>>
>     packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> 
> unit size <integer_cst 0x7ffff6d1a0e0 4>
>     align 8 offset_align 128
>     offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 
> sizetype> constant 0>
>     bit offset <integer_cst 0x7ffff6d02e00 type <integer_type 
> 0x7ffff6d170a8 bitsizetype> constant 0> context <record_type 
> 0x7ffff6e1c3f0 Foo>>

This is on x86, right?  If the alignment of the field cannot be guaranteed to be that of its type, then it should be made a bit-field.  Maybe it's already made a bit-field on strict-alignment targets.
Comment 22 rguenther@suse.de 2013-04-03 09:20:21 UTC
On Wed, 3 Apr 2013, ebotcazou at gcc dot gnu.org wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> --- Comment #21 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-04-03 08:51:44 UTC ---
> > One of the C frontend issues is that the type of the address of
> > the field of the packed struct is int *, not int attribute((aligned(1))) 
> > *.  And this is so because nothing adjusts the type of the FIELD_DECL
> > to be a less aligned type.  That is, we have
> > 
> >  <field_decl 0x7ffff6d245f0 i
> >     type <integer_type 0x7ffff6d175e8 int public SI
> >         size <integer_cst 0x7ffff6d1a0c0 constant 32>
> >         unit size <integer_cst 0x7ffff6d1a0e0 constant 4>
> >         align 32 symtab 0 alias set -1 canonical type 0x7ffff6d175e8 
> > precision 32 min <integer_cst 0x7ffff6d1a060 -2147483648> max <integer_cst 
> > 0x7ffff6d1a080 2147483647>
> >         pointer_to_this <pointer_type 0x7ffff6d1f2a0>>
> >     packed SI file t.c line 2 col 9 size <integer_cst 0x7ffff6d1a0c0 32> 
> > unit size <integer_cst 0x7ffff6d1a0e0 4>
> >     align 8 offset_align 128
> >     offset <integer_cst 0x7ffff6d02d80 type <integer_type 0x7ffff6d17000 
> > sizetype> constant 0>
> >     bit offset <integer_cst 0x7ffff6d02e00 type <integer_type 
> > 0x7ffff6d170a8 bitsizetype> constant 0> context <record_type 
> > 0x7ffff6e1c3f0 Foo>>
> 
> This is on x86, right?  If the alignment of the field cannot be guaranteed to
> be that of its type, then it should be made a bit-field.  Maybe it's already
> made a bit-field on strict-alignment targets.

Note the FIELD_DECL is perfectly ok (align 8), it is its TREE_TYPE
that is "bogus", and this type is used when building the pointer type
used for taking the address of it (so you could argue _that_ is the
bug - it shouldn't literally take TREE_TYPE of a FIELD_DECL when
building the address of a COMPONENT_REF - the COMPONENT_REF
surely only caring about the type of the value not the storage).

Richard.
Comment 23 Sven 2015-03-10 13:29:35 UTC
FYI: I have asked the llvm folks to add a warning to their compiler for the when a pointer to a member of a packed struct is assigned to an "ordinary" pointer with higher alignment guarantees. Clearly, I agree with comment #18 that compilers should warn about this.

See https://llvm.org/bugs/show_bug.cgi?id=22821
Comment 24 Sven 2015-03-10 13:40:47 UTC
Comment #4 mentions typedef int myint __attribute__((aligned(1)));
That shouldn't even work. The GCC documentation on Type Attributes mentions that "The aligned attribute can only increase the alignment". It goes on to mention the packed attribute, which can be used to decrease alignment. But as far as I know, that attributes was designed for structs. Anyhow, it seems that the aligned attribute is intended for increasing the alignment only - not for decreasing.

Yet, when I checked __alignof(myint) with both gcc and clang, it was in fact decreased from 4 to 1. Not sure why. That seems to contradict the documentation.
Comment 25 Martin Sebor 2017-06-01 18:57:10 UTC
(In reply to Sven from comment #24)

Agreed.  This is the subject of bug 69502.
Comment 26 Eric Gallager 2017-06-02 00:45:18 UTC
(In reply to Keith Thompson from comment #3)
> I see no "-Walign" option, either in the versions of gcc I'm using or in the
> online documentation.  Were you thinking of a different option?
>

Maybe -Wpacked?
Comment 27 Eric Gallager 2017-11-02 01:03:36 UTC
(In reply to Eric Gallager from comment #26)
> (In reply to Keith Thompson from comment #3)
> > I see no "-Walign" option, either in the versions of gcc I'm using or in the
> > online documentation.  Were you thinking of a different option?
> >
> 
> Maybe -Wpacked?

gcc 8 adds -Wpacked-not-aligned; does that fix this bug?
Comment 28 Sven 2017-11-02 01:53:38 UTC
(In reply to Eric Gallager from comment #27)
> gcc 8 adds -Wpacked-not-aligned; does that fix this bug?

I couldn't find documentation on what this switch is supposed to do. Can you point me in the right direction? Is there some commit explaining -Wpacked-not-aligned?

FYI: the LLVM developers have added -Waddress-of-packed-member which addresses the issue discussed here.
Comment 29 Eric Gallager 2017-11-02 04:29:39 UTC
(In reply to Sven from comment #28)
> (In reply to Eric Gallager from comment #27)
> > gcc 8 adds -Wpacked-not-aligned; does that fix this bug?
> 
> I couldn't find documentation on what this switch is supposed to do. Can you
> point me in the right direction? Is there some commit explaining
> -Wpacked-not-aligned?
> 

I don't remember the exact commit number but HJ Lu added it; I added him on cc with my previous comment, so maybe he can explain.
Comment 30 H.J. Lu 2017-11-02 12:45:38 UTC
(In reply to Eric Gallager from comment #29)
> 
> I don't remember the exact commit number but HJ Lu added it; I added him on
> cc with my previous comment, so maybe he can explain.

It is r251180.
Comment 31 Sven 2017-11-02 13:05:32 UTC
https://gcc.gnu.org/viewcvs/gcc/trunk/?view=log&pathrev=251180

I am reading the commit message, and the example doesn't make any sense.
The aligned attribute is for providing additional alignment guarantees in addition to the types default alignment. In particular, the documentation of the aligned type attribute specifically states, that this attribute cannot decrease alignment. It can only _increase_ alignment.

In particular, aligned(4) in combination with a 64bit type makes little sense.
Comment 32 H.J. Lu 2017-11-02 13:11:28 UTC
(In reply to Sven from comment #31)
> https://gcc.gnu.org/viewcvs/gcc/trunk/?view=log&pathrev=251180
> 
> I am reading the commit message, and the example doesn't make any sense.
> The aligned attribute is for providing additional alignment guarantees in
> addition to the types default alignment. In particular, the documentation of
> the aligned type attribute specifically states, that this attribute cannot
> decrease alignment. It can only _increase_ alignment.
> 
> In particular, aligned(4) in combination with a 64bit type makes little
> sense.

long long is aligned to 4 bytes in struct for i386.
Comment 33 H.J. Lu 2017-11-02 13:14:16 UTC
[hjl@gnu-skl-1 gcc]$ cat x.c
#include <stdio.h>
#include <stddef.h>
typedef int aligned_int __attribute__((warn_if_not_aligned(4)));
int main(void)
{
    struct foo {
        char c;
	aligned_int x;
    } __attribute__((packed));
    struct foo arr[2] = { { 'a', 10 }, {'b', 20 } };
    int *p0 = &arr[0].x;
    int *p1 = &arr[1].x;
    printf("sizeof(struct foo) = %d\n",
           (int)sizeof(struct foo));
    printf("offsetof(struct foo, c) = %d\n",
           (int)offsetof(struct foo, c));
    printf("offsetof(struct foo, x) = %d\n",
           (int)offsetof(struct foo, x));
    printf("arr[0].x = %d\n", arr[0].x);
    printf("arr[1].x = %d\n", arr[1].x);
    printf("p0 = %p\n", (void*)p0);
    printf("p1 = %p\n", (void*)p1);
    printf("*p0 = %d\n", *p0);
    printf("*p1 = %d\n", *p1);
    return 0;
}
[hjl@gnu-skl-1 gcc]$ ./xgcc -B./ -S x.c -Wall 
x.c: In function \u2018main\u2019:
x.c:9:5: warning: alignment 1 of \u2018struct foo\u2019 is less than 4 [-Wif-not-aligned]
     } __attribute__((packed));
     ^
x.c:8:14: warning: \u2018x\u2019 offset 1 in \u2018struct foo\u2019 isn't aligned to 4 [-Wif-not-aligned]
  aligned_int x;
              ^
[hjl@gnu-skl-1 gcc]$
Comment 34 Sven 2017-11-02 13:27:31 UTC
(In reply to H.J. Lu from comment #32)
> long long is aligned to 4 bytes in struct for i386.

Understood. So the aligned(4) was just added to explicitly restating the alignment?

Anyhow, the two warnings added by that commit seem unrelated to this issue. The warnings added check the alignment of variables and members in memory. When they fall below the given threshold, a warning is issued.

This bug report however is about a different issue. Every member of a packed struct has an alignment guarantee of 1 - no more than that. However, a regular int* variable guarantees, when dereferenced, guarantees an alignment of 4 (on i386 and arm, for example). So the following code should produce a warning:

struct foo
{
  int i;
} __attribute__((packed));

struct foo *x;
int *y = &(x->i);


The issue becomes more obvious with this struct:

struct foo2
{
  char c;
  int i;
} __attribute__((packed));


However, both foo and foo2 only have an alignment guarantee of at most 1, so also the int member inside both structs has an alignment guarantee of at most 1.

As mentioned in the initial post, gcc will generate the proper machine code to read an unaligned int (on arm for example) when using x->i directly. However, when using *y, gcc will assume 4 byte alignment. That is to be expected, hence gcc should warn about the fact, and the address of a (potentially) unaligned int is assigned to a regular int* pointer.
Comment 35 Sven 2017-11-02 13:29:48 UTC
(In reply to Sven from comment #34)
> That is to be
> expected, hence gcc should warn about the fact, and the address of a
> (potentially) unaligned int is assigned to a regular int* pointer.

Sorry, typo:
That is to be expected, hence gcc should warn about the fact that the address of a (potentially) unaligned int is assigned to a regular int* pointer.
Comment 36 Alexey Salmin 2018-01-12 10:57:30 UTC
FYI a test case that actually triggers SIGSEGV on x86 with gcc 7.2: https://godbolt.org/g/kPw6NJ

Compiler emits a movaps instruction which is alignment-sensitive. A warning would be helpful.

struct pair_t {
    char c;
    __int128_t i;
} __attribute__((packed));

struct pair_t p = {0, 1};
__int128_t *addr = &p.i;

int main() {
    *addr = ~(__int128_t)0;
    return (p.i != 1) ? 0 : 1;
}
Comment 37 H.J. Lu 2018-01-13 16:16:06 UTC
Created attachment 43120 [details]
Add -Waddress-of-packed-member to C/C++ FE

Please try this.
Comment 38 H.J. Lu 2018-01-15 02:06:57 UTC
A patch is posted at

https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01237.html
Comment 39 Alexey Salmin 2018-01-16 15:06:58 UTC
Thank you, this patch works for me.

Gives a warning in the attached test case, but still allows to take an address of a packed struct members when the packed attribute is preserved in the pointer.
Comment 40 H.J. Lu 2018-01-16 15:47:54 UTC
(In reply to Alexey Salmin from comment #39)
> Thank you, this patch works for me.
> 
> Gives a warning in the attached test case, but still allows to take an
> addre of a packed struct members when the packed attribute is preserved in
> the pointer.

Do you have such a testcase with the packed attribute preserved in
the pointer?
Comment 41 Sven 2018-01-16 17:03:31 UTC
(In reply to Alexey Salmin from comment #39)
> .. when the packed attribute is preserved in the pointer.

What do you mean by that? GCC documentation explicitly forbids to use the packed attribute for anything but structs, unions, and enums.
Comment 42 Alexey Salmin 2018-01-17 10:29:55 UTC
Sorry for being unclear.

When I need a pointer to an unaligned type I wrap it in a struct. E.g. a fix for SIGSEGV from comment#36 would look like this:

struct pair_t {
    char c;
    __int128_t i;
} __attribute__((packed));

typedef struct unaligned_int128_t_ {
    __int128_t value;
} __attribute__((packed)) unaligned_int128_t;

struct pair_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

int main() {
    addr->value = ~(__int128_t)0;
    return (p.i != 1) ? 0 : 1;
}

I was trying to say that I'm glad that the "address-of-packed-member" warning isn't triggered by this code. It still relies on the "address of packed member" but in a safe way.
Comment 43 H.J. Lu 2018-01-17 11:45:47 UTC
(In reply to Alexey Salmin from comment #42)
> 
> I was trying to say that I'm glad that the "address-of-packed-member"
> warning isn't triggered by this code. It still relies on the "address of
> packed member" but in a safe way.

Please try the current patch:

https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01535.html
Comment 44 Sven 2018-01-17 13:21:22 UTC
(In reply to Alexey Salmin from comment #42)
> typedef struct unaligned_int128_t_ {
>     __int128_t value;
> } __attribute__((packed)) unaligned_int128_t;

You can combine the packed attribute with the aligned attribute. Then you can define one struct with aligned(4) and one with aligned(8). Does the warning trigger if you cast between those types? Or does the cast simply override the warning because it's now the programmers responsibility to make sure that the alignment is correct?
Comment 45 H.J. Lu 2018-01-17 13:37:44 UTC
(In reply to Sven from comment #44)
> 
> You can combine the packed attribute with the aligned attribute. Then you
> can define one struct with aligned(4) and one with aligned(8). Does the
> warning trigger if you cast between those types? Or does the cast simply
> override the warning because it's now the programmers responsibility to make
> sure that the alignment is correct?

Yes, my patch handles it correctly:

[hjl@gnu-tools-1 pr51628]$ cat g3.i 
struct pair_t {
    __int128_t x;
    __int128_t i;
} __attribute__((packed, aligned (4)));

typedef struct unaligned_int128_t_ {
    __int128_t value;
} __attribute__((packed, aligned (8))) unaligned_int128_t;

struct pair_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

int main() {
    addr->value = ~(__int128_t)0;
    return (p.i != 1) ? 0 : 1;
}
[hjl@gnu-tools-1 pr51628]$ /export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -O2 -S g3.i
g3.i:11:28: warning: initialization of ‘unaligned_int128_t *’ {aka ‘struct unaligned_int128_t_ *’} from address of packed member of ‘struct pair_t’ may result in an unaligned pointer value [-Waddress-of-packed-member]
 unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);
                            ^
[hjl@gnu-tools-1 pr51628]$ cat g4.i 
struct pair_t {
    __int128_t x;
    __int128_t i;
} __attribute__((packed, aligned (8)));

typedef struct unaligned_int128_t_ {
    __int128_t value;
} __attribute__((packed, aligned (4))) unaligned_int128_t;

struct pair_t p = {0, 1};
unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

int main() {
    addr->value = ~(__int128_t)0;
    return (p.i != 1) ? 0 : 1;
}
[hjl@gnu-tools-1 pr51628]$ /export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -O2 -S g4.i
[hjl@gnu-tools-1 pr51628]$
Comment 46 Alexey Salmin 2018-01-18 13:31:26 UTC
Tested the latest patch, behavior looks very reasonable even in tricky cases.

1) No warning, gcd(12, 8) == 4
    struct tuple_t {
        char c[12];
        __int128_t i;
    } __attribute__((packed, aligned(8)));

    typedef struct unaligned_int128_t_ {
        __int128_t value;
    } __attribute__((packed, aligned(4))) unaligned_int128_t;

    struct tuple_t p = {0, 1};
    unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);

2) Gives a warning, gcd(10, 8) < 4
    struct tuple_t {
        char c[10];
        __int128_t i;
    } __attribute__((packed, aligned(8)));

    typedef struct unaligned_int128_t_ {
        __int128_t value;
    } __attribute__((packed, aligned(4))) unaligned_int128_t;

    struct tuple_t p = {0, 1};
    unaligned_int128_t *addr = (unaligned_int128_t *)(&p.i);
Comment 47 W.H. Ding 2018-04-12 08:26:24 UTC
Hi, everyone

I wonder if this issue has to do with the bug-like problem I encountered when accessing an unaligned stand-alone global variable (rather than a member of a packed struct). A test case is as follows:

char g_c = 'x';
int g_d __attribute__((aligned(1))) = 13;

int main(void)
{
    g_c = 'z';
    //================
    g_d = 33;                // Crash on, in my case, ARM Cortex-M0
    //================
    return 0;
}

Due to the presence of g_c, g_d is at an odd address, as the map file indicates:

 .data.g_c      0x20000020        0x1 ./src/test.o
                0x20000020                g_c
 .data.g_d      0x20000021        0x4 ./src/test.o
                0x20000021                g_d

The generated assembly, however, accesses g_d as if it were properly aligned:

   8:../src/test.c ****     //================
   9:../src/test.c ****     g_d = 33;
  52                            .loc 1 9 0
  53 000a 044B     		ldr	r3, .L2+4
  54 000c 2122     		movs	r2, #33
  55 000e 1A60     		str	r2, [r3]
  10:../src/test.c ****     //================

BTW: I'm using GNU ARM Embedded Toolchain 7-2017-q4-major and on Windows 7.

Any ideas?
Comment 48 rguenther@suse.de 2018-04-12 08:57:35 UTC
On Thu, 12 Apr 2018, dingcurie at icloud dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628
> 
> W.H. Ding <dingcurie at icloud dot com> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |dingcurie at icloud dot com
> 
> --- Comment #47 from W.H. Ding <dingcurie at icloud dot com> ---
> Hi, everyone
> 
> I wonder if this issue has to do with the bug-like problem I encountered when
> accessing an unaligned stand-alone global variable (rather than a member of a
> packed struct). A test case is as follows:
> 
> char g_c = 'x';
> int g_d __attribute__((aligned(1))) = 13;
> 
> int main(void)
> {
>     g_c = 'z';
>     //================
>     g_d = 33;                // Crash on, in my case, ARM Cortex-M0
>     //================
>     return 0;
> }
> 
> Due to the presence of g_c, g_d is at an odd address, as the map file
> indicates:
> 
>  .data.g_c      0x20000020        0x1 ./src/test.o
>                 0x20000020                g_c
>  .data.g_d      0x20000021        0x4 ./src/test.o
>                 0x20000021                g_d
> 
> The generated assembly, however, accesses g_d as if it were properly aligned:
> 
>    8:../src/test.c ****     //================
>    9:../src/test.c ****     g_d = 33;
>   52                            .loc 1 9 0
>   53 000a 044B                  ldr     r3, .L2+4
>   54 000c 2122                  movs    r2, #33
>   55 000e 1A60                  str     r2, [r3]
>   10:../src/test.c ****     //================
> 
> BTW: I'm using GNU ARM Embedded Toolchain 7-2017-q4-major and on Windows 7.
> 
> Any ideas?

It looks like RTL expansion is broken here (also on x86_64):

;; g_d = 33;

(insn 6 5 0 (set (mem/c:SI (symbol_ref:DI ("g_d") [flags 0x2]  <var_decl 
0x7fea38d38ea0 g_d>) [1 g_d+0 S4 A32])
        (const_int 33 [0x21])) "t.c":7 -1
     (nil))

notice how the MEM has 32bit alignment.  This is probably because
while g_d has a DECL_ALIGN of 8 it has a type with TYPE_ALIGN of 32.
IIRC there's related bugs for the FEs that taking the address of
g_d results in a 32bit aligned int * pointer and thus
*&g_d has different alignment than g_d.

A workaround is to place such variables in a struct I guess.

The "bug" is in set_mem_attributes_minus_bitpos (when you ignore
the FE bug of mismatch of decl and type alignment):

  /* We can set the alignment from the type if we are making an object or if
     this is an INDIRECT_REF.  */
  if (objectp || TREE_CODE (t) == INDIRECT_REF)
    attrs.align = MAX (attrs.align, TYPE_ALIGN (type));

a workaround would be to do the following as in the !TYPE_P path
we already use the type to derive alignment when allowed.

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c      (revision 259337)
+++ gcc/emit-rtl.c      (working copy)
@@ -1985,9 +1985,8 @@ set_mem_attributes_minus_bitpos (rtx ref
         able to simply always use TYPE_ALIGN?  */
     }
 
-  /* We can set the alignment from the type if we are making an object or 
if
-     this is an INDIRECT_REF.  */
-  if (objectp || TREE_CODE (t) == INDIRECT_REF)
+  /* We can set the alignment from the type if we are making an object.  
*/
+  if (objectp && TYPE_P (t))
     attrs.align = MAX (attrs.align, TYPE_ALIGN (type));
 
   /* If the size is known, we can set that.  */
Comment 49 Sven 2018-04-12 09:57:13 UTC
(In reply to W.H. Ding from comment #47)
> Hi, everyone
> 
> I wonder if this issue has to do with the bug-like problem I encountered
> when accessing an unaligned stand-alone global variable (rather than a
> member of a packed struct). A test case is as follows:
> 
> char g_c = 'x';
> int g_d __attribute__((aligned(1))) = 13;
> 
> int main(void)
> {
>     g_c = 'z';
>     //================
>     g_d = 33;                // Crash on, in my case, ARM Cortex-M0
>     //================
>     return 0;
> }

This doesn't work. The aligned attribute is for providing additional alignment hints. The GCC documentation clearly states, that aligned can increase the alignment. So g_d is still 4 byte aligned, and correctly so.

Also, you cannot use the packed attribute (which reduces the alignment to 1) for simple types. You can only use it for structs.

Try a packed struct that contains a single int. That will work.
Comment 50 Sven 2018-04-12 09:58:28 UTC
(In reply to Sven from comment #49)
> This doesn't work. The aligned attribute is for providing additional
> alignment hints. The GCC documentation clearly states, that aligned can
> increase the alignment. So g_d is still 4 byte aligned, and correctly so.

Submitted too soon. Should have been "The GCC documentation clearly states that the aligned attribute can only increase the alignment"
Comment 51 W.H. Ding 2018-04-13 07:25:03 UTC
(In reply to Sven from comment #49)

> This doesn't work. The aligned attribute is for providing additional
> alignment hints. The GCC documentation clearly states, that aligned can
> increase the alignment. So g_d is still 4 byte aligned, and correctly so.

It's really confusing about using 'aligned' attributes in various situations.

Under the title "Specifying Attributes of Variables", the doc says "When used as part of a typedef, the aligned attribute can both increase and decrease alignment" without an example. While under the title "Specifying Attributes of Types", the doc gives an example of typedef but states at the end "The aligned attribute can only increase alignment."

So perhaps, my test case should have been:

typedef int unaligned_int __attribute__((aligned(1)));

char g_c = 'x';
unaligned_int g_d = 13;

int main(void)
{
    g_c = 'z';
    //================
    g_d = 33;                // Crash on, in my case, ARM Cortex-M0
    //================
    return 0;
}

But the result is the same. After all, the 'aligned' attribute took effect in either case since g_d is at an odd address as a consequence.
Comment 52 W.H. Ding 2018-04-16 00:20:33 UTC
(In reply to rguenther@suse.de from comment #48)

So, is there an old bug that covers my problem, or should I file a new one? Thank you.
Comment 53 Eric Gallager 2018-10-16 01:56:33 UTC
latest version of patch adding -Waddress-of-packed-member is here: https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01452.html 

(In reply to W.H. Ding from comment #52)
> (In reply to rguenther@suse.de from comment #48)
> 
> So, is there an old bug that covers my problem, or should I file a new one?
> Thank you.

If there is an old bug, I don't know of it. I'd just file a new one and we can close it as a duplicate if in fact someone can find an old bug it's a dup of
Comment 54 hjl@gcc.gnu.org 2018-12-20 21:42:20 UTC
Author: hjl
Date: Thu Dec 20 21:41:48 2018
New Revision: 267313

URL: https://gcc.gnu.org/viewcvs?rev=267313&root=gcc&view=rev
Log:
C/C++: Add -Waddress-of-packed-member

When address of packed member of struct or union is taken, it may result
in an unaligned pointer value.  This patch adds -Waddress-of-packed-member
to check alignment at pointer assignment and warn unaligned address as
well as unaligned pointer:

$ cat x.i
struct pair_t
{
  char c;
  int i;
} __attribute__ ((packed));

extern struct pair_t p;
int *addr = &p.i;
$ gcc -O2 -S x.i
x.i:8:13: warning: taking address of packed member of ‘struct pair_t’ may result in an unaligned pointer value [-Waddress-of-packed-member]
8 | int *addr = &p.i;
  |             ^

$ cat c.i
struct B { int i; };
struct C { struct B b; } __attribute__ ((packed));

long* g8 (struct C *p) { return p; }
$ gcc -O2 -S c.i -Wno-incompatible-pointer-types
c.i: In function ‘g8’:
c.i:4:18: warning: converting a packed ‘struct C *’ pointer (alignment 1) to ‘long int *’ (alignment 8) may may result in an unaligned pointer value [-Waddress-of-packed-member]
4 | long* g8 (struct C *p) { return p; }
  |                  ^
c.i:2:8: note: defined here
2 | struct C { struct B b; } __attribute__ ((packed));
  |        ^
$

This warning is enabled by default.  Since read_encoded_value_with_base
in unwind-pe.h has

  union unaligned
    {
      void *ptr;
      unsigned u2 __attribute__ ((mode (HI)));
      unsigned u4 __attribute__ ((mode (SI)));
      unsigned u8 __attribute__ ((mode (DI)));
      signed s2 __attribute__ ((mode (HI)));
      signed s4 __attribute__ ((mode (SI)));
      signed s8 __attribute__ ((mode (DI)));
    } __attribute__((__packed__));
  _Unwind_Internal_Ptr result;

and GCC warns:

gcc/libgcc/unwind-pe.h:210:37: warning: taking address of packed member of 'union unaligned' may result in an unaligned pointer value [-Waddress-of-packed-member]
    result = (_Unwind_Internal_Ptr) u->ptr;
                                    ^
we need to add GCC pragma to ignore -Waddress-of-packed-member.

gcc/

	PR c/51628
	* doc/invoke.texi: Document -Wno-address-of-packed-member.

gcc/c-family/

	PR c/51628
	* c-common.h (warn_for_address_or_pointer_of_packed_member): New.
	* c-warn.c (check_alignment_of_packed_member): New function.
	(check_address_of_packed_member): Likewise.
	(check_and_warn_address_of_packed_member): Likewise.
	(warn_for_address_or_pointer_of_packed_member): Likewise.
	* c.opt: Add -Wno-address-of-packed-member.

gcc/c/

	PR c/51628
	* c-typeck.c (convert_for_assignment): Call
	warn_for_address_or_pointer_of_packed_member.

gcc/cp/

	PR c/51628
	* call.c (convert_for_arg_passing): Call
	warn_for_address_or_pointer_of_packed_member.
	* typeck.c (convert_for_assignment): Likewise.

gcc/testsuite/

	PR c/51628
	* c-c++-common/pr51628-1.c: New test.
	* c-c++-common/pr51628-2.c: Likewise.
	* c-c++-common/pr51628-3.c: Likewise.
	* c-c++-common/pr51628-4.c: Likewise.
	* c-c++-common/pr51628-5.c: Likewise.
	* c-c++-common/pr51628-6.c: Likewise.
	* c-c++-common/pr51628-7.c: Likewise.
	* c-c++-common/pr51628-8.c: Likewise.
	* c-c++-common/pr51628-9.c: Likewise.
	* c-c++-common/pr51628-10.c: Likewise.
	* c-c++-common/pr51628-11.c: Likewise.
	* c-c++-common/pr51628-12.c: Likewise.
	* c-c++-common/pr51628-13.c: Likewise.
	* c-c++-common/pr51628-14.c: Likewise.
	* c-c++-common/pr51628-15.c: Likewise.
	* c-c++-common/pr51628-26.c: Likewise.
	* c-c++-common/pr51628-27.c: Likewise.
	* c-c++-common/pr51628-28.c: Likewise.
	* c-c++-common/pr51628-29.c: Likewise.
	* c-c++-common/pr51628-30.c: Likewise.
	* c-c++-common/pr51628-31.c: Likewise.
	* c-c++-common/pr51628-32.c: Likewise.
	* gcc.dg/pr51628-17.c: Likewise.
	* gcc.dg/pr51628-18.c: Likewise.
	* gcc.dg/pr51628-19.c: Likewise.
	* gcc.dg/pr51628-20.c: Likewise.
	* gcc.dg/pr51628-21.c: Likewise.
	* gcc.dg/pr51628-22.c: Likewise.
	* gcc.dg/pr51628-23.c: Likewise.
	* gcc.dg/pr51628-24.c: Likewise.
	* gcc.dg/pr51628-25.c: Likewise.
	* c-c++-common/asan/misalign-1.c: Add
	-Wno-address-of-packed-member.
	* c-c++-common/asan/misalign-2.c: Likewise.
	* c-c++-common/ubsan/align-2.c: Likewise.
	* c-c++-common/ubsan/align-4.c: Likewise.
	* c-c++-common/ubsan/align-6.c: Likewise.
	* c-c++-common/ubsan/align-7.c: Likewise.
	* c-c++-common/ubsan/align-8.c: Likewise.
	* c-c++-common/ubsan/align-10.c: Likewise.
	* g++.dg/ubsan/align-2.C: Likewise.
	* gcc.target/i386/avx512bw-vmovdqu16-2.c: Likewise.
	* gcc.target/i386/avx512f-vmovdqu32-2.c: Likewise.
	* gcc.target/i386/avx512f-vmovdqu64-2.c: Likewise.
	* gcc.target/i386/avx512vl-vmovdqu16-2.c: Likewise.
	* gcc.target/i386/avx512vl-vmovdqu32-2.c: Likewise.
	* gcc.target/i386/avx512vl-vmovdqu64-2.c: Likewise.

libgcc/

	* unwind-pe.h (read_encoded_value_with_base): Add GCC pragma
	to ignore -Waddress-of-packed-member.

Added:
    trunk/gcc/testsuite/c-c++-common/pr51628-1.c
    trunk/gcc/testsuite/c-c++-common/pr51628-10.c
    trunk/gcc/testsuite/c-c++-common/pr51628-11.c
    trunk/gcc/testsuite/c-c++-common/pr51628-12.c
    trunk/gcc/testsuite/c-c++-common/pr51628-13.c
    trunk/gcc/testsuite/c-c++-common/pr51628-14.c
    trunk/gcc/testsuite/c-c++-common/pr51628-15.c
    trunk/gcc/testsuite/c-c++-common/pr51628-16.c
    trunk/gcc/testsuite/c-c++-common/pr51628-2.c
    trunk/gcc/testsuite/c-c++-common/pr51628-26.c
    trunk/gcc/testsuite/c-c++-common/pr51628-27.c
    trunk/gcc/testsuite/c-c++-common/pr51628-28.c
    trunk/gcc/testsuite/c-c++-common/pr51628-29.c
    trunk/gcc/testsuite/c-c++-common/pr51628-3.c
    trunk/gcc/testsuite/c-c++-common/pr51628-30.c
    trunk/gcc/testsuite/c-c++-common/pr51628-31.c
    trunk/gcc/testsuite/c-c++-common/pr51628-32.c
    trunk/gcc/testsuite/c-c++-common/pr51628-4.c
    trunk/gcc/testsuite/c-c++-common/pr51628-5.c
    trunk/gcc/testsuite/c-c++-common/pr51628-6.c
    trunk/gcc/testsuite/c-c++-common/pr51628-7.c
    trunk/gcc/testsuite/c-c++-common/pr51628-8.c
    trunk/gcc/testsuite/c-c++-common/pr51628-9.c
    trunk/gcc/testsuite/gcc.dg/pr51628-17.c
    trunk/gcc/testsuite/gcc.dg/pr51628-18.c
    trunk/gcc/testsuite/gcc.dg/pr51628-19.c
    trunk/gcc/testsuite/gcc.dg/pr51628-20.c
    trunk/gcc/testsuite/gcc.dg/pr51628-21.c
    trunk/gcc/testsuite/gcc.dg/pr51628-22.c
    trunk/gcc/testsuite/gcc.dg/pr51628-23.c
    trunk/gcc/testsuite/gcc.dg/pr51628-24.c
    trunk/gcc/testsuite/gcc.dg/pr51628-25.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.h
    trunk/gcc/c-family/c-warn.c
    trunk/gcc/c-family/c.opt
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/asan/misalign-1.c
    trunk/gcc/testsuite/c-c++-common/asan/misalign-2.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-10.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-2.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-4.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-6.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-7.c
    trunk/gcc/testsuite/c-c++-common/ubsan/align-8.c
    trunk/gcc/testsuite/g++.dg/ubsan/align-2.C
    trunk/gcc/testsuite/gcc.target/i386/avx512bw-vmovdqu16-2.c
    trunk/gcc/testsuite/gcc.target/i386/avx512f-vmovdqu32-2.c
    trunk/gcc/testsuite/gcc.target/i386/avx512f-vmovdqu64-2.c
    trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu16-2.c
    trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu32-2.c
    trunk/gcc/testsuite/gcc.target/i386/avx512vl-vmovdqu64-2.c
    trunk/libgcc/ChangeLog
    trunk/libgcc/unwind-pe.h
Comment 55 H.J. Lu 2018-12-20 21:48:34 UTC
Fixed for GCC 9.
Comment 56 Christophe Lyon 2018-12-21 16:07:13 UTC
The updated testcase pr51628-10 fails at execution on aarch64
FAIL: c-c++-common/pr51628-10.c  -Wc++-compat  execution test
Comment 57 H.J. Lu 2018-12-21 16:19:25 UTC
(In reply to Christophe Lyon from comment #56)
> The updated testcase pr51628-10 fails at execution on aarch64
> FAIL: c-c++-common/pr51628-10.c  -Wc++-compat  execution test

Do you know why it fails?
Comment 58 Christophe Lyon 2018-12-21 16:41:05 UTC
No, I haven't reproduced it manually yet. The log only says:
Execution returned 1
Comment 59 H.J. Lu 2019-01-13 04:44:07 UTC
A missing warning:

[hjl@gnu-cfl-1 pr51628-7]$ cat pr51628-33.c
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);
}
[hjl@gnu-cfl-1 pr51628-7]$ /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 pr51628-33.c
[hjl@gnu-cfl-1 pr51628-7]$
Comment 60 Jakub Jelinek 2019-01-16 14:19:18 UTC
Author: jakub
Date: Wed Jan 16 14:18:47 2019
New Revision: 267970

URL: https://gcc.gnu.org/viewcvs?rev=267970&root=gcc&view=rev
Log:
	PR c/51628
	PR target/88682
	* c-c++-common/pr51628-10.c (unaligned_int128_t): Add
	may_alias attribute.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/pr51628-10.c
Comment 61 hjl@gcc.gnu.org 2019-01-18 13:05:50 UTC
Author: hjl
Date: Fri Jan 18 13:05:18 2019
New Revision: 268075

URL: https://gcc.gnu.org/viewcvs?rev=268075&root=gcc&view=rev
Log:
c-family: Update unaligned adress of packed member check

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.

Added:
    trunk/gcc/testsuite/c-c++-common/pr51628-33.c
    trunk/gcc/testsuite/c-c++-common/pr51628-35.c
    trunk/gcc/testsuite/c-c++-common/pr88664-1.c
    trunk/gcc/testsuite/c-c++-common/pr88664-2.c
    trunk/gcc/testsuite/gcc.dg/pr51628-34.c
Modified:
    trunk/gcc/c-family/ChangeLog
    trunk/gcc/c-family/c-common.h
    trunk/gcc/c-family/c-warn.c
    trunk/gcc/c/ChangeLog
    trunk/gcc/c/c-typeck.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/typeck.c
    trunk/gcc/testsuite/ChangeLog