[FFmpeg-devel] [PATCH] [libavutil] Add saturated add/sub operations for int64_t.

[Dale Curtis]

Bump. I have 5 integer overflow fuzzing issues awaiting our resolution of
this discussion. Thanks.

- dale

On Fri, May 1, 2020 at 1:13 PM Dale Curtis <dalecurtis at chromium.org> wrote:

> On Fri, May 1, 2020 at 12:53 PM Michael Niedermayer <michael at niedermayer.cc>
> wrote:
>
>> On Thu, Apr 30, 2020 at 05:39:43PM -0700, Dale Curtis wrote:
>> > On Thu, Apr 30, 2020 at 5:21 PM James Almer <jamrial at gmail.com> wrote:
>> >
>> > > On 4/30/2020 7:19 PM, Dale Curtis wrote:
>> > > > Many places are using their own custom code for handling overflow
>> > > > around timestamps or other int64_t values. There are enough of these
>> > > > now that having some common saturated math functions seems sound.
>> > > >
>> > > > This adds implementations that just use the builtin functions for
>> > > > recent gcc, clang when available or implements its own version for
>> > > > older compilers.
>> > >
>> > > These look like 64 bit versions of av_sat_add32 and av_sat_sub32, from
>> > > common.h, so you should probably add them there and rename them
>> > > accordingly.
>> > >
>> > >
>> > Ah! I was looking for those, but missed them. Thanks. Done.
>>
>> one disadvantage the av_sat* functions have is the lack of inexact
>> detection
>>
>> In addition to av_sat*
>> In situations where its better to fail than to clip, something that
>> emulates what (+-Inf/)NaN is for float may make sense.
>> That would allow to simply check after a computation if any inexactness
>> occured
>>
>> Such a thing could be usefull in situations where a exact value or an
>> error is wanted.
>>
>>
> The __builtin functions provide exactly this API, we're just hiding it. I
> could add something like:
> int did_overflow = av_checked_sat_(add|sub)64(int64_t a, int64_t b,
> int64_t* result)
>
> |result| would still satuate and thus av_sat_(add|sub)64 could use it
> without checking the return value, but those which want to check and abort
> could do so. This is similar to the API shape we expose in Chromium modulo
> the fact we enforce an assert.
>
> - dale
>
>