Compiler warning about undefined operation

Question

Here is my code to deserialize uint64_t value from a byte array:

uint8_t* s = blah;

uint64_t output =
     (((uint64_t)*s++) << 56) +
     (((uint64_t)*s++) << 48) +
     (((uint64_t)*s++) << 40) +
     (((uint64_t)*s++) << 32) +
     (((uint64_t)*s++) << 24) +
     (((uint64_t)*s++) << 16) +
     (((uint64_t)*s++) <<  8) +
     (((uint64_t)*s)        );

I am compiling this code with g++ version 5.4 on Ubuntu.

While the code works exactly as expected, I get a compile time warning on the last line:

 warning: operation on 's' may be undefined [-Wsequence-point]

Wondering what could potentially be wrong and how I can fix it. Regards.

Possible duplicate https://stackoverflow.com/questions/4176328/undefined-behavior-and-sequence-points — Baum mit Augen, Nov 11 '16 at 23:22
The order of evaluation of the various `s++` subexpressions is unspecified. An in this case, where the same object is modified multiple times, the standard goes beyond saying that they can be evaluated in any order (which would be bad enough) and says that the behavior is completely undefined. — Keith Thompson, Nov 11 '16 at 23:25
Any reason you don't use a simple loop? Let the compiler do the unrolling. — too honest for this site, Nov 11 '16 at 23:25

score 3 · Accepted Answer · answered Nov 11 '16 at 23:22

Your code has unsequenced modifications of the variable s. This behaviour is undefined. You can fix it by using indexes instead of advancing the pointer (I'm assuming you're reading a little-endian variable. This is non-obvious from your code):

uint64_t output =
     (((uint64_t)s[0]) << 56) +
     (((uint64_t)s[1]) << 48) +
     (((uint64_t)s[2]) << 40) +
     (((uint64_t)s[3]) << 32) +
     (((uint64_t)s[4]) << 24) +
     (((uint64_t)s[5]) << 16) +
     (((uint64_t)s[6]) <<  8) +
     (((uint64_t)s[7])        );

supercat · Answer 2 · 2016-11-11T23:28:50.630

Your code modifies the value of s seven times within an instruction, and then uses it an eighth time. Neither the C Standard or the C++ Standard imposes any requirements upon what an implementation would do if it receives input that could cause such code to be executed or make its execution inevitable. Note, though, that the existence of such code in an application would not affect the correctness of the application if the program never receives input that would make the execution of the code inevitable, so it isn't a compilation error. Adjusting the code using s[0], s[1], s[2], etc. would make the code clearer and avoid such problems.

1) That's C++, 2) The C standard is clear about this being undefined behaviour. 3) Same for C++. — too honest for this site, Nov 11 '16 at 23:24

Compiler warning about undefined operation

2 Answers2