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

Michael Niedermayer michael at niedermayer.cc
Mon Jul 1 21:07:23 EEST 2024


On Mon, Jul 01, 2024 at 03:39:23PM +0200, 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."
> 
> before this i thought iam mostly done with my coverity work.
> now truth is, the STF text speaks about 673 issues at the time and not
> what appears after the work started, but it makes me a bit sad if i categorize
> ~700+ issues and then fix the ones that are bugs just to find coverity
> hallucinate 200 new issues a month that ill have to leave open for future
> efforts.
> 
> I did not expect that years of ignoring coverity accumulate 673 issues and
> then suddenly the rate of new issues to shoot up like this. I kind of expected
> that i can fix all new issues appearing during the work with insignificant extra effort

I will try to adjust the modelling file we use and see if that reduces the
av_rescale() hallucinations

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
-------------- 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/20240701/21c1bf63/attachment.sig>


More information about the ffmpeg-devel mailing list