Subtleties of __attribute__((packed))

Subtleties of __attribute__((packed))

[Phil Endecott]

Dear All,

I used to think that this:

struct foo {
   int a  __attribute__((packed));
   char b __attribute__((packed));
   ... more fields, all packed ...
};

was exactly the same as this:

struct foo {
   int a;
   char b;
   ... more fields ...
} __attribute__((packed));

but it is not, in a subtle way.

Maybe you experts all know this already, but it was new to me so I 
thought I ought to share it, since there have been a few patches 
recently changing the first form to the second form to avoid gcc 
warnings.  (See for example 
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;h=1a7a3f50e40b0a956f44511e42b124a6be98b30b;hp=74f6889f834f1679f09ccd8bbc772fdafd6aade2;hb=e2bf2e26c0915d54208315fc8c9864f1d987217a;f=arch/powerpc/platforms/iseries/main_store.h 
or http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=6a878184c202395ea17212f111ab9ec4b5f6d6ee)

The difference comes when you declare a variable of this struct type 
like this:

   char c;
   struct foo f;

If you use the first form in the declaration of struct foo, a gap will 
be left between c and f so that the start of the struct is aligned.  
But if you use the second form, f will be packed immediately after c, unaligned.

On x86 of course none of this matters for correct behaviour since the 
hardware supports unaligned accesses.  Assuming that your hardware 
doesn't do unaligned accesses then some code will still work.  In 
particular, if you access f like this:

   f.a++;

or probably

   func1(f.a);  // func1 takes an int

then gcc will generate the necessary byte-shuffling code.  However, if 
you write this:

   func2(&f.a);  // func2 takes an int*

then an unaligned pointer is passed to func2.  When func2 dereferences 
the pointer the hardware fails in some way.

GCC does not seem to generate an error or warning when you take the 
address of an unaligned field like this.

I believe that the solution is to write something like this:

struct foo {
   int a;
   char b;
   ... more fields ...
} __attribute__((packed)) __attribute__((aligned(4)));

Now the fields within the struct will be packed, but variables of the 
struct type will be aligned to a 4-byte boundary.

It could be that the kernel code is all safe.  I discovered this issue 
in user code that used structs from the kernel headers.  But I 
encourage anyone who changed their use of __attribute__((packed)) to 
avoid a gcc warning to review what they have done, especially if they 
have tested it only on x86.

Thanks for your attention, and please forgive me if you all know all 
this already!

Regards,

Phil.


(You are welcome to cc: me with any replies.)

Re: Subtleties of __attribute__((packed))

[Frederik Deweerdt]

On Wed, Dec 06, 2006 at 01:20:41PM +0000, Phil Endecott wrote:
> Dear All,
> 
> I used to think that this:
> 
> struct foo {
>   int a  __attribute__((packed));
>   char b __attribute__((packed));
>   ... more fields, all packed ...
> };
> 
> was exactly the same as this:
> 
> struct foo {
>   int a;
>   char b;
>   ... more fields ...
> } __attribute__((packed));
> 
> but it is not, in a subtle way.
> 
This is likely a gcc bug isn't it? The gcc info page states:
  Specifying this attribute for `struct' and `union' types is
  equivalent to specifying the `packed' attribute on each of the
  structure or union members.

Regards,
Frederik

Re: Subtleties of __attribute__((packed))

[Phil Endecott]

Frederik Deweerdt wrote:
> On Wed, Dec 06, 2006 at 01:20:41PM +0000, Phil Endecott wrote:
>> I used to think that this:
>> 
>> struct foo {
>>   int a  __attribute__((packed));
>>   char b __attribute__((packed));
>>   ... more fields, all packed ...
>> };
>> 
>> was exactly the same as this:
>> 
>> struct foo {
>>   int a;
>>   char b;
>>   ... more fields ...
>> } __attribute__((packed));
>> 
>> but it is not, in a subtle way.
>> 
> This is likely a gcc bug isn't it? The gcc info page states:
>   Specifying this attribute for `struct' and `union' types is
>   equivalent to specifying the `packed' attribute on each of the
>   structure or union members.

A gcc *documentation* bug?

I asked on the gcc list about this before posting here, and although 
replies are still coming in the first opinion was "it's doing exactly 
what you asked it to do".

Phil.

Re: Subtleties of __attribute__((packed))

[Jan Blunck]

On 12/6/06, Phil Endecott <phil_arcwk_endecott@chezphil.org> wrote:
> Dear All,
>
> I used to think that this:
>
> struct foo {
>    int a  __attribute__((packed));
>    char b __attribute__((packed));
>    ... more fields, all packed ...
> };
>
> was exactly the same as this:
>
> struct foo {
>    int a;
>    char b;
>    ... more fields ...
> } __attribute__((packed));
>
> but it is not, in a subtle way.
>

The same code is generated. The difference is that usually packing the
whole struct isn't as error-prone as packing every element. Besides
that the gcc warns about packing objects that have an alignment of 1.
This is the reason why we should use the second approach.

Re: Subtleties of __attribute__((packed))

[Phil Endecott]

Jan Blunck wrote:
> On 12/6/06, Phil Endecott <phil_arcwk_endecott@chezphil.org> wrote:
>> I used to think that this:
>>
>> struct foo {
>>    int a  __attribute__((packed));
>>    char b __attribute__((packed));
>>    ... more fields, all packed ...
>> };
>>
>> was exactly the same as this:
>>
>> struct foo {
>>    int a;
>>    char b;
>>    ... more fields ...
>> } __attribute__((packed));
>>
>> but it is not, in a subtle way.
>>
>
> The same code is generated. [...]

I don't think so.  Example:

struct test {
   int a __attribute__((packed));
   int b __attribute__((packed));
};

char c = 1;
struct test t = { .a=2, .b=3 };

$ arm-linux-gnu-gcc -O2 -S -W -Wall test1.c

	.file	"test2.c"
	.global	c
	.data
	.type	c, %object
	.size	c, 1
c:
	.byte	1
	.global	t
	.align	2               <<<<<<<<===== t is aligned
	.type	t, %object
	.size	t, 8
t:
	.word	2
	.word	3
	.ident	"GCC: (GNU) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)"


Compare with:

struct test {
   int a;
   int b;
} __attribute__((packed));

char c = 1;
struct test t = { .a=2, .b=3 };

$ arm-linux-gnu-gcc -O2 -S -W -Wall test2.c

	.file	"test1.c"
	.global	c
	.data
	.type	c, %object
	.size	c, 1
c:
	.byte	1
	.global	t                    <<<<<<  "align" has gone, t is unaligned
	.type	t, %object
	.size	t, 8
t:
	.4byte	2
	.4byte	3
	.ident	"GCC: (GNU) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)"



Phil.

Re: Subtleties of __attribute__((packed))

[Jan Blunck]

On Wed, Dec 06, Phil Endecott wrote:

> I don't think so.  Example:
> 
> struct test {
>   int a __attribute__((packed));
>   int b __attribute__((packed));
> };
> 
> char c = 1;
> struct test t = { .a=2, .b=3 };
> 
> $ arm-linux-gnu-gcc -O2 -S -W -Wall test1.c
> 
> 	.file	"test2.c"
> 	.global	c
> 	.data
> 	.type	c, %object
> 	.size	c, 1
> c:
> 	.byte	1
> 	.global	t
> 	.align	2               <<<<<<<<===== t is aligned
> 	.type	t, %object
> 	.size	t, 8
> t:
> 	.word	2
> 	.word	3
> 	.ident	"GCC: (GNU) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)"
> 
> 
> Compare with:
> 
> struct test {
>   int a;
>   int b;
> } __attribute__((packed));
> 
> char c = 1;
> struct test t = { .a=2, .b=3 };
> 
> $ arm-linux-gnu-gcc -O2 -S -W -Wall test2.c
> 
> 	.file	"test1.c"
> 	.global	c
> 	.data
> 	.type	c, %object
> 	.size	c, 1
> c:
> 	.byte	1
> 	.global	t                    <<<<<<  "align" has gone, t is unaligned
> 	.type	t, %object
> 	.size	t, 8
> t:
> 	.4byte	2
> 	.4byte	3
> 	.ident	"GCC: (GNU) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)"
> 

Maybe the arm backend is somehow broken. AFAIK (and I verfied it on S390 and
i386) the alignment shouldn't change.

struct foo {
	int a;
	char b;
	int c;
};

struct bar1 {
	char a __attribute__((__packed__));
	struct foo b __attribute__((__packed__));
};

struct bar2 {
	char a;
	struct foo b;
} __attribute__((__packed__));

struct bar3 {
	char a;
	struct foo b;
};

struct bar1 packed1 = { 10, { 20, 30, 40 } };
struct bar2 packed2 = { 50, { 60, 70, 80 } };
struct bar3 unpacked = { 90, { 100, 110, 120 } };

s390x-linux-gcc -S packed2.c:
packed2.c:9: warning: '__packed__' attribute ignored for field of type 'char'

	.file	"packed2.c"
.globl packed1
.data
	.align	2
	.type	packed1, @object
	.size	packed1, 13
packed1:
	.byte	10
	.4byte	20
	.byte	30
	.zero	3
	.4byte	40
.globl packed2
	.align	2
	.type	packed2, @object
	.size	packed2, 13
packed2:
	.byte	50
	.4byte	60
	.byte	70
	.zero	3
	.4byte	80
.globl unpacked
	.align	4
	.type	unpacked, @object
	.size	unpacked, 16
unpacked:
	.byte	90
	.zero	3
	.long	100
	.byte	110
	.zero	3
	.long	120
	.ident	"GCC: (GNU) 4.1.2 20060531 (prerelease) (SUSE Linux)"
	.section	.note.GNU-stack,"",@progbits

Re: Subtleties of __attribute__((packed))

[Andreas Schwab]

Jan Blunck <jblunck@suse.de> writes:

> Maybe the arm backend is somehow broken.

A packed structure is something quite different than a structure of packed
members.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Subtleties of __attribute__((packed))

[Phil Endecott]

Jan Blunk wrote:
> Maybe the arm backend is somehow broken. AFAIK (and I verfied it on S390 and
> i386) the alignment shouldn't change.

To see a difference with your example structs you need to compare these two:

struct wibble1 {
   char c;
   struct bar1 b1;
};

struct wibble2 {
   char c;
   struct bar2 b2;
};

struct wibble1 w1 = { 1, { 2, {3,4,5} } };
struct wibble2 w2 = { 1, { 2, {3,4,5} } };

Can you try that with your compilers?  I get:

w1:
	.byte	1
	.space	3    <<<----
	.byte	2
	.4byte	3
	.byte	4
	.space	3
	.4byte	5
	.space	3
	.global	w2
	.align	2
	.type	w2, %object
	.size	w2, 16
w2:
	.byte	1
	.byte	2
	.4byte	3
	.byte	4
	.space	3
	.4byte	5
	.space	2


Phil.

Re: Subtleties of __attribute__((packed))

[Jan Blunck]

On Wed, Dec 06, Phil Endecott wrote:

> 
> To see a difference with your example structs you need to compare these two:
> 
> struct wibble1 {
>   char c;
>   struct bar1 b1;
> };
> 
> struct wibble2 {
>   char c;
>   struct bar2 b2;
> };
> 
> struct wibble1 w1 = { 1, { 2, {3,4,5} } };
> struct wibble2 w2 = { 1, { 2, {3,4,5} } };
> 
> Can you try that with your compilers?  I get:
> 

As I expected, I get:

	.file	"packed2a.c"
.globl w1
.data
	.align	2
	.type	w1, @object
	.size	w1, 14
w1:
	.byte	1
	.byte	2
	.4byte	3
	.byte	4
	.zero	3
	.4byte	5
.globl w2
	.align	2
	.type	w2, @object
	.size	w2, 14
w2:
	.byte	1
	.byte	2
	.4byte	3
	.byte	4
	.zero	3
	.4byte	5
	.ident	"GCC: (GNU) 4.1.2 20060531 (prerelease) (SUSE Linux)"
	.section	.note.GNU-stack,"",@progbits

Re: Subtleties of __attribute__((packed))

[Jan Blunck]

On Wed, Dec 06, Andreas Schwab wrote:

> Jan Blunck <jblunck@suse.de> writes:
> 
> > Maybe the arm backend is somehow broken.
> 
> A packed structure is something quite different than a structure of packed
> members.
> 

Well, right ... and where do you see a structure of packed members?

http://gcc.gnu.org/onlinedocs/gcc-4.1.1/gcc/Type-Attributes.html#Type-Attributes

Re: Subtleties of __attribute__((packed))

[Andreas Schwab]

Jan Blunck <jblunck@suse.de> writes:

> Well, right ... and where do you see a structure of packed members?

Read <http://sourceware.org/ml/binutils/2006-12/msg00039.html>.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Subtleties of __attribute__((packed))

[Russell King]

On Wed, Dec 06, 2006 at 04:54:39PM +0100, Jan Blunck wrote:
> Maybe the arm backend is somehow broken. AFAIK (and I verfied it on S390 and
> i386) the alignment shouldn't change.

Please read the info pages:

`packed'
     This attribute, attached to an `enum', `struct', or `union' type
     definition, specifies that the minimum required memory be used to
     represent the type.

     Specifying this attribute for `struct' and `union' types is
     equivalent to specifying the `packed' attribute on each of the
     structure or union members.  Specifying the `-fshort-enums' flag
     on the line is equivalent to specifying the `packed' attribute on
     all `enum' definitions.

Note that it says *nothing* about alignment.  It says "minimum required
memory be used to represent the type." which implies that the internals
of the structure are packed together as tightly as possible.

It does not say "and as such the struct may be aligned to any alignment".

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: Subtleties of __attribute__((packed))

[David Miller]

From: Russell King <rmk+lkml@arm.linux.org.uk>
Date: Wed, 6 Dec 2006 17:54:23 +0000

> It does not say "and as such the struct may be aligned to any alignment".

Consider the implication for arrays and pointer arithmetic, it's just
a logical consequence, that's all.  It's why the alignment cannot be
assumed for packed structures.

If you have, for example:

struct example {
	char b;
	short c;
} __attribute__((packed));

And I give you:

extern void foo(struct example *p);

and go:

	foo(p + 1);

It is clear that the compiler must assume that all instances
of a packed structure are not necessarily aligned properly.

Even if "p" is aligned, "p + 1" definitely won't be.  And this
goes for any array indexing of the given packed structure.

That's why every pointer to such a struct must be assumed to be
unaligned in these cases.

So even though the documentation may not say this explicitly, it's an
implicit logical side effect of packed structures.

Re: Subtleties of __attribute__((packed))

[Jan Blunck]

On Wed, Dec 06, Russell King wrote:

> On Wed, Dec 06, 2006 at 04:54:39PM +0100, Jan Blunck wrote:
> > Maybe the arm backend is somehow broken. AFAIK (and I verfied it on S390 and
> > i386) the alignment shouldn't change.
> 

Once again: I refered to "packed attribute on the struct vs. packed attribute
on each member of the struct". The alignment shouldn't be different.

> Please read the info pages:
> 
> `packed'
>      This attribute, attached to an `enum', `struct', or `union' type
>      definition, specifies that the minimum required memory be used to
>      represent the type.
> 
>      Specifying this attribute for `struct' and `union' types is
>      equivalent to specifying the `packed' attribute on each of the
>      structure or union members.  Specifying the `-fshort-enums' flag
>      on the line is equivalent to specifying the `packed' attribute on
>      all `enum' definitions.
> 
> Note that it says *nothing* about alignment.  It says "minimum required
> memory be used to represent the type." which implies that the internals
> of the structure are packed together as tightly as possible.
> 
> It does not say "and as such the struct may be aligned to any alignment".
> 

And this is why it makes sense to think about align attribute when you use
packed.

Re: Subtleties of __attribute__((packed))

[Andreas Schwab]

Jan Blunck <jblunck@suse.de> writes:

> Once again: I refered to "packed attribute on the struct vs. packed attribute
> on each member of the struct". The alignment shouldn't be different.

You are mistaken.  The alignment of a non-packed structure can be greater
than the maximum alignment of the containing members.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."