[FFmpeg-devel] [PATCH] avcodec/hevcdec: fix out of bounds index -1 for inter prediction

[Andreas Rheinhardt]

Nuo Mi:
> It's a false positive. We use -1 to index an array, but it's not actually used.
> This patch will make the "GCC UndefinedBehaviorSanitizer" happy.
> 
> Found by: checkasm-hevc_pel in https://fate.ffmpeg.org/report.cgi?time=20240211011905&slot=x86_64-archlinux-gcc-ubsan
> Reported-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/hevcdsp_template.c | 6 +++---
>  libavcodec/x86/hevcdsp_init.c | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/hevcdsp_template.c b/libavcodec/hevcdsp_template.c
> index 9b48bdf08e..a4b8901e5d 100644
> --- a/libavcodec/hevcdsp_template.c
> +++ b/libavcodec/hevcdsp_template.c
> @@ -301,9 +301,9 @@ IDCT_DC(32)
>  //
>  ////////////////////////////////////////////////////////////////////////////////
>  #define ff_hevc_pel_filters ff_hevc_qpel_filters
> -#define DECL_HV_FILTER(f)                                  \
> -    const uint8_t *hf = ff_hevc_ ## f ## _filters[mx - 1]; \
> -    const uint8_t *vf = ff_hevc_ ## f ## _filters[my - 1];
> +#define DECL_HV_FILTER(f)                                               \
> +    const uint8_t *hf = mx ? ff_hevc_ ## f ## _filters[mx - 1] : NULL;  \
> +    const uint8_t *vf = my ? ff_hevc_ ## f ## _filters[my - 1] : NULL;
>  
>  #define FW_PUT(p, f, t)                                                                                   \
>  static void FUNC(put_hevc_## f)(int16_t *dst, const uint8_t *src, ptrdiff_t srcstride, int height,        \
> diff --git a/libavcodec/x86/hevcdsp_init.c b/libavcodec/x86/hevcdsp_init.c
> index 31e81eb11f..e0f65177c4 100644
> --- a/libavcodec/x86/hevcdsp_init.c
> +++ b/libavcodec/x86/hevcdsp_init.c
> @@ -87,9 +87,9 @@ IDCT_FUNCS(avx)
>  
>  
>  #define ff_hevc_pel_filters ff_hevc_qpel_filters
> -#define DECL_HV_FILTER(f)                                  \
> -    const uint8_t *hf = ff_hevc_ ## f ## _filters[mx - 1]; \
> -    const uint8_t *vf = ff_hevc_ ## f ## _filters[my - 1];
> +#define DECL_HV_FILTER(f)                                               \
> +    const uint8_t *hf = mx ? ff_hevc_ ## f ## _filters[mx - 1] : NULL;  \
> +    const uint8_t *vf = my ? ff_hevc_ ## f ## _filters[my - 1] : NULL;  \

Spurious addition of trailing '\'.

>  
>  #define FW_PUT(p, a, b, depth, opt) \
>  void ff_hevc_put_hevc_ ## a ## _ ## depth ## _##opt(int16_t *dst, const uint8_t *src, ptrdiff_t srcstride,   \

I have just sent an alternative solution for this that avoids all these
branches (which the compiler will not be able to eliminate).
Unfortunately I can't test mips and loongarch myself; there is a
fate-runner for the latter, but nothing for the former. So hopefully
someone can test mips.

- Andreas

PS: The aarch64 code (hevcdsp_[eq]pel_neon.S) already offsets its
filters via dummy arrays. I wonder whether it could now reuse the
ordinary ones.