[cairo] fix alignment issue on SPARC processors

[Uli Schlachter]

Hi,

Am 04.12.2014 um 18:12 schrieb Nicolas Setton:
> There is an alignment issue on SPARC processors which causes a SIGBUS when
> trying to write data to edges in the polygon scan converter.

Could you provide some more information about this? I guess you are on 32bit
SPARC since otherwise all members of struct _pool_chunk already be 64bit in size.

So size_t is a 32 bit type for you and the prev_chunk member thus starts aligned
to a 64 bit boundary for you. Since it is a 32 bit pointer, the end of the
struct is no longer 32bit-aligned. You fix this by adding a 16-bit member in
front of prev_chunk so that (plus the 16-bit hole that this forces) the end of
struct becomes 64-bit aligned.

Did I understand this correctly so far?

Also, where exactly does the SIGBUS happen? I guess that you are refering to the
int64_t dy member of struct edge which e.g. polygon_add_edge() allocates from
some pool via pool_alloc(). So if the struct itself isn't aligned to a 64 bit
boundary, the dy member isn't either.

It would be nice if you submit your patch in the format produced by git
format-patch. That includes a commit message and tracks you as the author of
this, instead of someone else claiming the fame for this. All of the above
reasoning and thoughts about this change could go into the commit message.

The big downside of this: On 64bit platforms this adds an unused 64 bit hole to
the struct.

I would instead suggest to add a new member "int64_t data" to the struct with a
suitable comment explaining why it is of type int64_t. Instead of using
((unsigned char *)chunk + sizeof(*chunk)) for calculating the beginning of the
data area of the chunk, the code could then use (unsigned char*)&chunk->data
which IMO is more readable anyway.

However, I guess that such a change is more complicated since it requires
reviewing the code more carefully since now sizeof(*chunk) is no longer the size
of the "chunk header"...

> Here is the patch that we've applied to circumvent the issue. 
> 
> Hope this helps!
> 
> Nicolas
> 
> diff --git a/src/cairo-tor-scan-converter.c b/src/cairo-tor-scan-converter.c
> index 14922d0..d811825 100644
> --- a/src/cairo-tor-scan-converter.c
> +++ b/src/cairo-tor-scan-converter.c
> @@ -273,6 +273,11 @@ struct _pool_chunk {
>      /* # bytes total in this chunk */
>      size_t capacity;
>  
> +    /* Pad to fix alignment issue on sparc: the long long integers
> +     * are aligned on 64-bit boundaries. Make sure that this is the
> +     * case in the memory pool by bumping the size of this struct. */
> +    short padding;
> +
>      /* Pointer to the previous chunk or %NULL if this is the sentinel
>       * chunk in the pool header. */
>      struct _pool_chunk *prev_chunk;
> 

Cheers,
Uli
-- 
"For saving the Earth.. and eating cheesecake!"