ifsmirnov's blog

Yet another weird g++ warning
By ifsmirnov, history, 8 years ago, In English

Take a look at this snippet.


#include <bits/stdc++.h>
using namespace std;

void f(int x, int y) {
    if (x > y) swap(x, y);
    printf("%d %d\n", x, y);
}

void caller1(int i) {
    f(i-1, i);
}

void caller2(int i) {
    f(i+1, i);
}

int main() {
    caller1(1);
    caller2(1);
}

Have you noticed something strange? Or maybe redundant? Neither did I. But g++ did:

$ g++-4.8 -Wall -O2 a.cpp 
a.cpp: In function ‘void caller1(int)’:
a.cpp:5:5: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false [-Wstrict-overflow]
     if (x > y) swap(x, y);
     ^

Wait, what the hell? This if is used not only in caller1 but also in caller2, and in both cases the flow continues to the different branch. It seems that the optimizer examines only caller1 and doesn't even think that there could be some other users of that "redundant" line of code.

What is more strange, this error does not reproduce if only caller2 is present (in that case if condition always evaluates to true).

Hopefully, optimizer doesn't completely cut out the body of the conditional and the code works as expected albeit the warning is shown.

The bug reproduces with all g++ versions I have up to 5.0 and doesn't reproduce with clang of any version.

Tags bug, g++
  • Vote: I like it
  • +58
  • Vote: I do not like it

»
8 years ago, # |
← Rev. 2   Vote: I like it +59 Vote: I do not like it

It is not a bug.

g++ just warns you that it expects that i - 1 > i is false for all i.

This is indeed false for all cases, expect overflows. And signed overflow is undefined behaviour in c++.

However it makes interesting why g++ not warns for i + 1 > i — the situtation is pretty much the same.

→ Reply
  • »
    »
    8 years ago, # ^ |
      Vote: I like it +36 Vote: I do not like it

    And if you look at the generated assembly for caller2, you can see that the comparison gets optimized out:

     15:A.cpp          ****     f(i+1, i);
 121 006e 8D5001   		leal	1(%eax), %edx
 126 0071 89442404 		movl	%eax, 4(%esp)
 127 0075 89542408 		movl	%edx, 8(%esp)
 128 0079 E8000000 		call	_printf

    So the really intriguing question is not why the warning is being triggered for caller1, but why it's being triggered for only one of the functions when the same optimization is being done for both of them.

    → Reply
  • »
    »
    8 years ago, # ^ |
      Vote: I like it +20 Vote: I do not like it

    What I think is a bug here is that this is a warning (and not a note). This makes it pretty painful to combine -Wall with -Werror.

    → Reply
»
8 years ago, # |
  Vote: I like it 0 Vote: I do not like it

Even though the line of code referenced is in f the warning says "In function ‘void caller1(int)’". It's a valid warning for that context.

→ Reply
»
8 years ago, # |
  Vote: I like it +13 Vote: I do not like it

Yes, I understand why the warning may appear if I compare two values with known result. But here the situation is more complex.

f could be a much more complex function. It performs some checks in the beginning and then does computations. Here I call it from some place where check is unneeded, and I receive a warning. I don't understand what does the compiler suggest: "Hey, this if is redundant, you'd better write some another function f without this check especially for this caller!"

I could even understand this warning if the caller1 was the only caller of f. But this is not the case.

→ Reply
  • »
    »
    8 years ago, # ^ |
    ← Rev. 2   Vote: I like it +5 Vote: I do not like it

    Probably, The point is not that you remove check. It so that you know that compiler get advantage of UB and may work not as you intended(eg expecting overflow) and you may need to use fwrapv or change code

    But I'm surprised it's in Wall

    → Reply
  • »
    »
    8 years ago, # ^ |
      Vote: I like it +23 Vote: I do not like it

    It seems that g++ inlines f into both callers. Then the suggestion makes sense (it should probably warn twice, though).

    → Reply
»
8 years ago, # |
  Vote: I like it +10 Vote: I do not like it

The Wstrict-overflow=n warning exists in GCC 6.1 as well.

https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Warning-Options.html#Warning-Options

→ Reply
»
8 years ago, # |
  Vote: I like it +47 Vote: I do not like it

I wonder where people participating in this discussion acquired knowledge about compilers. Did you take a university class or is it far more simple than that? What resources can I use to be able to participate in the discussion?

Thanks.

→ Reply
  • »
    »
    8 years ago, # ^ |
      Vote: I like it +21 Vote: I do not like it

    I gained many knowledge from such discussions, actually. Some notes are posted, some links are posted, and if you google it later you'll find out something interesting and useful.

    More, don't be afraid of reading g++ manpages. It is frightening as a whole so I wouldn't recommend you reading it from top to bottom (however, I know few people who did, crazy they are). But if you know what you need (e.g. a particular option) you get an explanation first-hand.

    Finally, I gain helpful advice at work. There are many experienced people in my openspace, every day something is discussed, you only have to listen.

    → Reply
    • »
      »
      »
      8 years ago, # ^ |
        Vote: I like it 0 Vote: I do not like it

      Thank you.

      It's amusing. That's how I learned just about anything I now know about programming and algorithms -- by researching the little things I was interested in. And looking back they all together are now what make me who I am as a programmer. Yet, there is still a lot to be learnt :o

      → Reply
»
8 years ago, # |
← Rev. 4   Vote: I like it -23 Vote: I do not like it

clang only doesn't warn you. I am getting the same wrong output, compiling with both of them. I am passing INT_MAX as the argument to caller1 and caller2, and I am getting 2147483647 -2147483648 as the output of caller2.

I checked the code produced by passing -fdump-tree-optimized to g++ and it seems f is just being used like an inline function and there is no reference to f anywhere in the code :| (maybe I am wrong) and the code for function f still exists in the output!!! both caller1 and caller2 have their own optimized copy of f inside them.

[edit] And yes that if (x > y) comparison is completely gone in the optimized code for both caller1 and caller2! none of these happen if I compile with -O0 or -O1.

→ Reply
  • »
    »
    8 years ago, # ^ |
    ← Rev. 2   Vote: I like it +25 Vote: I do not like it

    I am getting the same wrong output

    It's not wrong

    → Reply
    • »
      »
      »
      8 years ago, # ^ |
        Vote: I like it 0 Vote: I do not like it

      But I get -2147483648 2147483647 when I compile with -O0.

      → Reply
      • »
        »
        »
        »
        8 years ago, # ^ |
          Vote: I like it 0 Vote: I do not like it

        What do you expect to get?

        → Reply
        • »
          »
          »
          »
          »
          8 years ago, # ^ |
          ← Rev. 4   Vote: I like it 0 Vote: I do not like it

          I expect to get -2147483648 2147483647, which I get when I compile the code below with no optimization (-O0). And I get 2147483647 -2147483648 when I use -O2, which I suppose is incorrect, since that if statement should guarantee that x should be less than or equal to y, or they are swapped otherwise.

          #include <bits/stdc++.h>

using namespace std;

void f(int x, int y) {
    if (x > y) swap(x, y);
    printf("%d %d\n", x, y);
}

void caller2(int i) {
    f(i+1, i);
}

int main() {
    caller2(INT_MAX);
}

          What I am trying to say is that I get different outputs regarding whether I use -O0 or -O2.

          → Reply
          • »
            »
            »
            »
            »
            »
            8 years ago, # ^ |
              Vote: I like it +3 Vote: I do not like it

            But signed integer overflow is undefined behavior.

            → Reply
      • »
        »
        »
        »
        8 years ago, # ^ |
          Vote: I like it +3 Vote: I do not like it

        So what?

        → Reply