On Sat, Sep 26, 2015 at 10:32 PM, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> Hi,
>
> On Sat, Sep 26, 2015 at 7:19 PM, Ganesh Ajjanagadde <gajja...@mit.edu>
> wrote:
>
>> On Sat, Sep 26, 2015 at 7:11 PM, Michael Niedermayer <michae...@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 <gajjanaga...@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@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to