[U-Boot] packed attribute problem

[Detlev Zundel]

Hi Scott,

> On Tue, 5 Oct 2010 13:43:01 +0200
> Detlev Zundel <dzu at denx.de> wrote:
>
>> Hi Reinhard,
>> 
>> [...]
>> 
>> >> Note that this is with GCC 4.2.2. Even GCC 4.0.0 behaves the same, so
>> >> this is *not* an issue with very recent tool chains.
>> >
>> > OK, for directly adressing elements inside a packed struct;
>> > but the original post said:
>> >
>> > "struct xyz {
>> > 	int	x;
>> > 	int	y;
>> > 	int	z[CONST];
>> > } __attribute__ ((packed));
>> >
>> > struct xyz *abc;
>> > u32 * status_reg = (u32 *)&abc->z[0];
>> >
>> > writel(status, status_reg);"
>> >
>> > So the "status_reg" pointer is in a completely unrelated (to the packed struct)
>> > "u32 *" and still the access is done like it was packed. If the
>> > compiler silently drags that attribute along into the "u32 *"
>> > THAT is really sick!
>> 
>> I cannot follow why you think this is sick - actually I think this is
>> clever.  GCC "knows" that abc points to a packed struct and it has no
>> clue on actual values, so it _has to assume_ it is not aligned.
>
> So instead of the code breaking in the obvious situation where the
> unaligned address is assigned to a pointer type that implies alignment,
> it breaks when sufficient barriers to optimization are added that the
> compiler can no longer keep track of where the value came from?

This is getting somewhat theoretical here, but I do not believe that "a
pointer type implies alignment" in C.  I may be wrong here, but I cannot
think of any statement in the standards in this regard - it's only data
types that imply alignments.  So if this is true, then for pointers gcc
will always have to reason from actual values assigned to them.

> Seems like the right thing is for GCC to warn about the assignment to a
> plain "u32 *", and allow a warningless assignment to something like
> "u32 __attribute__((packed)) *".

Somehow I still feel that gcc should _always_ generate 32-bit accesses
when I dereference a u32 pointer in code.  Silently substituting 4 byte
accesses is simply not the right thing to do (maybe with some
-i-know-what-i-am-doing option, but certainly not by default).  

If such an access traps the machine, then so be it.  Then the code has
to be changed to reflect this in the source code level which will _make
programmers ware of it_.  Doing such a "silent fixup" which extends to
other situations thereby changing general semantics of the C language is
not a good thing IMHO.

Cheers
  Detlev

-- 
Modern technique has made it possible for leisure, within limits, to be not
the prerogative of small privileged classes, but a right evenly distributed
throughout the community.  The morality of  work is the morality of slaves,
and the modern world has no need of slavery.            -- Bertrand Russell
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de