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

[Ganesh Ajjanagadde]

On Sat, Sep 26, 2015 at 10:55 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> 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.

Just checked: it is something triggered at -O3 but not at -O2.
However, this does not change the key points made above.

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