| Message ID | TYSPR06MB6433517914B4C4432D6799E5AA492@TYSPR06MB6433.apcprd06.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/hevcdec: fix out of bounds index -1 for inter prediction | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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 '\'. > > #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.
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; \ #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, \