13

In my program, I found the loop unable to exit correctly when i is int32_t. It seems like integer overflow, and is much larger than 10, and the loop does not stop. Please tell me what happened and how I can avoid this error in a large project.

#include <iostream>
#include <stdint.h>
int f(int n){

    for (int32_t i = 0; i < 10; ++i)
    {
        int64_t time = 4500000000 +  (i) * 500000000;
        std::cout << time<< " i: " << i << std::endl;

    }
    return 0;
}

int main ()
{
    return f(10);
}

code link

Boann
  • 48,794
  • 16
  • 117
  • 146
huhu
  • 131
  • 4
  • 7
    `i*500000000` will cause signed integer overflow (undefined behaviour). To avoid this, think about the possible range of values when using multiplication – M.M Apr 21 '22 at 04:30
  • `4500000000 + (i) * 500000000;` --> `4500000000 + i * (int64_t)500000000;` or `4500000000 + i * UINT64_C(500000000);`. – chux - Reinstate Monica Apr 24 '22 at 05:09

1 Answers1

23

If you use GCC 11.2.0 and the -Wall -O2 options, you will see a warning about undefined behavior:

test.cpp: In function 'int f(int)':
test.cpp:7:42: warning: iteration 5 invokes undefined behavior [-Waggressive-loop-optimizations]
    7 |         int64_t time = 4500000000 +  (i) * 500000000;
      |                                      ~~~~^~~~~~~~~~~
test.cpp:5:27: note: within this loop
    5 |     for (int32_t i = 0; i < 10; ++i)
      |                         ~~^~~~

The compiler knows that 5 * 500000000 is too large to fit in an int (which is typically 32-bit). Signed integer overflow is undefined behavior in C++. Therefore, the compiler is free to assume that this overflow never happens, so it will assume that i can never reach 10, so it can get rid of the part of your for loop that checks i < 10. I know that sounds crazy, but if your program does undefined behavior, the compiler is free to do whatever it wants.

Just add some casts to specify that you want to do 64-bit arithmetic. This eliminates the warnings, the overflows, and the undefined behavior:

int64_t time = (int64_t)4500000000 + i * (int64_t)500000000;

Update: For a larger project that could have more bugs, you might consider using GCC's -fwrapv option, which makes the behavior of signed integer overflow be defined. You could also use -fsanitize=signed-integer-overflow or -fsanitize=undefined to detect these issues at run time, if your toolchain supports those options.

David Grayson
  • 84,103
  • 24
  • 152
  • 189
  • 3
    `for (int64_t i = 0; ...` is another option. – David C. Rankin Apr 21 '22 at 04:35
  • 6
    you just need `4500000000LL` instead of `(int64_t)4500000000`. Same to `500000000LL` – phuclv Apr 21 '22 at 04:35
  • 2
    I believe `LL` means `long long` so that seems less portable to me. I'm not totally sure what `long long` will be on every system. – David Grayson Apr 21 '22 at 04:40
  • 6
    You generally shouldn't be C-style casting in C++. `std::int64_t{4500000000}` seems most right to me here – Ryan Haining Apr 21 '22 at 04:42
  • @phuclv I was afraid that might not work on Microsoft Visual C++, but evidently it does: [`LL` vs `i64` suffix in C++ Visual Studio compiler](https://stackoverflow.com/q/70851015/5987). – Mark Ransom Apr 21 '22 at 04:42
  • My favorite exploration into the murky effects of undefined behavior is: [Undefined behavior can result in time travel (among other things, but time travel is the funkiest)](https://devblogs.microsoft.com/oldnewthing/20140627-00/?p=633). – Mark Ransom Apr 21 '22 at 04:48
  • 1
    @DavidGrayson I believe long long required to be at least 64 bits. – apple apple Apr 21 '22 at 04:51
  • 4
    The `long long` type is guranteed to have atleast 64 bits. https://stackoverflow.com/a/271132/14065 – Martin York Apr 21 '22 at 05:02
  • 5
    @DavidGrayson `LL` is in C++11 so there's nothing non-portable in it. In fact `LL`, `UINT64_C` and `uint64_t` all were introduced since C++11, so if you can use `int64_t` then you must be able to use `LL` – phuclv Apr 21 '22 at 05:05
  • 1
    @JHBonarius What do you mean by not compiler checked? Of course the compiler checks stuff about the cast. – Passer By Apr 21 '22 at 05:45
  • 1
    @PasserBy my bad, misread something. Still, C-style casts can have weird side effects. – JHBonarius Apr 21 '22 at 06:42
  • 1
    @RyanHaining "You generally shouldn't be C-style casting in C++" Why? Is the syntax not complicated enough? – Boann Apr 21 '22 at 12:40
  • 2
    If you don't trust `LL` and prefer to be explicit, you may consider this user defined literals: https://stackoverflow.com/a/65120744/4944425 – Bob__ Apr 21 '22 at 12:59
  • 4
    @Boann I think the reason is that C-style casts can be either `reinterpret_cast`, `const_cast`, or `static_cast` depending on the types involved. C++-style casts express the intent explicitly. – Barmar Apr 21 '22 at 15:37
  • 2
    @Boann as Barmar said, C-style casts look simple but [are a sledgehammer](https://en.cppreference.com/w/cpp/language/explicit_cast)(and can actually be a combination of several explicit casts). You should be casting rarely, and when you do cast, it should be obvious what your intention is and not easy to overlook. – Ryan Haining Apr 21 '22 at 18:39
  • 2
    @RyanHaining far too often the reason for a cast is to just silence the compiler errors. The cast that's most flexible and easy to type wins out. That just makes it a double red flag. – Mark Ransom Apr 21 '22 at 22:57
  • 2
    -fsantisize=undefined may be a typo, use -fsanitize=undefined – huhu Apr 24 '22 at 03:37
  • `LL` not needed with `4500000000` as `4_500_000_000` is certainly a 64-bit constant already. – chux - Reinstate Monica Apr 24 '22 at 05:08