Message ID | 20171122200058.2843-2-michael@niedermayer.cc |
---|---|
State | New |
Headers | show |
2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c > index a0ef6859f1..0db932a105 100644 > --- a/libavcodec/sbrdsp_fixed.c > +++ b/libavcodec/sbrdsp_fixed.c > @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) > > round = 1U << (nz-1); > mant = (int)((accu + round) >> nz); > - mant = (mant + 0x40)>>7; > + mant = (mant + 0x40ll)>>7; LL? Carl Eugen
On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c >> index a0ef6859f1..0db932a105 100644 >> --- a/libavcodec/sbrdsp_fixed.c >> +++ b/libavcodec/sbrdsp_fixed.c >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) >> >> round = 1U << (nz-1); >> mant = (int)((accu + round) >> nz); >> - mant = (mant + 0x40)>>7; >> + mant = (mant + 0x40ll)>>7; > > LL? > More correctly, shouldnt this use one of those fancy integer constant macros, like INT64_C(0x40)? (I don't actually know if those are supposed to work with hex constants, but the fact that they exist seems to indicate that LL is not entirely portable) - Hendrik
On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote: > On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > > >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c > >> index a0ef6859f1..0db932a105 100644 > >> --- a/libavcodec/sbrdsp_fixed.c > >> +++ b/libavcodec/sbrdsp_fixed.c > >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) > >> > >> round = 1U << (nz-1); > >> mant = (int)((accu + round) >> nz); > >> - mant = (mant + 0x40)>>7; > >> + mant = (mant + 0x40ll)>>7; > > > > LL? I can change it to LL if preferred, i guess ill do that, LL seems more common in our source > > > > More correctly, shouldnt this use one of those fancy integer constant > macros, like INT64_C(0x40)? (I don't actually know if those are > supposed to work with hex constants, but the fact that they exist > seems to indicate that LL is not entirely portable) we use both LL and ll already, so there should be no issue that we arent already affected by since "forever" [...]
On 11/22/2017 7:59 PM, Hendrik Leppkes wrote: > On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: >> >>> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c >>> index a0ef6859f1..0db932a105 100644 >>> --- a/libavcodec/sbrdsp_fixed.c >>> +++ b/libavcodec/sbrdsp_fixed.c >>> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) >>> >>> round = 1U << (nz-1); >>> mant = (int)((accu + round) >> nz); >>> - mant = (mant + 0x40)>>7; >>> + mant = (mant + 0x40ll)>>7; >> >> LL? >> > > More correctly, shouldnt this use one of those fancy integer constant > macros, like INT64_C(0x40)? (I don't actually know if those are > supposed to work with hex constants, but the fact that they exist > seems to indicate that LL is not entirely portable) > > - Hendrik LL/ULL are used plenty of times in the tree, so it's portable enough. What i remember not working at least on old msvc was LLU.
On Thu, Nov 23, 2017 at 11:00:21PM +0100, Michael Niedermayer wrote: > On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote: > > On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>: > > > > > >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c > > >> index a0ef6859f1..0db932a105 100644 > > >> --- a/libavcodec/sbrdsp_fixed.c > > >> +++ b/libavcodec/sbrdsp_fixed.c > > >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) > > >> > > >> round = 1U << (nz-1); > > >> mant = (int)((accu + round) >> nz); > > >> - mant = (mant + 0x40)>>7; > > >> + mant = (mant + 0x40ll)>>7; > > > > > > LL? > > I can change it to LL if preferred, i guess ill do that, LL seems more > common in our source and applied with LL [...]
diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c index a0ef6859f1..0db932a105 100644 --- a/libavcodec/sbrdsp_fixed.c +++ b/libavcodec/sbrdsp_fixed.c @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu) round = 1U << (nz-1); mant = (int)((accu + round) >> nz); - mant = (mant + 0x40)>>7; + mant = (mant + 0x40ll)>>7; mant *= 64; expo = nz + 15; return av_int2sf(mant, 30 - expo);
Fixes: signed integer overflow: 2147483598 + 64 cannot be represented in type 'int' Fixes: 4337/clusterfuzz-testcase-minimized-6192658616680448 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/sbrdsp_fixed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)