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

[Nuo Mi <nuomi2021@gmail.com>]

On Sun, Feb 11, 2024 at 5:00 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> 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@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 '\'.

oh, you are so careful

>


> >
> >  #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).
>
Think about it for about 3 seconds, then give up :)
👍

> 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.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".