3

I'm having some weird behavior that I am not expecting from my optimizer. Basically it's a case where a variable is overflowing and the logic I'm trying to use to handle that is breaking.

Here is the complete program (weird.cpp):

#include <stdio.h>

class Example {
public:
  void Change() {
    change_count_++;

    // Check for overflow. If it does, set to 1 which is still valid
    if(change_count_ <= 0) {
      change_count_ = 1;
    }
  }

public:
  int change_count_ = 0;
};

int main() {
  Example example;
  printf("Pre: %d\n", example.change_count_);

  example.change_count_ = 2147483647; // MAX_INT
  printf("Mid: %d\n", example.change_count_);

  example.Change();
  printf("Pst: %d\n", example.change_count_);

  return 0;
}

When building using these commands:

gcc  -fPIC -g3 -O1 -g   -std=gnu++11  weird.cpp -o optimized
gcc weird.cpp -o normal

The program normal executes as expected with the following output:

Pre: 0
Mid: 2147483647
Pst: 1

But optimized gives the following unexpected behavior:

Pre: 0
Mid: 2147483647
Pst: -2147483648

Attaching to a debugger shows me that the increment is done last in the function. Is behavior for overflows undefined in C++? Or is there a different way I should be handling this?

Here is the version of clang I'm using:

tiny.local:~/scratch/weird_inc$ gcc -v
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 10.0.0 (clang-1000.10.44.4)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

I tested with gcc 5.4.0 in Ubuntu and it gave me correct answers.

TinyTheBrontosaurus
  • 4,010
  • 6
  • 21
  • 34

1 Answers1

4

Signed integer overflow is undefined behavior in the current C++ standard.

A compiler is free to assume that undefined behavior never happens, and is not required to compile code that depends on undefined behavior.

This means that the compiler can safely assume that incrementing a positive value greater than 0 will never result in a negative value, or 0.

Your compiler is aggressively optimizing the code. The compiler sees that a variable is being initialized to a positive value, and incremented. Therefore, the compiler assumes that the result cannot be negative or 0, and therefore the comparison does not even get compiled, because it can't possibly ever be true.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
  • Wow. Now i'm fascinated about why that's undefined behavior. I've changed to `if((change_count_ <= 0) || (change_count_ == INT_MAX))`, which I imagine is ironically now less efficient – TinyTheBrontosaurus Aug 25 '19 at 22:13
  • @TinyTheBrontosaurus Why not just use an `unsigned int`? – melpomene Aug 25 '19 at 22:14
  • I would be surprised if you will be able to measure the alleged performance hit. Modern CPUs are just too fast, and spend a lot of time waiting to be fed with RAM. An extra comparison, plus speculative execution, is likely to be done at the same time as waiting for the next chunk of memory to load into the CPU cache. And it's undefined behavior because the question I linked to gives a site to the standard that specifies this: "If during the evaluation of an expression, the result is not mathematically defined or not in the range of representable values for its type, the behavior is undefined." – Sam Varshavchik Aug 25 '19 at 22:15
  • @melpomemt because that's not the question :-) But thank for the suggestion. I switched to `unsigned int` in my code before I asked the question, but now I have some extra casts due to interfacing with some other code – TinyTheBrontosaurus Aug 25 '19 at 22:15