From 7b7c5fe69443085250ce8fc3511dddd0cfa2d756 Mon Sep 17 00:00:00 2001 From: Sean McGovern Date: Tue, 2 Jul 2024 23:07:54 -0400 Subject: [RFC PATCH] swscale: prevent undefined behaviour in the PUTRGBA macro --- Notes: Sending this as an RFC as I'm not sure it is the correct fix. It does address the undefined behaviour of the C version of yuv2rgb tested in 'fate-checkasm-sw_yuv2rgb', but since swscale new territory for me I'm not sure what I propose is appropriate. I think the AltiVec version will still need a fix after this, and Ramiro suggested there might be an issue in the LoongArch version as well. Conversation points: - Is usage of '__typeof__' OK? Is it a GCC-ism? In the rest of the codebase it seems to be limited to AltiVec acceleration. - Should this instead just cast the shifted arguments to 'int32_t' and be done with it? Aside: the macro soup in this file has very high cognitive complexity. libswscale/yuv2rgb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c index 977eb3a7dd..ab5192aab4 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] + ((__typeof__(*dst))(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] + ((__typeof__(*dst))(asrc[2 * i + 1]) << abase); #define PUTRGB48(dst, src, asrc, i, abase) \ Y = src[ 2 * i]; \ -- 2.39.2