PR 1098: "struct ipovly" not packed.

[Jay Monkman]

Ralf Corsepius wrote:
> On Sat, 2006-06-24 at 14:50 +1000, Chris Johns wrote:
>> This PR:
>>
>> http://www.rtems.com/cgi-bin/gnatsweb.pl?cmd=view&database=RTEMS&pr=1098
>>
>> contains a patch related to this thread:
>>
>> http://www.rtems.org/ml/rtems-users/2006/may/msg00060.html
>>
>> It requires the packing of "struct ipovly" and "struct tcpiphdr".
>>
>> I would like to commit this patch and close the PR but before I do this 
>> I would like to know why this patch is needed for a specific ARM board 
>> and network device and not others.
> And I would like to know why this patch is needed at all.
> 
> 1. I fail to understand why this patch is required at all, because I
> don't see why __attribute__(packed) should have any influence on the
> structs you are trying to pack.
> 
> 2. __attribute__(packed) is hack and should not be used.
> 
> Can you provide an isolated example exposing the problem (Which?) you
> are trying to solve?
> 
> Ralf
> 
> 

Sorry for the late response - I just saw this related to arm.

That patch doesn't what it's supposed to. At least not with my compiler -
gcc version 4.0.1 (RTEMS gcc-4.0.1-20050810/newlib-1.13.0-20050810-4)

Putting __attribute__((packed)) at the end of struct doesn't pack the structure.
I assume that it would cause an array of odd sized structs to be packed, but I'm
not sure.

If you want the structure to be packed, you need to put __attribute__((packed))
on each element. At least that's my understanding. See my test at the end of
this email

I don't understand how this patch would help anyone, since I don't see how it
would change the generated object code.

Networking has been working on ARM for quite a while now. At least in 4.6 and
the 4.7 branch. (I haven't tested the latest code in the 4.7 branch, but as of a
month ago it worked.)

I'm not 100% convinced that the network code is bug-free on ARM because the
stack doesn't align data nicely, but I know that it works for the common cases.
It's been tested with millions of packets on hundreds of devices with no problems.



I do disagree that __attribute__((packed)) should never be used. Without
something like that, there's no guarantee how the compiler will align the data.
I've seen it use two byte alignment on CPUs like x86. When we first started
trying to get the network stack working on ARM we had to fix several of the
problems.

Do you know of a better alternative?



----------------------------------------------------------------------------
Here's some example source code with different uses of __attribute__((packed))
and the generated output (arm-rtems4.7-gcc -O1 -s test.c).

I actually get a warning about __attribute__((packed)) being ignored for the
struct where it comes at the end (s2_t).



----------------------------------------------------------------------------
Test code:
----------------------------------------------------------------------------
typedef struct {
    long  l1;
    short s1;
    long  l2;
} s1_t;

typedef struct {
    long  l1;
    short s1;
    long  l2;
} s2_t __attribute__((packed));  /* does nothing */

typedef struct {
    long  l1 __attribute__((packed));
    short s1 __attribute__((packed));
    long  l2 __attribute__((packed));
} s3_t;


extern void func(void *);

void test1(void)
{

    s1_t *s1 = (s1_t *)0x0;

    func(s1);
    func(&s1->l2);
}

void test2(void)
{

    s2_t *s2 = (s2_t *)0x0;

    func(s2);
    func(&s2->l2);
}

void test3(void)
{

    s3_t *s3 = (s3_t *)0x0;

    func(s3);
    func(&s3->l2);
}

----------------------------------------------------------------------------
Generated assembly (abridged)
----------------------------------------------------------------------------
test1:
	str	lr, [sp, #-4]!
	mov	r0, #0
	bl	func
	mov	r0, #8      @ Element s1->l2 at offset of 8 - not packed
	bl	func
	ldr	pc, [sp], #4

test2:
	str	lr, [sp, #-4]!
	mov	r0, #0
	bl	func
	mov	r0, #8      @ Element s2->l2 at offset of 8 - not packed
	bl	func
	ldr	pc, [sp], #4

test3:
	str	lr, [sp, #-4]!
	mov	r0, #0
	bl	func
	mov	r0, #6      @ Element s3->l2 at offset of 6 - packed
	bl	func
	ldr	pc, [sp], #4