Re: Update on unaligned traps...
There's more going on here than is initially apparent. The proposed fix
happens to work in this case, but will likely fail in other similar
situations. I think a more reliable solution is needed.
Chris Chimelis wrote:
> Ok. I'm going to relate the discussion to fdisk since that's the first
> programme where I noticed the problem.
>
> fidks uses a struct from the /usr/include/linux/genhd.h header file:
>
> struct partition {
> unsigned char boot_ind; /* 0x80 - active */
> unsigned char head; /* starting head */
> unsigned char sector; /* starting sector */
> unsigned char cyl; /* starting cylinder */
> unsigned char sys_ind; /* What partition type */
> unsigned char end_head; /* end head */
> unsigned char end_sector; /* end sector */
> unsigned char end_cyl; /* end cylinder */
> unsigned int start_sect; /* starting sector counting from 0 */
> unsigned int nr_sects; /* nr of sectors in partition */
> };
>
> Basically, it holds all of the information on each device. The way that
> fdisk uses this struct is by creating an array of pointers called
> part_type. Because of the way that the pointers are assigned (by
> assignment and not by memcpy's) not to mention that fdisk does some
> strange operations on the pointers, it turns out that the part_type[]
> array itself is not aligned properly for the 16-byte struct.
More specifically, the addresses the array (actually called part_table)
contains are not 32-bit aligned, because they reference the disk
partition table held in a memory image of the boot sector.
> Now, since the Alpha doesn't care about the alignment of the char members
> in the struct, accessing them doesn't generate any unaligned trap
> warnings, but the same does not go for the unsigned int members. The
> Alpha (and at least one other processor I know of) requires strict
> alignment of all but the chars.
>
> The other thing that may have been noticed about the fdisk warnings is
> that there were only four generated, but the programme actually accesses
> those unsigned ints more times than that. Well, after perusing the kernel
> sources a bit, I noticed that the kernel stops warning about unaligned
> traps after four per process.
>
> Ok...on to the fix.
>
> In normal compilation, structs are often broken up in memory a bit and
> padded creatively/as needed. But, in this case, we can't allow that or
> else we will get unaligned trap warnings. So, we're going to use a gcc
> extention to force the struct to be packed together for alignment...like
> this:
The reason the compiler normally pads structures is precisely to ensure
proper alignment for structure members. To do this, it must assume that
the structure itself resides at an address whose alignment is at least
as strict as that required by any of its members. In this case, this
assumption is not valid, because the structure is being mapped into a
binary image at an arbitrary address (determined by the layout of the
disk boot sector).
> struct partition {
> unsigned char boot_ind; /* 0x80 - active */
> unsigned char head; /* starting head */
> unsigned char sector; /* starting sector */
> unsigned char cyl; /* starting cylinder */
> unsigned char sys_ind; /* What partition type */
> unsigned char end_head; /* end head */
> unsigned char end_sector; /* end sector */
> unsigned char end_cyl; /* end cylinder */
> unsigned int start_sect; /* starting sector counting from 0 */
> unsigned int nr_sects; /* nr of sectors in partition */
> } __attribute__((packed));
>
> The __attribute__((packed)) is an extension provided by gcc to force the
> struct to be aligned exactly as we need it to be.
try this:
struct partition {
u_char boot_ind; u_char head; u_char sector; u_char cyl;
u_char sys_ind; u_char end_head; u_char end_sector; u_char end_cyl;
u_int start_sect; u_int nr_sects;
};
struct partition_packed {
u_char boot_ind; u_char head; u_char sector; u_char cyl;
u_char sys_ind; u_char end_head; u_char end_sector; u_char end_cyl;
u_int start_sect; u_int nr_sects;
} __attribute__((packed));
printf("struct partition: %d struct partition_packed: %d\n",
sizeof(struct partition),sizeof(struct partition_packed));
The compiler reports the size of both structures as 16 bytes. Because
the int members are preceded by eight char members, no padding is
necessary, and declaring the structure to be packed does not change the
layout at all. In this case, this is fortunate, because if the structure
layout was changed, it would no longer match the arrangement of the disk
boot sector which it is mapped into, and the program would not work.
This change fixes the alignment traps for a different reason: the
compiler generates different code to access the members of the packed
struct.
u_int read_partition(struct partition *p)
{
return(p->start_sect);
}
u_int read_partition_packed(struct partition_packed *p)
{
return(p->start_sect);
}
gcc compiles this into the following instruction sequences:
.ent read_partition
read_partition:
read_partition..ng:
.frame $30,0,$26,0
.prologue 0
ldl $0,8($16)
ret $31,($26),1
.end read_partition
.ent read_partition_packed
read_partition_packed:
read_partition_packed..ng:
.frame $30,0,$26,0
.prologue 0
ldq_u $3,8($16) ; load byte 0
addq $16,8,$1
extbl $3,$1,$3
ldq_u $1,9($16) ; load byte 1
addq $16,9,$2
extbl $1,$2,$1
sll $1,8,$1
bis $1,$3,$1
ldq_u $2,10($16) ; load byte 2
addq $16,10,$3
extbl $2,$3,$2
sll $2,16,$2
bis $2,$1,$2
ldq_u $0,11($16) ; load byte 3
addq $16,11,$16
extbl $0,$16,$0
sll $0,24,$0
bis $0,$2,$0
addl $0,$31,$0
ret $31,($26),1
.end read_partition_packed
The code for reading the packed structure member makes no assumptions
about the alignment of that data; it reads it a byte at a time. (This is
NOT the most efficient way of doing this; the architecture reference
manual lists a six-instruction sequence.) The "ldq_u" instruction (Load
Quadword Unaligned) avoids the alignment trap by clearing the three
least significant bits of the effective address.
> The good news with doing this is that it will not affect architectures
> that do not require such strict alignment, so you don't have to surround
> it with #ifdef's and stuff. The bad news is...well...there is no bad news
> :P
>
> That's about it. Thanks again to Richard Henderson for the solution.
>
> Hopefully, unless our programmes use info directly from the kernel, we
> shouldn't encounter too many unaligned traps.
This technique works in this case because of the fortuitous circumstance
that the structure in question was already as packed as it could get; if
this had not been so, the program would have malfunctioned, as discussed
above. It therefore does not qualify as a general solution. Note that
this is probably not an issue with kernel data structures; the kernel
header files will contain the relevant structure declaration, and the
appropriate code will be generated, whether or not the structure was
declared as packed, because kernel and user code will have the same
understanding of its layout and alignment.
The problem is only likely to occur with structures which must be binary
compatible across different architectures or which are mapped into
binary images at computed addresses; fdisk meets both these criteria. I
suggest another solution in such cases.
int32_t read_int32_unaligned(char *);
void write_int32_unaligned(char *,u_int32_t);
int64_t read_int64_unaligned(char *);
void write_int64_unaligned(char *,u_int64_t);
These functions can be written in assembly language using the most
efficient instruction sequence; on the Alpha, this appears to be about
six to twelve instructions. On architectures where alignment does not
matter, they can be coded as macros which simply convert and dereference
the pointer:
#define read_int32_unaligned(p) (*(int32_t *)(p))
#define write_int32_unaligned(p,x) (*(u_int32_t *)(p)=(x))
#define read_int64_unaligned(p) (*(int64_t *)(p))
#define write_int64_unaligned(p,x) (*(u_int64_t *)(p)=(x))
The typenames are platform-independent typedefs from glibc; they are
used because the width of the built-in integer types is not guaranteed
to be the same across different CPUs, or even different compilers. The
return type and parameter are declared as signed and unsigned
respectively, so that the same function will work for both signed and
unsigned values without the compiler complaining about implicit unsigned
to signed conversions. The compiler appears to assume that short
pointers are 16-bit aligned, so word versions are probably required as
well. The compiler makes no assumptions about the alignment of char
pointers, so byte versions are unnecessary.
Instead of a structure declaration, macros can be defined to access
specific fields by name:
#define start_sect(p) read_int32_unaligned((char *)(p)+8)
#define wr_start_sect(p,x) write_int32_unaligned((char *)(p)+8,(x))
#define nr_sects(p) read_int32_unaligned((char *)(p)+12)
#define wr_nr_sects(p,x) write_int32_unaligned((char *)(p)+12,(x))
At the cost of slightly more complicated syntax, this solution should be
portable across all architectures, regardless of how the native compiler
lays out structures, and should completely avoid alignment traps on
systems where this matters.
Ian Willmott
Northern Telecom
Ottawa Ontario Canada
willmott@nortel.ca
--
TO UNSUBSCRIBE FROM THIS MAILING LIST: e-mail the word "unsubscribe" to
debian-alpha-request@lists.debian.org .
Trouble? e-mail to templin@bucknell.edu .
Reply to: