[Bug c/51628] New: __attribute__((packed)) is unsafe in some cases

[Bug c/51628] New: __attribute__((packed)) is unsafe in some cases

[Keith.S.Thompson at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

             Bug #: 51628
           Summary: __attribute__((packed)) is unsafe in some cases
    Classification: Unclassified
           Product: gcc
           Version: 4.5.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: Keith.S.Thompson@gmail.com


Created attachment 26147
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26147
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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[Keith.S.Thompson at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #1 from Keith Thompson <Keith.S.Thompson at gmail dot com> 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".

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 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

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[Keith.S.Thompson at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #3 from Keith Thompson <Keith.S.Thompson at gmail dot com> 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).

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-12-20
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org, rguenth at gcc dot
                   |                            |gnu.org
     Ever Confirmed|0                           |1

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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 ;-)

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #8 from rguenther at suse dot de <rguenther at suse dot 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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).

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot 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)

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #13 from rguenther at suse dot de <rguenther at suse dot 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)

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #14 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[peter at axium dot co.nz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

incrediball <peter at axium dot co.nz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter at axium dot co.nz

--- Comment #15 from incrediball <peter at axium dot co.nz> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> 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 ...

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #17 from Richard Biener <rguenth at gcc dot gnu.org> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[peter at axium dot co.nz]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #18 from incrediball <peter at axium dot co.nz> 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #20 from rguenther at suse dot de <rguenther at suse dot 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[ebotcazou at gcc dot gnu.org]

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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #22 from rguenther at suse dot de <rguenther at suse dot 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.

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[sven.koehler at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #23 from Sven <sven.koehler at gmail dot com> ---
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

[Bug c/51628] __attribute__((packed)) is unsafe in some cases

[sven.koehler at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51628

--- Comment #24 from Sven <sven.koehler at gmail dot com> ---
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.