1

I ran into a bug in my program:

for (int i = 0; i < objArray.size() - 1; ++i)

In my case objArray.size() is an unsigned long long and an empty vector minus 1 equals about 18 quintillion. I was wondering, does the loop on every iteration have to cast an int to an unsigned long long? I checked the assembly and while using an int creates different code than size_t without optimisations, with -O2 specified it generates exactly the same assembly. Does this mean it's not implicitly casting?

I don't understand assembly, but the code it generated was:

test rcx, rcx
je .L32
add rdx, rax

and then :

cmp rdx, rax
jne .L28
Zebrafish
  • 11,682
  • 3
  • 43
  • 119
  • Just to be certain... your CPU is 64 bit, right? – byxor Oct 13 '17 at 20:12
  • Note that, supposing `i` is constant in the loop, the compiler may assume that `i` is never negative. So it may just reinterpret as `unsigned` and compare – WorldSEnder Oct 13 '17 at 20:13
  • My CPU is 64 bit. In Visual Studio running in 64 bit mode it was an infinite loop. Even after the int got over about 2.2 billion it just overflowed and kept going, because it was comparing to about 18 quintillion each time I think. The assembly I added was from godbolt.org – Zebrafish Oct 13 '17 at 20:15
  • Just for confirmation though, is it always better form to use a size_t in this situation? Most code I see floating around doing it the way I did it seems kinda normal. – Zebrafish Oct 13 '17 at 20:17
  • The answer to the title question, without needing to look at the code, is no. There is no such thing as an implicit cast. A **cast** is something you write in your source code to tell the compiler to do a **conversion**. – Pete Becker Oct 13 '17 at 22:13

2 Answers2

1

This may be caused by a compiler optimization. The c++ standard says that overflow of signed integral types is undefined. In this case, i starts at 0. Supposing that i is not written to in the loop, the compiler can thus deduce, that i >= 0, since overflowing is undefined behaviour and can be pruned.

Normally, for an signed-unsigned comparison, the signed value would have to be converted to the unsigned type following the rules you can see here. These rules are the reason for the compiler warnings when comparing a signed and unsigned type (leading to confusion, e.g. -1 > 2U is true). In this case, that doesn't matter though.

With the assumption i >= 0 and 2-complement signed types though, the compiler can safely reinterpret i as an unsigned long long since he knows the sign-bit is 0. That's what your assemly output shows.

Now, we can see that there is indeed a bug. Suppose objArray.size() - 1 does not fit into a positive signed int. This would cause i to overflow, thus cause undefined behaviour which is always bad news.

WorldSEnder
  • 4,875
  • 2
  • 28
  • 64
  • size( ) is not signed. It is doing signed/unsigned comparison which would use the promotion rules https://stackoverflow.com/questions/5416414/signed-unsigned-comparisons – Beached Oct 13 '17 at 20:22
  • @Beached doesn't matter. When `i` is negative it already overflowed and you can't say anything about undefined behaviour. – WorldSEnder Oct 13 '17 at 20:23
  • So what you're saying is that with optimisations turned on it just turned my int declaration into an unsigned long long for simpler comparison? – Zebrafish Oct 13 '17 at 20:27
  • @Zebrafish judging from your bytecode, yes. But the important thing is that you can not rely on that behaviour and it may change in the future. – WorldSEnder Oct 13 '17 at 20:28
  • I don't see how i can be negative, it starts at 0 and is checked if it is less than size( )-1. size() is not an int but a size_t – Beached Oct 13 '17 at 20:30
  • @Beached judging from the assembly, it can not. But the code allows for an overflow, if `sizeof(int) < sizeof(size_t)` (again, strictly speaking overflowing an int is UB) – WorldSEnder Oct 13 '17 at 20:31
  • Seems there's a major gotcha case in the second answer by Nawaz in that signed/unsigned link. I never would have believed int i = -1; unsigned int j = 1; i < j is false – Zebrafish Oct 13 '17 at 20:39
0

Lets dissect the code for (int i = 0; i < objArray.size() - 1; ++i)

you are doing a comparison between and int and a size_t. The size( )-1 is an unsigned underflow when the array is empty and results in a value of std::numeric_limits::max( ). The comparison, will be signed/unsigned and use the type promotion rules as outlined here Signed/unsigned comparisons

Beached
  • 1,608
  • 15
  • 18
  • Though if it was promoting one type to another it should show up in the assembly, right? I think WorldSender is right when he says that as an optimisation it just turned my int declaration into the size_t type for more efficient comparison. – Zebrafish Oct 13 '17 at 20:29
  • It's also UB when size( ) < 1 as the loop index will overflow – Beached Oct 13 '17 at 21:32