[FFmpeg-devel] [PATCHv3] swscale: prevent undefined behaviour in the PUTRGBA macro

[Leo Izen]

On 7/9/24 2:34 PM, Sean McGovern wrote:
> For even small values of 'asrc', shifting them by 24 bits or more
> will cause arithmetic overflow and be caught by
> GCC's undefined behaviour sanitizer.
> 
> Ensure the values do not overflow by up-casting the bracketed
> expressions involving 'asrc' to int32_t.
> ---
>   libswscale/yuv2rgb.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c
> index 977eb3a7dd..ac0b811f61 100644
> --- a/libswscale/yuv2rgb.c
> +++ b/libswscale/yuv2rgb.c
> @@ -100,9 +100,9 @@ const int *sws_getCoefficients(int colorspace)
>   
>   #define PUTRGBA(dst, ysrc, asrc, i, abase)                              \
>       Y              = ysrc[2 * i];                                       \
> -    dst[2 * i]     = r[Y] + g[Y] + b[Y] + (asrc[2 * i]     << abase);   \
> +    dst[2 * i]     = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i])     << abase);   \
>       Y              = ysrc[2 * i + 1];                                   \
> -    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + (asrc[2 * i + 1] << abase);
> +    dst[2 * i + 1] = r[Y] + g[Y] + b[Y] + ((int32_t)(asrc[2 * i + 1]) << abase);
>   
>   #define PUTRGB48(dst, src, asrc, i, abase)          \
>       Y                = src[ 2 * i];                 \

Are these able to be negative? If not, it may be wise to cast to 
uint32_t instead as left shifting a negative integer is UB.

- Leo Izen (Traneptora)