Bug 34841 - 'make check' of libsndfile-1.0.17 fails with gcc-4.2.2 -O2 optimization, OK with -O1 one
Summary: 'make check' of libsndfile-1.0.17 fails with gcc-4.2.2 -O2 optimization, OK w...
Status: RESOLVED DUPLICATE of bug 32102
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 4.2.2
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2008-01-17 23:57 UTC by Sergei Steshenko
Modified: 2008-01-21 06:55 UTC (History)
4 users (show)

See Also:
Host: i686-linux-gnu
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-01-21 06:55:57


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Steshenko 2008-01-17 23:57:40 UTC
I've noticed the problem while building libsndfile-1.0.17 from source after   
switching from SUSE 10.2 to SUSE 10.3, and thus from gcc-4.1.2 to gcc-4.2.1.

I have built myself a number of 'gcc' versions: 3.4.6, 4.2.0, 4.2.1, 4.2.2.

With gcc-3.4.6, gcc-4.1.2 default build of libsndfile-1.0.17 (i.e. -O2
optimization) progresses just fine, 'make check' does not fail.

With all the above gcc-4.2.x default build fails during 'make check' with
these error messages:


"
Line 765: Signal is all zeros (-27260928, 0xFE600800).
make[1]: *** [wav-tests] Error 1
make[1]: Leaving directory `/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17/tests'
make: *** [check-recursive] Error 1
".

So, I believe, the bug was introduced into gcc-4.2.0 and is still there.

Setting '-O1' on 'configure' command line like this:

CFLAGS=-O1

allows 'make check' to complete normally.


One can see examples of successful and failing runs in the to be uploaded

gcc4.2.x-O2_bug.tar.gz

tarball.

When unpackaged, the tarball should produce three directories of interest:

gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17
gcc4.2.x-O2_bug/gcc-4.2.2-O1/libsndfile-1.0.17
gcc4.2.x-O2_bug/gcc-3.4.6-O2/libsndfile-1.0.17
.

In the above directories one can see 'make_check.log' files and

gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17/make_check.log

file contains the above error messages indicating the failure.

Likewise, each of the above directories contains a simple 'run.sh'
script looking like this:

"
sergei@amdam2:/mnt/sda8/sergei> cat gcc4.2.x-O2_bug/gcc-4.2.2-O1/libsndfile-1.0.17/run.sh
#!/bin/sh -v

make clean 1>make_clean.log 2>&1

./configure CC=/maxtor5/sergei/AppsFromScratchWD/install/gcc-4.2.2/bin/gcc CFLAGS=-O1 1>configure.log 2>&1 \
&& make 1>make.log 2>&1 \
&& make check 1>make_check.log 2>&1
sergei@amdam2:/mnt/sda8/sergei>
".

So, one can reproduce the bug (and the correct behavior) by changing the
path after "CC=" and running the script as "./run.sh".

Since I am not a libsndfile-1.0.17 developer, I am not familiar with inner
workings of the library, hopefully folks on libsndfile-devel@mega-nerd.com
list (unfortunately, the list does not have WEB archive) will be able to help.

People on the list confirm the problem.


As you all probably already know, 'libsndfile' "lives" here:

http://www.mega-nerd.com/libsndfile/
.
Comment 1 Sergei Steshenko 2008-01-18 00:40:54 UTC
The tarball: http://www.filelime.com/upload/files/gcc4.2.x-O2_bug.tar.gz .
Comment 2 Andrew Pinski 2008-01-18 01:18:44 UTC
Can you try -O2 -fno-strict-aliasing or -O2 -fwrapv ?  This might not be a GCC issue but the source could be dependent on undefined behavior. 
Comment 3 Sergei Steshenko 2008-01-18 01:28:58 UTC
With "-O2 -fno-strict-aliasing" the failure is still there, I'll check with
"-O2 -fwrapv" right away.
Comment 4 Sergei Steshenko 2008-01-18 01:33:11 UTC
"-O2 -fwrapv" fixes the problem.
Comment 5 Andrew Pinski 2008-01-18 01:36:20 UTC
So I think this is just a case of the source depending on signed integers overflow being defined.  Can you try to see if there are any warnings with -Wstrict-overflow ?  If so it might be best if the source gets fixed.
Comment 6 Sergei Steshenko 2008-01-18 01:43:54 UTC
I've tried CFLAGS='-O2 -fwrapv -Wstrict-overflow' and I see no warnings at all
in 'make_check.log' file - I tried "grep -i warn make_check.log".

OTOH:

"
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep -i warn make.log
sndfile.c:491: warning: the address of 'sf_error' will never be NULL
sndfile-play.c:292: warning: the address of ‘status’ will always evaluate as ‘true’
floating_point_test.c:338: warning: comparison between signed and unsigned
floating_point_test.c:388: warning: comparison between signed and unsigned
floating_point_test.c:438: warning: comparison between signed and unsigned
floating_point_test.c:488: warning: comparison between signed and unsigned
floating_point_test.c:538: warning: comparison between signed and unsigned
floating_point_test.c:588: warning: comparison between signed and unsigned
floating_point_test.c:638: warning: comparison between signed and unsigned
floating_point_test.c:688: warning: comparison between signed and unsigned
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> 
",

but these warning say nothing about overflow since it's compilation, not
execution.

Did you mean CFLAGS='-O2 -fwrapv -Wstrict-overflow' or, rather,
CFLAGS='-O2 -Wstrict-overflow' ?
Comment 7 Andrew Pinski 2008-01-18 01:46:25 UTC
(In reply to comment #6)
> Did you mean CFLAGS='-O2 -fwrapv -Wstrict-overflow' or, rather,
> CFLAGS='-O2 -Wstrict-overflow' ?

The latter, you will only get the warning if -fwrapv is off as it warns when the optimizers are optimizing based on the undefinedness.
Comment 8 Sergei Steshenko 2008-01-18 01:52:56 UTC
With CFLAGS='-O2 -Wstrict-overflow' still no warnings in 'make_check.log' and

"
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep -i warn make.log
sndfile.c:491: warning: the address of 'sf_error' will never be NULL
sndfile-play.c:292: warning: the address of ‘status’ will always evaluate as ‘true’
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17>  
".

Here is a quote from a libsndfile-devel@mega-nerd.com contributor:

"
Yep, I had the same problem when upgrading my gcc from 4.1.2 to 4.2.2.
The test fails because some of the bit shuffling tricks
that Eric uses there does not work correctly with 4.2.2.
I backported lossy_comp_test.c from pre 1.0.18 - which does pass the test with 4.2.2 -
to my 1.0.17 and the test did then run correctly again.

Unfortunately, I later found out that gcc 4.2.2 with -03 does produce wrong code
when I use stl:set in my c++ functions. So I now switched back to use 4.1.2.
Still, if you want you may try  to use the diff below that is the difference between the 
original version from 1.0.17 and the patched version that works with gcc 4.2.2.
libsndfile worked for me with gcc 4.2.2 without problems.
".
Comment 9 İsmail Dönmez 2008-01-18 02:45:51 UTC
File  lossy_comp_test.c starting line 761 :

sum_abs = abs (sum_abs + abs (abs (data [k]) - 256)) ;

if (sum_abs < 1.0)
        {       
printf ("\n\nLine %d: Signal is all zeros (%d, 0x%X).\n", __LINE__, sum_abs, sum_abs) ;
exit (1) ;
} ;

sum_abs gets a value of -27260928 . Making sum_abs unsigned fixes the problem.

Comment 10 Sergei Steshenko 2008-01-18 03:05:57 UTC
Ismail, the problem, as I see it, is not the failure itself, but rather
dependency on optimization level.

My point is that if the code is buggy WRT signedness, it should be the same
way buggy for any level of optimization.

What I and others see is that code behavior changes depending on optimization.

I myself very much dislike "comparison between signed and unsigned" warnings
and do not allow myself to produce such a code, but in this case the warnings
might be a blessing in disguise because the possibly buggy 'libsndfile'
code shows what looks to me a 'gcc' bug.
Comment 11 İsmail Dönmez 2008-01-18 03:10:03 UTC
Actually the only bug here is that -Wstrict-overflow should issue a warning for that line. 

About the dependency on optimization level, signed integer overflow is undefined in C standard so its not a good idea to depend on it. What GCC does is exploiting this fact for optimizations which is fine.

What -fwrapv does that it defines wrapping for signed integer overflow. I hope this makes the issue more clear.
Comment 12 Sergei Steshenko 2008-01-18 03:20:39 UTC
Regarding

"
About the dependency on optimization level, signed integer overflow is
undefined in C standard so its not a good idea to depend on it. What GCC does
is exploiting this fact for optimizations which is fine.
"

- I do not think this is fine. As end user I want to see my errors the same
way at any optimization level.

My expectations are that regardless of optimization level I'll get my program
functioning the same way, but with different memory consumption/speed.

I am not talking about out of bounds indices and possible reshuffling of
variables in memory, I'm talking about arithmetic/logic expressions, i.e.

printf("some_var=%g\n", some_var);

should always print the same.
Comment 13 İsmail Dönmez 2008-01-18 03:22:36 UTC
I don't think thats possible given the fact that an optimization pass modifies code to be able to well "optimize" it. Implications and merits of -fwrapv is discussed deeply before, you might want to Google for it.
Comment 14 Manuel López-Ibáñez 2008-01-18 09:47:51 UTC
(In reply to comment #12)
> 
> - I do not think this is fine. As end user I want to see my errors the same
> way at any optimization level.
> 

We strive to for this when it is feasible and reasonable, but there is a trade-off between functionality and compilation speed. Detecting some errors requires analyzing the code and performing transformations. This is precisely what optimisations do. The higher the optimisation level, the deeper the analysis and the larger the transformations. Doing the maximum analysis and transformations at -O0 and throwing away the results would be pointless. Would you like if -O0 were to take more time than -O3? Most users won't.

> My expectations are that regardless of optimization level I'll get my program
> functioning the same way, but with different memory consumption/speed.

If you believe this, you are going to be deeply disappointed by reality. Apart from the unavoidable fact that bugs in the code (like in this case), race conditions and hardware problems may be more evident with one optimisation or another, there are issues with floating point precision (PR323) and others.

> I am not talking about out of bounds indices and possible reshuffling of
> variables in memory, I'm talking about arithmetic/logic expressions, i.e.
> 
> printf("some_var=%g\n", some_var);
> 
> should always print the same.
> 

That can't be the case always. Arithmetic/logic expressions have precision errors that are different depending on the order of operations. And altering the order of operations is precisely what you are asking when you enable optimizations. Anyway, this is not the place for such theoretical discussion

If "gcc -O2 -Wstrict-overflow=5" does not give a warning, we may have a bug (or it may be a case too difficult to detect). Could you try that, please?
Comment 15 Sergei Steshenko 2008-01-18 14:08:53 UTC
With CFLAGS='-O2 -Wstrict-overflow=5' still there is no warnings in
'make_check.log':

"

sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep -i warn make.log
sndfile.c:491: warning: the address of 'sf_error' will never be NULL
sndfile-play.c:292: warning: the address of &#8216;status&#8217; will always evaluate as &#8216;true&#8217;
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep -i warn make_check.log
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep -P '\-Wstrict-overflow=5' make.log | wc -l
341
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17>             
",

so I think you are right there is a bug in gcc.

And there is apparently another bug.

If I do _not_ have "-Wstrict-overflow", I _do_ have these warnings during
compilation:

"
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep "comparison between signed and unsigned" ../../gcc-4.2.2-O1/libsndfile-1.0.17/make.log
floating_point_test.c:338: warning: comparison between signed and unsigned
floating_point_test.c:388: warning: comparison between signed and unsigned
floating_point_test.c:438: warning: comparison between signed and unsigned
floating_point_test.c:488: warning: comparison between signed and unsigned
floating_point_test.c:538: warning: comparison between signed and unsigned
floating_point_test.c:588: warning: comparison between signed and unsigned
floating_point_test.c:638: warning: comparison between signed and unsigned
floating_point_test.c:688: warning: comparison between signed and unsigned
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17>
".

If I have "-Wstrict-overflow", I do _not_ have the above warnings:

"
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep "comparison between signed and unsigned" make.log
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> 
".

I believe I should have the warnings in both cases.

Folks, libsndfile is easy to compile - it has (kind of) no external 
dependencies, i.e. it depends only on basic libraries like libm, glibc, etc.,
so you can easily conduct such experiments yourselves - the the needed "beef",
including libsdnfile sources, is in the uploaded file.
Comment 16 Sergei Steshenko 2008-01-18 14:19:08 UTC
A general though regarding optimization - do _not_ optimize code producing
warnings, and notify end user, so there will be much more incentive to write
clean code. 
Comment 17 Manuel López-Ibáñez 2008-01-18 14:49:09 UTC
(In reply to comment #15)
> With CFLAGS='-O2 -Wstrict-overflow=5' still there is no warnings in
> 'make_check.log':
> 
> If I do _not_ have "-Wstrict-overflow", I _do_ have these warnings during
> compilation:

Any of those looks like a bug to me. At a minimum, one warning flag that doesn't warn should not affect another that does.

> Folks, libsndfile is easy to compile - it has (kind of) no external 
> dependencies, i.e. it depends only on basic libraries like libm, glibc, etc.,
> so you can easily conduct such experiments yourselves - the the needed "beef",
> including libsdnfile sources, is in the uploaded file.

I personally hack gcc in my precious free time. The larger the file that triggers the bug, the harder and tedious it becomes to simply understand what is going on, let alone having a chance to fix it.
Comment 18 Manuel López-Ibáñez 2008-01-18 18:47:12 UTC
(In reply to comment #15)
> With CFLAGS='-O2 -Wstrict-overflow=5' still there is no warnings in

BTW, is your makefile adding -Wstrict-overflow after or before -Wall -Wextra?
Comment 19 Sergei Steshenko 2008-01-18 23:33:01 UTC
Regarding "BTW, is your makefile adding -Wstrict-overflow after or before -Wall -Wextra?".

Here is how the first action line in 'make.log' looks:

"
    23  if /bin/sh ../../libtool --tag=CC --mode=compile /maxtor5/sergei/AppsFromScratchWD/install/gcc-4.2.2/bin/gcc -DHAVE_CONFIG_H -I. -I. -I../../src     -O2 -Wstrict-overflow=5 -std=gnu99 -W -Wall -Wdeclaration-after-statement -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes -Waggregate-return -Wcast-align -Wcast-qual -Wnested-externs -Wshadow -Wbad-function-cast -Wwrite-strings  -pipe  -MT add.lo -MD -MP -MF ".deps/add.Tpo" -c -o add.lo add.c; \
    24          then mv -f ".deps/add.Tpo" ".deps/add.Plo"; else rm -f ".deps/add.Tpo"; exit 1; fi
".

My understanding is that whatever is given to 'configure' as CFLAGS is inserted
before the flags 'configure' and friends put.
Comment 20 Manuel López-Ibáñez 2008-01-19 00:22:14 UTC
There is a bug (PR32102) where -Wall after -Wstrict-overflow resets the latter to its default value. I think this is why you didn't get the warning. Removing -Wall or moving -Wstrict-overflow=5 after it should generate the warning.

Now, why the warnings from -Wsign-comparison appear and disappear? That I don't know but I don't have more free time to investigate this. I hope someone can look into it or, at least, find a reduced testcase.
Comment 21 Sergei Steshenko 2008-01-20 22:30:47 UTC
Now that the flags are in this order: -Wall -Wstrict-overflow=5 :

a) the warnings during compilation:

"
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17> grep warn make.log
lpc.c:220: warning: assuming signed overflow does not occur when simplifying conditional to constant
g72x.c:249: warning: assuming signed overflow does not occur when simplifying conditional to constant
sndfile.c:491: warning: the address of 'sf_error' will never be NULL
aiff.c:557: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C1 +- C2
sd2.c:540: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C1 +- C2
sd2.c:558: warning: assuming signed overflow does not occur when changing X +- C1 cmp C2 to X cmp C1 +- C2
sndfile-play.c:292: warning: the address of &#8216;status&#8217; will always evaluate as &#8216;true&#8217;
lossy_comp_test.c:1342: warning: assuming signed overflow does not occur when simplifying abs (X) to X or -X
lossy_comp_test.c:1527: warning: assuming signed overflow does not occur when simplifying abs (X) to X or -X
lossy_comp_test.c:761: warning: assuming signed overflow does not occur when simplifying abs (X) to X or -X
lossy_comp_test.c:573: warning: assuming signed overflow does not occur when simplifying abs (X) to X or -X
peak_chunk_test.c:127: warning: assuming signed overflow does not occur when simplifying division
peak_chunk_test.c:194: warning: assuming signed overflow does not occur when simplifying division
fix_this.c:155: warning: assuming signed overflow does not occur when simplifying abs (X) to X or -X
sergei@amdam2:/mnt/sda8/sergei/gcc4.2.x-O2_bug/gcc-4.2.2-O2/libsndfile-1.0.17>
";

b) 'make check' still fails with the same

"
Line 765: Signal is all zeros (-27260928, 0xFE600800).
"

message.

Comment 22 Manuel López-Ibáñez 2008-01-21 01:10:32 UTC
"make check" failing is expected since there is undefined behaviour in the program and we warn about it with -Wstrict-overflow=5 (I guess that we warn with lower values as well, probably simply with -Wstrict-overflow). So this is a duplicate of PR 32102.

About the signed/unsigned warnings, it seems a minor issue. But if you still want us to investigate it, please open a new PR (this one got a bit messy). If you include just the relevant floating_point_test.c file and the headers to make it compile (or better, use -save-temps and attach floating_point_test.i), then we can easily reproduce the bug at will and investigate it. Thanks.

*** This bug has been marked as a duplicate of 32102 ***
Comment 23 Sergei Steshenko 2008-01-21 05:07:13 UTC
It's too bad the bug is closed just as a duplicate of another bug.

The main points of this bug are:

1) the code triggering the bug uses undefined in "C" standards language
features - behavior in case of integer overflow + signed <-> unsigned
comparison;

2) "C" standard is not the only standard having some cases undefined - whenever
compiler developers face an undefined case they should somehow define the case
themselves and publish their definition/decision;

3) it's very desirable for the compiler developers to be consistent, i.e.
whenever the undefined by standard case is encountered, the compiler behaves
the same way, i.e. produces code implementing the same algorithm;

4) I do see consistency in gcc-3.4.6, gcc-4.1.2 - regardless of -O1 vs -O2
generated by gcc code functionally behaves the same way;

5) gcc-4.2.x shows lack of consistency - -O1 code behaves differently compared
to -O2 one. It's a pity that newer gcc versions are less consistent than the 
old ones. As I suggested earlier, the cleanest solution would be _not_ to
perform optimization on unclean code, thus ensuring consistency.
Comment 24 Manuel López-Ibáñez 2008-01-21 06:55:57 UTC
(In reply to comment #23)
> It's too bad the bug is closed just as a duplicate of another bug.

I am sorry that you feel disappointed. I believed that the rationale behind closing this was fairly clear. I tried to answer your concerns without getting into a debate.

Point 1), 2) and 3) are about the meaning of undefined behaviour and this have been lengthy discussed in the mailing list. 

Probably the latest discussion here:
http://gcc.gnu.org/ml/gcc/2007-01/msg00881.html

More to be found in the archives:
http://gcc.gnu.org/ml/gcc/

About points 4) and 5), gcc-3.4.6 and gcc-4.1.2 are only consistent in the examples you have tried but, as discussed in comment 14, optimization changes code and different code has different behaviour. We strive to give consistent results for defined behavior. And we strive to give the best optimized code without breaking the standards. Again, this has been discussed thoroughly in the mailing list in the context of signed overflow and also strict aliasing (Wstrict-aliasing).

It is unlikely that whatever point you made hasn't been covered in a discussion already, this is why gcc developers generally avoid any discussion about undefined behaviour. This is nothing new.