[Pixman] [PATCH 2/3] mmx: fix unaligned accesses

[Soeren Sandmann]

mattst88 at gmail.com writes:

> @@ -2569,20 +2585,31 @@ mmx_composite_in_8_8 (pixman_implementation_t *imp,
>  	src_line += src_stride;
>  	w = width;
>  
> -	if ((((unsigned long)dest_image & 3) == 0) &&
> -	    (((unsigned long)src_image & 3) == 0))
> +	while (w && (unsigned long)dst & 3)
>  	{
> -	    while (w >= 4)
> -	    {
> -		uint32_t *s = (uint32_t *)src;
> -		uint32_t *d = (uint32_t *)dst;
> +	    uint8_t s, d;
> +	    uint16_t tmp;
> +
> +	    s = *src;
> +	    d = *dst;
>  
> -		*d = store8888 (in (load8888 (*s), load8888 (*d)));
> +	    *dst = MUL_UN8 (s, d, tmp);
>  
> -		w -= 4;
> -		dst += 4;
> -		src += 4;
> -	    }
> +	    src++;
> +	    dst++;
> +	    w--;
> +	}
> +
> +	while (w >= 4)
> +	{
> +	    uint32_t *s = (uint32_t *)src;
> +	    uint32_t *d = (uint32_t *)dst;
> +
> +	    *d = store8888 (in (load8888 (*s), load8888 (*d)));
> +
> +	    w -= 4;
> +	    dst += 4;
> +	    src += 4;
>  	}
>  
>  	while (w--)

The previous code here is totally bogus. These two checks:

       if ((((unsigned long)dst_image & 3) == 0) &&
           (((unsigned long)src_image & 3) == 0))

check that the pointer to the *image struct* is aligned, which is
completely useless. There are some other instances of this problem
(in_n_8_8, add_n_8_8). This was not introduced by your patch, but we may
as well get them fixed anyway, not the least because those will also
SIGBUS on ARM.

Your patch changes the code to just check that the destination is
aligned. Is that actually sufficient? In this code:

   while (w >= 4)
    {
        uint32_t *s = (uint32_t *)src;
        uint32_t *d = (uint32_t *)dst;

        *d = store8888 (in (load8888 (*s), load8888 (*d)));

        w -= 4;
        dst += 4;
        src += 4;
    }

there doesn't seem to be any protection against (*s) being an unaligned
access. The same thing seems to apply to other changes in the patch.


Soren