[FFmpeg-devel] [PATCH] avcodec/fft_template: Fix multiple runtime error: signed integer overflow: -1943918714 - 1935113003 cannot be represented in type 'int'
Michael Niedermayer
michael at niedermayer.cc
Fri May 26 02:57:42 EEST 2017
On Thu, May 25, 2017 at 03:29:59PM +0100, Rostislav Pehlivanov wrote:
> On 25 May 2017 at 15:10, Michael Niedermayer <michael at niedermayer.cc> wrote:
>
> > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408
> >
> > Found-by: continuous fuzzing process https://github.com/google/oss-
> > fuzz/tree/master/projects/ffmpeg
> > Signed-off-by
> > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg%0ASigned-off-by>:
> > Michael Niedermayer <michael at niedermayer.cc>
> > ---
> > libavcodec/fft_template.c | 50 +++++++++++++++++++++++-------
> > -----------------
> > 1 file changed, 25 insertions(+), 25 deletions(-)
> >
> > diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
> > index 480557f49f..e3a37e5d69 100644
> > --- a/libavcodec/fft_template.c
> > +++ b/libavcodec/fft_template.c
> > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) {
> >
> > int nbits, i, n, num_transforms, offset, step;
> > int n4, n2, n34;
> > - FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > + SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8;
> > FFTComplex *tmpz;
> > const int fft_size = (1 << s->nbits);
> > int64_t accu;
> > @@ -260,14 +260,14 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> > {
> > offset = ff_fft_offsets_lut[n] << 2;
> > tmpz = z + offset;
> >
> > - tmp1 = tmpz[0].re + tmpz[1].re;
> > - tmp5 = tmpz[2].re + tmpz[3].re;
> > - tmp2 = tmpz[0].im + tmpz[1].im;
> > - tmp6 = tmpz[2].im + tmpz[3].im;
> > - tmp3 = tmpz[0].re - tmpz[1].re;
> > - tmp8 = tmpz[2].im - tmpz[3].im;
> > - tmp4 = tmpz[0].im - tmpz[1].im;
> > - tmp7 = tmpz[2].re - tmpz[3].re;
> > + tmp1 = tmpz[0].re + (SUINT)tmpz[1].re;
> > + tmp5 = tmpz[2].re + (SUINT)tmpz[3].re;
> > + tmp2 = tmpz[0].im + (SUINT)tmpz[1].im;
> > + tmp6 = tmpz[2].im + (SUINT)tmpz[3].im;
> > + tmp3 = tmpz[0].re - (SUINT)tmpz[1].re;
> > + tmp8 = tmpz[2].im - (SUINT)tmpz[3].im;
> > + tmp4 = tmpz[0].im - (SUINT)tmpz[1].im;
> > + tmp7 = tmpz[2].re - (SUINT)tmpz[3].re;
> >
> > tmpz[0].re = tmp1 + tmp5;
> > tmpz[2].re = tmp1 - tmp5;
> > @@ -288,19 +288,19 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> > {
> > offset = ff_fft_offsets_lut[n] << 3;
> > tmpz = z + offset;
> >
> > - tmp1 = tmpz[4].re + tmpz[5].re;
> > - tmp3 = tmpz[6].re + tmpz[7].re;
> > - tmp2 = tmpz[4].im + tmpz[5].im;
> > - tmp4 = tmpz[6].im + tmpz[7].im;
> > + tmp1 = tmpz[4].re + (SUINT)tmpz[5].re;
> > + tmp3 = tmpz[6].re + (SUINT)tmpz[7].re;
> > + tmp2 = tmpz[4].im + (SUINT)tmpz[5].im;
> > + tmp4 = tmpz[6].im + (SUINT)tmpz[7].im;
> > tmp5 = tmp1 + tmp3;
> > tmp7 = tmp1 - tmp3;
> > tmp6 = tmp2 + tmp4;
> > tmp8 = tmp2 - tmp4;
> >
> > - tmp1 = tmpz[4].re - tmpz[5].re;
> > - tmp2 = tmpz[4].im - tmpz[5].im;
> > - tmp3 = tmpz[6].re - tmpz[7].re;
> > - tmp4 = tmpz[6].im - tmpz[7].im;
> > + tmp1 = tmpz[4].re - (SUINT)tmpz[5].re;
> > + tmp2 = tmpz[4].im - (SUINT)tmpz[5].im;
> > + tmp3 = tmpz[6].re - (SUINT)tmpz[7].re;
> > + tmp4 = tmpz[6].im - (SUINT)tmpz[7].im;
> >
> > tmpz[4].re = tmpz[0].re - tmp5;
> > tmpz[0].re = tmpz[0].re + tmp5;
> > @@ -311,13 +311,13 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> > {
> > tmpz[6].im = tmpz[2].im + tmp7;
> > tmpz[2].im = tmpz[2].im - tmp7;
> >
> > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp1 + tmp2);
> > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp1 + tmp2);
> > tmp5 = (int32_t)((accu + 0x40000000) >> 31);
> > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 - tmp4);
> > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 - tmp4);
> > tmp7 = (int32_t)((accu + 0x40000000) >> 31);
> > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp2 - tmp1);
> > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp2 - tmp1);
> > tmp6 = (int32_t)((accu + 0x40000000) >> 31);
> > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 + tmp4);
> > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 + tmp4);
> > tmp8 = (int32_t)((accu + 0x40000000) >> 31);
> > tmp1 = tmp5 + tmp7;
> > tmp3 = tmp5 - tmp7;
> > @@ -348,10 +348,10 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z)
> > {
> > offset = ff_fft_offsets_lut[n] << nbits;
> > tmpz = z + offset;
> >
> > - tmp5 = tmpz[ n2].re + tmpz[n34].re;
> > - tmp1 = tmpz[ n2].re - tmpz[n34].re;
> > - tmp6 = tmpz[ n2].im + tmpz[n34].im;
> > - tmp2 = tmpz[ n2].im - tmpz[n34].im;
> > + tmp5 = tmpz[ n2].re + (SUINT)tmpz[n34].re;
> > + tmp1 = tmpz[ n2].re - (SUINT)tmpz[n34].re;
> > + tmp6 = tmpz[ n2].im + (SUINT)tmpz[n34].im;
> > + tmp2 = tmpz[ n2].im - (SUINT)tmpz[n34].im;
> >
> > tmpz[ n2].re = tmpz[ 0].re - tmp5;
> > tmpz[ 0].re = tmpz[ 0].re + tmp5;
> > --
> > 2.13.0
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
>
> libavcodec/avfft.h:typedef float FFTSample;
> libavcodec/fft.h:typedef int32_t FFTSample;
> libavcodec/fft.h:typedef int16_t FFTSample;
>
> You're forcing something to be integer in a templated file, and also I'm
> unhappy with how you change typedef'd types to this SUINT thing.
> Please never change typedef'd types even if they're always integer and
> instead change the typedef to go from "typedef int <something>" to "typedef
> SUINT <something>". Things are typedeffed for a reason and clarity (e.g. to
> not mislead devs into thinking they're just integers and there's nothing
> special going on).
The changed code is under #if FFT_FIXED_32
so the changed type is always int32 and FFTSample is actually
misleading as it suggests it can be something else, especially in a
template. Not that i really have an oppinion on this, just pointing
it out.
Iam not sure i understand what exactly you suggest but
I can introduce a SUFFTSample typdef if thats preferred?
or do you prefer / meant something else ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170526/a0c1f6d5/attachment.sig>
More information about the ffmpeg-devel
mailing list