Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [RFC] av_rescale() coverity
@ 2024-07-01 13:39 Michael Niedermayer
  2024-07-01 18:07 ` Michael Niedermayer
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-01 13:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 3274 bytes --]

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

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 13:39 [FFmpeg-devel] [RFC] av_rescale() coverity Michael Niedermayer
@ 2024-07-01 18:07 ` Michael Niedermayer
  2024-07-01 18:50 ` Timo Rothenpieler
  2024-07-02 22:27 ` Michael Niedermayer
  2 siblings, 0 replies; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-01 18:07 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 3648 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 13:39 [FFmpeg-devel] [RFC] av_rescale() coverity Michael Niedermayer
  2024-07-01 18:07 ` Michael Niedermayer
@ 2024-07-01 18:50 ` Timo Rothenpieler
  2024-07-01 20:19   ` Michael Niedermayer
  2024-07-02 22:27 ` Michael Niedermayer
  2 siblings, 1 reply; 9+ messages in thread
From: Timo Rothenpieler @ 2024-07-01 18:50 UTC (permalink / raw)
  To: ffmpeg-devel

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.

> 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
> 
> thx
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 18:50 ` Timo Rothenpieler
@ 2024-07-01 20:19   ` Michael Niedermayer
  2024-07-01 21:00     ` Michael Niedermayer
  2024-07-02 12:36     ` Timo Rothenpieler
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-01 20:19 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 3641 bytes --]

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.

thx

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 20:19   ` Michael Niedermayer
@ 2024-07-01 21:00     ` Michael Niedermayer
  2024-07-02  4:51       ` Vittorio Giovara
  2024-07-02 12:36     ` Timo Rothenpieler
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-01 21:00 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 4384 bytes --]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 21:00     ` Michael Niedermayer
@ 2024-07-02  4:51       ` Vittorio Giovara
  2024-07-02 18:02         ` Michael Niedermayer
  0 siblings, 1 reply; 9+ messages in thread
From: Vittorio Giovara @ 2024-07-02  4:51 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Mon, Jul 1, 2024 at 11:00 PM Michael Niedermayer <michael@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?
-- 
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 20:19   ` Michael Niedermayer
  2024-07-01 21:00     ` Michael Niedermayer
@ 2024-07-02 12:36     ` Timo Rothenpieler
  1 sibling, 0 replies; 9+ messages in thread
From: Timo Rothenpieler @ 2024-07-02 12:36 UTC (permalink / raw)
  To: ffmpeg-devel

On 01/07/2024 22:19, 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.
> 
> thx
> 
> [...]

I do wonder if sending them two builds at once like this is not supported?
I found examples of doing it like this though, they even document how to 
combine report generated on separate hosts. So it really should be possible.
Cause I think the huge jumps up and down in detection started only after 
adding the mingw builds.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-02  4:51       ` Vittorio Giovara
@ 2024-07-02 18:02         ` Michael Niedermayer
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-02 18:02 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 5254 bytes --]

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@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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [RFC] av_rescale() coverity
  2024-07-01 13:39 [FFmpeg-devel] [RFC] av_rescale() coverity Michael Niedermayer
  2024-07-01 18:07 ` Michael Niedermayer
  2024-07-01 18:50 ` Timo Rothenpieler
@ 2024-07-02 22:27 ` Michael Niedermayer
  2 siblings, 0 replies; 9+ messages in thread
From: Michael Niedermayer @ 2024-07-02 22:27 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2809 bytes --]


On Mon, Jul 01, 2024 at 03:39:23PM +0200, Michael Niedermayer wrote:

latest coverity fun:

CID 1604534: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression gain, which is equal to 4294967295, where get_bits1(gb) ? get_bits(gb, 4) - 7U : 4294967295U is known to be equal to 4294967295, overflows the type that receives it, a signed integer 32 bits wide.
371        int gain = get_bits1(gb) ? get_bits(gb, 4) - 7 : -1;

In case you dont see it:
storing -1 in an int is a overflow

(yes i see its unsigned intermediate but that doesnt matter, thats normal code
 and perfectly well defined)


 and another one:

CID 1604357: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression state << 8, which is equal to 17179869184, where state is known to be equal to 72057594105036800, overflows the type that receives it, an unsigned integer 64 bits wide.
 61        state = (state << 8) | buf[i];

Just to clarify this, there is NOTHING else here, nothing explains why coverity
"thinks" this has value 72057594105036800 before
of course either way this doesnt matter and is perfectly fine code


More fun:
     	5. known_value_assign: chunk_type = bytestream2_get_le32(&gb), its value is now 0.
361        chunk_type = bytestream2_get_le32(&gb);
     	6. Condition !chunk_type, taking false branch.
362        if (!chunk_type)
363            break;

do you spot the brilliant logic ?


6. known_value_assign: segments = segments, its value is now 4294967295.
CID 1604539: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
7. overflow_const: Expression segments--, which is equal to 4294967295, where segments is known to be equal to 0, underflows the type that receives it, an unsigned integer 32 bits wide.
 82    while (segments--) {

my god the loop reached 0, can you imagine

coverity reallly is cracking down on unsigned
heres another:

4. function_return: Function bytestream2_get_byte(gbc) returns 0.
CID 1604484: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
5. overflow_const: Expression version_major, which is equal to 4294967295, where bytestream2_get_byte(gbc) - 1U is known to be equal to 4294967295, overflows the type that receives it, a signed integer 32 bits wide.
version_major = bytestream2_get_byte(gbc) - 1;

ohh my god storing -1 in a signed integer


these new issues are almost hillarious and entertaining, if they wouldnt
cost time to investigate each and close


thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-07-02 22:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-01 13:39 [FFmpeg-devel] [RFC] av_rescale() coverity Michael Niedermayer
2024-07-01 18:07 ` Michael Niedermayer
2024-07-01 18:50 ` Timo Rothenpieler
2024-07-01 20:19   ` Michael Niedermayer
2024-07-01 21:00     ` Michael Niedermayer
2024-07-02  4:51       ` Vittorio Giovara
2024-07-02 18:02         ` Michael Niedermayer
2024-07-02 12:36     ` Timo Rothenpieler
2024-07-02 22:27 ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git