30

When I make the following calculation:

unsigned long long int data_size = 60123456 * 128 * sizeof(double);
printf("data_size= %llu  \n", data_size);

I surprisingly get overflow warning:

test.c:20:49: warning: overflow in expression; result is -894132224 with type 'int' [-Winteger-overflow]
    unsigned long long int data_size = 60123456 * 128 * sizeof(double);
                                                ^
1 warning generated.

I cannot understand why this error appears even though I am using unsigned long long int! Can someone explain why? Thank you

Kristofer
  • 1,457
  • 2
  • 19
  • 27

5 Answers5

45

Overflow occurs before value is assigned to the variable. To avoid that use long long suffix:

60123456LL * 128 * sizeof(double)

(comments also suggest ULL which is not necessary here because values are within signed range, but that would be the generic way to go)

Jean-François Fabre
  • 137,073
  • 23
  • 153
  • 219
  • Mentionning that U is not needed: +1 – glglgl Jul 24 '17 at 13:22
  • 22
    @glglgl `U` is not need here in this case, yet `U` is better for the generic case. Consider `60123456LL * 300000000000`. Without the `U`, this again is signed integer overflow even though the math product fits in `unsigned long long`. Best to do the math in a least with width of the target type. – chux - Reinstate Monica Jul 24 '17 at 14:19
  • 2
    @chux agreed, pretty much all the coding guidelines I've ever seen don't appreciate silent signed -> unsigned conversion. – CodeMonkey Jul 25 '17 at 06:36
18

You are converting to "long long" too late. 60123456 * 128 is still an overflowing int expression.

Try 60123456LL * 128 instead. (LL because the resulting value is not guaranteed to fit in a long.)

LarsW
  • 1,514
  • 1
  • 13
  • 25
glglgl
  • 89,107
  • 13
  • 149
  • 217
  • 2
    `LL` is useful to get closer to a calculation that fits, yet the target type is `unsigned long long`, not `long long`. ULL is best for general computation of constants meant for an `unsigned long long` target. – chux - Reinstate Monica Jul 25 '17 at 06:56
12
unsigned long long int data_size = 60123456 * 128 * sizeof(double);  // trouble-some code

The type of destination unsigned long long int data_size = is irrelevant to the product calculation 60123456 * 128 * sizeof(double).

Best to insure math is done using at least the size of the target type to avoid overflow. In OP's case, that implies a constant with LLU.


There are 2 product calculations, each with their own type math.

60123456 is an int or long depending on int range. Let us assume it is an int.

60123456 * 128 is an int * int. The math product 7695802368 exceeds 32-bit signed integer range, thus signed integer overflow or undefined behavior (UB) with a 32-bit int.

If 60123456 * 128 did not overflow, says 64-bit int, than the next multiplication would be * sizeof(double); and so int * size_t results in a size_t type product.

Product calculation should use at least unsigned long long math as below:

unsigned long long int data_size = 1LLU * 60123456 * 128 * sizeof(double);

// or
unsigned long long int data_size = 60123456LLU * 128 * sizeof(double);

// or 
unsigned long long int data_size = 60123456;
data_size *= 128 * sizeof(double);

// or avoiding naked magic numbers
#define A_HEIGHT 60123456
#define A_WIDTH 128
unsigned long long int data_size = 1LLU * A_HEIGHT * A_WIDTH * sizeof(double);

The sizeof (double) hints that code is attempting to find the size of some 2D-like structure. I'd expect code like the following. Notice the result type of sizeof is size_t, so the product math is done using at least size_t math.

size_t data_size = sizeof(double) * 60123456 * 128;
printf("data_size= %zu\n", data_size);

See also Why write 1,000,000,000 as 1000*1000*1000 in C? and my answer reasons not to use 1000 * 1000 * 1000 for applicable details..

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • 1
    Just a small remark: *`int * size_t` resulting in a `size_t` type product.* You are skipping a step that might be of importance: `int * size_t` first converts `int` to `size_t` and then performs `size_t * size_t -> size_t`. If the `int` is negative, the result may be surprising. – chqrlie Jul 24 '17 at 17:31
  • 1
    @chqrlie True. That surprising outcome is more likely with `int` objects unlike the positive `int` _integer constants_ here. – chux - Reinstate Monica Jul 24 '17 at 17:36
4

The constants 60123456 and 128 are of type int, therefore the expression 60123456 * 128 is also of type int. The value of this expression would overflow the range of an int. The compiler is able to detect it in this case because both operands are constants.

You should use the ULL suffix on the first operand to make the type of the constant unsigned long long. That way it matches the type being assigned to. Then for any operation involving this value, the other operand will be promoted to unsigned long long before the operation is applied and you won't have any overflow.

So the resulting expression should look like this:

unsigned long long int data_size = 60123456ULL * 128 * sizeof(double);
dbush
  • 205,898
  • 23
  • 218
  • 273
3

The lexer attaches types on constants during translation phase 7 (conversion of preprocessing tokens into C tokens.) cf ISO9899 5.1.1.2 Translation phases. As the others said, the lexer will attach on your constants the type int instead of unsigned long long as you wish, and in order for the default arithmetic conversions to generate a constant of type unsigned long long on the right side of the assignment you need to say to the lexer to attach the type unsigned long long on at least one constant from the operation * that generates the overflow, so by writing 60123456ULL * 128 or 60123456 * 128ULL or both.

alinsoar
  • 15,386
  • 4
  • 57
  • 74