Indefinite behavior from operators

Question

Indefinite behavior from operators

Navigation

#1 by (1 votes)
#2 by (1 votes)

0

Good morning,

I am doing a program in C where I need to execute the following block of code:

uint8_t local_index=0;
uint8_t buffer_start[100];
uint16_t temporal_uint16;

temporal_uint16 = (uint16_t) (buffer_start[local_index++] << 8) | buffer_start[local_index++];

The idea is to take two consecutive bytes of an array (depending on the value of local_index at that moment) and put them inside a variable of the type uint16_t doing logical operations.

The program is working correctly, but compiling it produces the following warning and I would like to understand why:

"operation on 'local_index' may be undefined"

I have other alternatives to solve the problem, what I want is to simply understand the why of that warning, since in my opinion the syntax of the code line is correct.

Thank you very much!

array c

asked by cventu 19.10.2016 в 22:50

source

2 answers

1

The problem is that in the same instruction you access a fix position while modifying it and increasing the index:

(buffer_start[local_index++] << 8) | buffer_start[local_index++]
 \------modificar elemento------/
              \---índice--/                       \---índice--/

It is not specified what operation will be done before:

(buffer_start[local_index++] << 8) before buffer_start[local_index++] .
buffer_start[local_index++] before (buffer_start[local_index++] << 8) .

The compiler has the freedom to reorder the operations to apply optimizations, whether it is moving bits to the right, increasing the indexes or reading the array element, so depending on what operation is done before the result will be different, ergo: the operation can be undefined since it can give different results in different compilers or different configurations in the same compiler.

To solve it, you should separate the operation and apply it in the order you need:

uint16_t l = (buffer_start[local_index++] << 8);
uint16_t r = buffer_start[local_index++];
temporal_uint16 = (uint16_t) l | r;

uint16_t r = buffer_start[local_index++];
uint16_t l = (buffer_start[local_index++] << 8);
temporal_uint16 = (uint16_t) l | r;

answered by 20.10.2016 в 09:17

Change icons when plotting a route in Google Maps Error extracting database text

score 1 · Accepted Answer

Pre-increment and post-increment operators must be used with care. You are assuming that the code to execute is equivalent to this:

temporal_uint16  = (uint16_t) (buffer_start[local_index] << 8);
local_index++;
temporal_uint16 += (uint16_t) (buffer_start[local_index]);
local_index++;

But it does not have to be like that. The standard does not specify when the increase should occur. It leaves the compiler a certain degree of freedom to choose the best way to do it. Thus, the post-increment could occur at the end of the execution of the line, then your code would be equivalent to this:

temporal_uint16  = (uint16_t) (buffer_start[local_index] << 8);
temporal_uint16 |= (uint16_t) (buffer_start[local_index]);
local_index++;
local_index++;

And the same happens with the pre-increments. It could happen that they were all cascaded before executing any other operation of the current line.

As a general rule, do not include several pre-increment or post-increment operations in the same instruction if these operations affect the same variable since the final result will depend on the compiler and I do not think you like the results.

You can choose to cut the line:

temporal_uint16  = (uint16_t) (buffer_start[local_index++] << 8);
temporal_uint16 |= (uint16_t) (buffer_start[local_index++]);

or you can replace the increments with sums:

temporal_uint16 = (uint16_t) (buffer_start[local_index] << 8)
                | buffer_start[local_index+1];
local_index+=2;

Greetings.