2

I have some code that has recently been 'ported' to VS. What follows is the statement and the assembler generated, hopefully the 'error' is obvious. Of course the way it has generated this code may be a feature of VS, but I can't say I've seen this before.

switchcount = (int) (*d++) + (*d++<<8) + (*d++<<16) + (*d++<<24);

00A722F6  mov         eax,dword ptr [d]  
00A722F9  movzx       ecx,byte ptr [eax]  
00A722FC  mov         edx,dword ptr [d]  
00A722FF  movzx       eax,byte ptr [edx]  
00A72302  shl         eax,8  
00A72305  add         ecx,eax  
00A72307  mov         edx,dword ptr [d]  
00A7230A  movzx       eax,byte ptr [edx]  
00A7230D  shl         eax,10h  
00A72310  add         ecx,eax  
00A72312  mov         edx,dword ptr [d]  
00A72315  movzx       eax,byte ptr [edx]  
00A72318  shl         eax,18h  
00A7231B  add         ecx,eax  
00A7231D  mov         dword ptr ds:[0AD8CA8h],ecx  
00A72323  mov         ecx,dword ptr [d]  
00A72326  add         ecx,1  
00A72329  mov         dword ptr [d],ecx

So the code the compiler has generated is essentially

switchcount = (int) (*d) + (*d<<8) + (*d<<16) + (*d<<24);
d += 4 ;

Can anyone show me how to convince the compiler to generate the correct code?

Marwie
  • 3,177
  • 3
  • 28
  • 49
mvandere
  • 21
  • 1

2 Answers2

3

C does not allow modifying the same variable multiple times between sequence points, and the compiler is free to handle it any way it likes - generating the code it does is perfectly fine as far as the C standard is concerned, but the more useful behaviour would be to abort compilation, or at least issue a warning like clang does:

warning: multiple unsequenced modifications to 'd' [-Wunsequenced]

A correct way to write the code would be

switchcount = (int) (d[0]) + (d[1]<<8) + (d[2]<<16) + (d[3]<<24);
d += 4;

As a side note, the cast to int is unnecessary because of integer promotion, but if you expect the code to be read by C novices, you may leave it in for clarity.

Depending on the input values for d, you should replace the cast to int with a cast to unsigned - otherwise, if d[3] is at least 128, your last term will shift value bits into the sign bit, which is again 'undefined behaviour' (but normally works as expected if integers are represented by two's complement).

Christoph
  • 164,997
  • 36
  • 182
  • 240
  • Thanks very much for the clear and concise answer. It's sad that the designers of these languages didn't remember primary school BODMAS/BOMDAS when working this out. The brackets should ensure that the increment happens Left to right on the closing brackets. – mvandere Aug 29 '14 at 21:38
0

The behaviour in C for handling subexpression like d++ mutliple times in one statement is undefined. You are now facing the different ways of interpretations of the statement by different compilers - Avoid using expressions in this manner.

See Pre & post increment operator behavior in C, C++, Java, & C#

Community
  • 1
  • 1
Marwie
  • 3,177
  • 3
  • 28
  • 49
  • Again, whilst this example is correct for most compilers, it is sad that simple () do not alter its behaviour. – mvandere Aug 29 '14 at 21:42