Integer Overflow affects abs()

[hockeydave]

There seems to be a problem with a local variable declared as an int32_t that overflows versus a local static variable that overflows with regards to the abs function call as shown below:

Code: [Select all] [Expand/Collapse]

1. static uint32_t M_HashGen(const char* s)
2. {
3.    
4.   const int32_t A_PRIME = 54059;
5.   const int32_t B_PRIME = 76963;
6.   const int32_t C_PRIME = 999983;
7.  
8.   int32_t myVar = 37;
9.  
10.  
11.   while (*s) {
12.       myVar = (myVar * A_PRIME) ^ (s[0] * B_PRIME);
13.       printf ("myVar: %d, abs(myVar): %d, s[0] = %c\n", myVar, abs(myVar));
14.       s++;
15.   }
16.  
17.    
18.   /* Since all ints are used in the above algorithm, ensure that a positive value
19.      is used as the input to the modulo operation */
20.   return (abs(myVar) % C_PRIME);
21.  
22. }

GeSHi © Codebox Plus Extension

Output:
myVar: 8388078, abs(myVar): 8388078
myVar: -1817466845, abs(myVar): -1817466845
myVar: 1228909722, abs(myVar): 1228909722
myVar: -929334365, abs(myVar): -929334365
myVar: -649350200, abs(myVar): -649350200
myVar: -457962121, abs(myVar): -457962121
myVar: -788115438, abs(myVar): -788115438
myVar: 1346474570, abs(myVar): 1346474570
myVar: -2032483059, abs(myVar): -2032483059
myVar: -149788679, abs(myVar): -149788679

As can be seen from the output, the abs() calls fail on negative numbers after the first integer overflow of myVar. However if the code above is changed to have myVar become static as shown below, all is well:

Code: [Select all] [Expand/Collapse]

1. static uint32_t M_HashGen(const char* s)
2. {
3.    
4.   const int32_t A_PRIME = 54059;
5.   const int32_t B_PRIME = 76963;
6.   const int32_t C_PRIME = 999983;
7.  
8.   static int32_t myVar = 37;
9.  
10.  
11.   while (*s) {
12.       myVar = (myVar * A_PRIME) ^ (s[0] * B_PRIME);
13.       printf ("myVar: %d, abs(myVar): %d, s[0] = %c\n", myVar, abs(myVar));
14.       s++;
15.   }
16.  
17.    
18.   /* Since all ints are used in the above algorithm, ensure that a positive value
19.      is used as the input to the modulo operation */
20.   return (abs(myVar) % C_PRIME);
21.  
22. }

GeSHi © Codebox Plus Extension

Output:
myVar: 8388078, abs(myVar): 8388078
myVar: -1817466845, abs(myVar): 1817466845
myVar: 1228909722, abs(myVar): 1228909722
myVar: -929334365, abs(myVar): 929334365
myVar: -649350200, abs(myVar): 649350200
myVar: -457962121, abs(myVar): 457962121
myVar: -788115438, abs(myVar): 788115438
myVar: 1346474570, abs(myVar): 1346474570
myVar: -2032483059, abs(myVar): 2032483059
myVar: -149788679, abs(myVar): 149788679

As can be seen the abs() performs correctly as long as myVar is declared as static. Is this a bug with the compiler or do I need to use a compiler option to fix or am I completely missing something.
FYI: I can't replicate with VS 2022 running this function on a WInodws 11 PC or other microcontrollers (Infineon and STM32).
Any help would be greatly appreciated

[ESP_igrr]

Could you please describe in which environment are you compiling this code (IDF, Arduino, PlatformIO) and what optimization level are you using?

I can say that if this code is compiled at -Os or -O2 optimization level, the call to "abs" would be optimized away. This is because the compiler performs optimizatitons under assumption that undefined behavior does not occur. Since singed integer overflow is an undefined behavior, the compiler can very easily eliminate the call to "abs"! Not having to call "abs" is both faster and produces smaller code size, so the optimizer will use this opportunity.

You can see this illustrated in the Compiler Explorer:

https://godbolt.org/z/qznsYPMY6 (with ARM gcc 8.2)

I have moved the abs() call onto a separate line to make it more obvious that this call gets eliminated. From
mov r2, r9 @, myVar
mov r1, r9 @, myVar
you can see that both 2nd and 3rd printf placeholder receive the same value — myVar, and there is no "abs" operation.

Different compiler versions produce different behavior. If you use the dropdown list to select a different compiler, you will see that some versions will actually perform an "abs" operation (e.g. "ARM gcc trunk linux")

As for avoiding this, i think the best option is to use an unsigned integer type for this calculation (uint32_t). Unsigned integer overflow is not an undefined behavior in C, so the calculation won't depend on the compiler.

[hockeydave]

Thank you very much for taking the time to investigate!!!