Strange behaviour of GCC 4.5.2

[mghis]

Hello all.

While I was writing some code for the keyboard interrupt handler, I noticed a strange fact that I cannot explain: whether I inserted a 'if' block or not, the assembly file generated by the compiler were exactly the same. I have cut the code so that it shortly explain the misbehaviour. Here it is.

Code: Select all

     1
     2
     3  char upcase;
     4
     5
     6  int kbd_h(char c)
     7  {
     8
     9      if (c == 170 || c == 182) {
    10          upcase = 0;
    11          return 0;
    12      }
    13
    14      if (c == 42 || c == 54) {
    15          upcase = 1;
    16          return 0;
    17      }
    18
    19
    20      return 0;
    21  }

If I compile this file (to generate only assembly, with -S flag) and another identical file whose lines 8 to 13 are commented out, I get two identical assembly outputs, even if in the second file the 'if' statement and the subsequent block are comments.

I use gcc version 4.5.2, the default from my Slackware Linux installation. It's not the latest release, but it's quite recent. Other compilers, (like pcc), output different code.

Could anyone reproduce this misbehaviour? Is there something I did wrong?

Thank you for any advice.

P.S.: Please forgive me for my bad English. It isn't my mother tongue.

[Antti]

signed char cannot be 170 or 182.

[Kazinsal]

char defaults to signed, and thus has a data width of -128 through 127 (at least, it does on most sane x86 platforms). You want it to be unsigned.

EDIT: Dang, Antti beat me to it!

[mghis]

Whoops! Right! This is why it just ignored that block.

Thanks a lot!

[sortie]

On your platform a char is a signed 8-bit value from -128 to 127. When the compiler seems the comparison of a char with a value that cannot possibly be represented as a char, it deduces that the case can never happen and is optimized out. You should be using unsigned char instead -- or better yet, uint8_t from <stdint.h> since the keyboard controller sends you bytes, not elements of whatever size a char is.

Note that in this case you write code in C, but the output "doesn't do" what your C code "does". This is usually triggered by two situations:

1) You don't understand what your code really does.
2) Your code invokes undefined behavior (this defined by the standard to make no sense, such as reading from NULL) and the compiler is free to do *anything*, such as optimizing it all away.

In this case, we are in situation 2. The behavior of your code is well-defined, however the condition of the if statement can never be true, and hence the compiler removes it as well. This was caused by you misunderstanding what your code does. You should look more carefully into the C language and how it works.

Note that you should pick better names for your forum topics. There is nothing to suggest that this problem is tied to your particular gcc version, have you tried with another version of gcc? Or perhaps with optimizations turned off? When these things go wrong, don't jump to the conclusion that something is wrong with the compiler, but rather that your code doesn't do what you think it does.

[bluemoon]

As a side note, you may enable all sort of warnings and prevent this kind of issue in the future.

[dozniak]

Hello all.

-Wall -Wextra -Werror is the minimal set of gcc warning options you might want to set.

[Owen]

People who set -Werror in their makefiles deserve to have every package they download have -Werror set when their compiler introduces a new warning.

Just be vigilant about removing warnings; if you're smart, you'll remove warnings as you introduce them. If you're not... that's your own problem.

[dozniak]

People who set -Werror in their makefiles deserve to have every package they download have -Werror set when their compiler introduces a new warning.

This is very sane behavior. Report upstream immediately if that breaks. Except a few vanishingly rare cases where compiler developers were wrong in introducing certain warnings, good compilers usually catch more and more things you definitely do not want in your or somebody else's code.

In the end it leads to more robust software.

[bluemoon]

I agree with Owen.
I enabled about 30 warning flags but not -Werror. While the philosophy is to eliminate all warnings, occasionally I may get unused parameters in test code and I retain my freedom to proceed for a test image.

However, this is only practical if the project has zero or very few warnings, otherwise you may not aware of any "new warnings".

[dozniak]

While the philosophy is to eliminate all warnings, occasionally I may get unused parameters in test code and I retain my freedom to proceed for a test image.

"-Wno-unused-parameter -Wno-unused-private-field"

I'm reluctant to add -Wno-unused-variable, because it can actually hide some silly errors. It's easy to just comment out that assignment.

-Wno-unused-private-field actually comes from Qt, who were silly enough to leave an unused field in their shared pointer implementation's public interface.

[sortie]

Sorry to go a bit off-topic, but to give my standard -Werror recommendations:

-Werror is a mistake. No, really. Warnings are not errors. Right now, you may consider all warnings that happen an error, but there are likely warnings you don't know exists (and wouldn't consider errors), warnings that will be added by the compiler in another compiler release, warnings that happen on other compilers, warnings that are generated using #warning, and so on. Rather than relentlessly say "all warnings are errors", you should handle this on a per-case basis. Perhaps you consider the gcc warning -Wself-init an error in addition to what gcc normally considers an error. It that case, you should add -Werror=self-init to the compile options. Do this for every warning that you encounter that you consider an error. However, do not pass -Werror in released source code, because it will only cause needless trouble for other people. Remember how warnings aren't errors, because they represent situations that the language standards do allow but that some people may find questionable. The compiler cannot always determine this reliably (see undecidable problems).

The option does have value when you want to rid your code base from warnings from a particular compiler and for development usage, but that is personal use and you accept the responsibility of fixing the code when the compiler is needlessly strict. Don't force other people to "fix your code" because their compiler is better at giving suggestive warnings.

[Edited by sortie: Delete quote, use more polite language and expand/reword.]

[dozniak]

-Werror is a mistake. No, really. Warnings are not errors. Proof: #warning. It is damn annoying to port software that has -Werror set becaues warnings do occur when porting software to platforms they have never occured on, or when a new compiler release comes out. However, developers of packages may well wish to use -Werror when developing, but it should never be set by default in released source code.

Yes, boss.

[Mikemk]

-Werror is a mistake. No, really. Warnings are not errors. Proof: #warning. It is damn annoying to port software that has -Werror set becaues warnings do occur when porting software to platforms they have never occured on, or when a new compiler release comes out. However, developers of packages may well wish to use -Werror when developing, but it should never be set by default in released source code.

I think the OP is still developing his code. Therego, your rant is unnecessary.

[Combuster]

Yet, if everyone used -Werror, the compiler devs wouldn't be free to add new warnings... no? Therefore, some people deserve to have more epic code and survive the -Werror onslaught compared to people not being able to fix simple crashes.

Code like $(DEITY). A -Werror $(DEITY). I dare you.