On Sun, Sep 27, 2015 at 12:58 PM, Michael Niedermayer <[email protected]> wrote: > On Sat, Sep 26, 2015 at 10:55:26PM -0400, Ganesh Ajjanagadde wrote: >> On Sat, Sep 26, 2015 at 10:32 PM, Ronald S. Bultje <[email protected]> >> wrote: >> > Hi, >> > >> > On Sat, Sep 26, 2015 at 7:19 PM, Ganesh Ajjanagadde <[email protected]> >> > wrote: >> > >> >> On Sat, Sep 26, 2015 at 7:11 PM, Michael Niedermayer <[email protected]> >> >> 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 <[email protected]> >> >> >> --- >> >> >> 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). > > iam not sure i understand correctly but > if the compiler changes the code and then warns that what it just > did might be unsafe then the compiler is broken
https://stackoverflow.com/questions/12984861/dont-understand-assuming-signed-overflow-warning - gives a detailed explanation. Some more info: this is triggered only when -finline-functions is enabled (done by default on -O3, not enabled by default on -O2). -finline-functions tries to inline stuff even when "inline" keyword is absent (like in this case). As for the warning, http://linux.die.net/man/1/gcc - search for -Wstrict-overflow. It is enabled due to -Wall, and as the man page suggests, it depends on optimization level as we can see in this example. I do consider the compiler broken in this case, but then again compilers are broken in so many different ways it is not even funny: see e.g -Warray-bounds, can't use the ISO C correct { 0 } initializer for compound data types, etc. If you don't like this, we should add a -Wnostrict-overflow either to configure, or a local enable/disable via pragmas/macros. I don't like either of these as compared to this simple workaround: 1. -Wnostrict-overflow: FFmpeg with the amount of integer arithmetic being done should benefit from this warning in general, so disabling it globally may be bad. 2. local enable/disable: too ugly and overkill when we need only 2 lines to be changed in trivial ways throughout the codebase: movenc and xface. IMO, we should consider this only if we run into more false positives. > > [...] > -- > 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 > > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
