[FFmpeg-devel] [PATCH] avformat/movenc: suppress -Wstrict-overflow warnings

[Ganesh Ajjanagadde]

On Sat, Sep 26, 2015 at 10:32 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Sat, Sep 26, 2015 at 7:19 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
> wrote:
>
>> On Sat, Sep 26, 2015 at 7:11 PM, Michael Niedermayer <michaelni at gmx.at>
>> wrote:
>> > On Fri, Sep 18, 2015 at 05:15:50PM -0400, Ganesh Ajjanagadde wrote:
>> >> This patch results in identical behavior of movenc, and suppresses
>> -Wstrict-overflow
>> >> warnings observed in GCC 5.2.
>> >> I have manually checked that all usages are safe, and overflow
>> possibility does
>> >> not exist with this expression rewrite.
>> >>
>> >> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> >> ---
>> >>  libavformat/movenc.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> >> index af03d1e..6e4a1a6 100644
>> >> --- a/libavformat/movenc.c
>> >> +++ b/libavformat/movenc.c
>> >> @@ -854,7 +854,7 @@ static int get_cluster_duration(MOVTrack *track,
>> int cluster_idx)
>> >>  {
>> >>      int64_t next_dts;
>> >>
>> >> -    if (cluster_idx >= track->entry)
>> >> +    if (cluster_idx - track->entry >= 0)
>> >
>> > i do not understand what this fixes or why
>> > also plese quote the actual warnings which are fixed in the commit
>> > message
>>
>> I have posted v2 with a more detailed commit message. It should be
>> self explanatory.
>
>
> Even with the new message, it's still not clear to me what's being fixed.
> What does the warning check for? What is the problem in the initial
> expression?

Compilers make transformations on the statements in order to possibly
get better performance when compiled with optimizations. However, some
of these optimizations require assumptions in the code. In particular,
the compiler is internally rewriting cluster_idx >= track->entry to
cluster_idx - track->entry >= 0 internally for some reason (I am not
an asm/instruction set guy, so I can't comment why it likes this).
However, such a transformation is NOT always safe as integer
arithmetic can overflow (try e.g extreme values close to INT_MIN,
INT_MAX). The warning is spit out since the compiler can't be sure
that this is safe, but it still wants to do it (I suspect only the
-O3/-O2 level that try this, can check if you want).

What I have done is manually audit this code to ensure this
transformation is safe, and rewritten the expression to match what the
compiler anyway transforms it into. This thereby suppresses the
warning.

>
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel