[FFmpeg-devel] [RFC] av_rescale() coverity

[Michael Niedermayer]

On Tue, Jul 02, 2024 at 06:51:16AM +0200, Vittorio Giovara wrote:
> On Mon, Jul 1, 2024 at 11:00 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Mon, Jul 01, 2024 at 10:19:31PM +0200, Michael Niedermayer wrote:
> > > On Mon, Jul 01, 2024 at 08:50:24PM +0200, Timo Rothenpieler wrote:
> > > > On 01.07.2024 15:39, Michael Niedermayer wrote:
> > > > > Hi all
> > > > >
> > > > > coverity seems to have started to do a new thing. Namely if theres a
> > > > > return statement it assumes it can independant of everything occurr
> > > > >
> > > > > an example would be av_rescale() which on overflow returns INT64_MIN
> > > > >
> > > > > also with the right flags av_rescale() will pass INT64_MIN and
> > INT64_MAX through
> > > > > from the input
> > > > >
> > > > > So coverity since a few days seems to treat every av_rescale() call
> > as if it returns
> > > > > INT64_MIN and INT64_MAX. coverity doesnt care if that return
> > statement is reachable or
> > > > > if the flags even include the execution path.
> > > > >
> > > > > An example is this:
> > > > >              AVRational time_base_q = AV_TIME_BASE_Q;
> > > > >              int64_t next_dts = av_rescale_q(ds->next_dts,
> > time_base_q, av_inv_q(ist->framerate));
> > > > >              ds->next_dts = av_rescale_q(next_dts + 1,
> > av_inv_q(ist->framerate), time_base_q);
> > > > >
> > > > > Here coverity as a initial statement claims next_dts is INT64_MAX
> > > > > and next_dts + 1 would overflow
> > > > >
> > > > >
> > > > >      8. function_return: Function av_rescale_q(ds->next_dts,
> > time_base_q, av_inv_q(ist->framerate)) returns 9223372036854775807.
> > > > >              9. known_value_assign: next_dts =
> > av_rescale_q(ds->next_dts, time_base_q, av_inv_q(ist->framerate)), its
> > value is now 9223372036854775807.
> > > > >      331            int64_t next_dts = av_rescale_q(ds->next_dts,
> > time_base_q, av_inv_q(ist->framerate));
> > > > >
> > > > >      CID 1604545: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
> > > > >      10. overflow_const: Expression next_dts + 1LL, which is equal
> > to -9223372036854775808, where next_dts is known to be equal to
> > 9223372036854775807, overflows the type that receives it, a signed integer
> > 64 bits wide.
> > > > >
> > > > >
> > > > > another example is this:
> > > > >
> > > > >      #define AV_TIME_BASE            1000000
> > > > >      pts = av_rescale(ds->dts, 1000000, AV_TIME_BASE);
> > > > >
> > > > > coverity hallucinates pts as a tainted negative number here nothing
> > says anything about
> > > > > the input ds->dts (and thats what would matter)
> > > > >
> > > > > In the past coverity provided a detailed list of steps on how a
> > > > > case is reached. One could then check these assumtions and mark
> > things
> > > > > as false positive when one assumtion is wrong. (coverity was most of
> > the time
> > > > > wrong)
> > > > >
> > > > > Now coverity just hallucinates claims out of the blue without any
> > > > > explanation how that can happen.
> > > > >
> > > > > Iam a bit at a loss how to deal with this and also why exactly this
> > > > > new behavior appeared.
> > > > >
> > > > > Has anyone changed any setting or anything in coverity ?
> > > > >
> > > > > The number of issues shot up to over 400 on the 22th june
> > > > > "194 new defect(s) introduced to FFmpeg/FFmpeg found with Coverity
> > Scan."
> > > >
> > > > Do you mean May?
> > > > Cause that's when I enabled also giving a Windows-Build to Coverity:
> > > >
> > https://github.com/FFmpeg/FFmpeg-Coverity/commit/3116e6960406f01f96d934516216bb3b402122fc
> > > >
> > > > Before that, only Linux was analyzed.
> > >
> > > no the 194 appeared in june
> > >
> > > I did saw some other spike of issues appear month? earlier or so but
> > these seemed
> > > mostly old issues that where detected prior already.
> > > and i dont see it in teh numbers coverity mails me
> > >
> >
> > > Only other spike i can find in the numbers was 11 feb 2024
> > > 103 new defect(s) introduced to FFmpeg/FFmpeg found with Coverity Scan.
> >
> > The mail for the windows spike went to my old email address from gmx, was
> > misidentified as spam and deleted by gmx. gmx "recently" forced their
> > broken
> > spam detection to be enabled even when explicitly disabled by the customer.
> > One has to download the mails from a specific folder on their IMAP server
> > within a month it seems. Which i didnt because i had their whole broken
> > spam detection disabled
> >
> > Its not imprtant but if someone has all the coverity mails, a list of
> > new and fixed bugs on each run would be interresting
> >
> > thx
> 
> 
> Have you tried getting in touch with coverity support about this new
> behavior?

i will if adjusting our modelling file doesnt fix it. It seems 56 issues
disappeared on the last run and 3 new av_rescale() issues appeared that look
more sane, but i did not yet had the time to really investigate how things
look now

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240702/b2b63dc7/attachment.sig>