Boris Brodski
Boris Brodski

Reputation: 8695

GCC crash in 64bit mode

Compiling 7-zip source code with MinGW GCC 4.8.2 on Windows64 I found a crash.

Here is a code snippet of 7-zip 9.20, Sha1.cpp:

const unsigned kBlockSize = 64;

...

void CContext::UpdateRar(Byte *data, size_t size, bool rar350Mode) {
   ...
   for (int i = 0; i < kBlockSizeInWords; i++) {
       UInt32 d = _buffer[i];
       data[i * 4 + 0 - kBlockSize] = (Byte)(d);      // <<= DISASSEMBLED
       data[i * 4 + 1 - kBlockSize] = (Byte)(d >>  8);
       data[i * 4 + 2 - kBlockSize] = (Byte)(d >> 16);
       data[i * 4 + 3 - kBlockSize] = (Byte)(d >> 24);
   }
}

Compiling with MinGW GCC 4.8.2 (on Windows) gets:

0x0000000045b65f75 <+149>:   mov    eax,0xffffffc0
0x0000000045b65f7a <+154>:   nop    WORD PTR [rax+rax*1+0x0]
0x0000000045b65f80 <+160>:   mov    r10d,DWORD PTR [r11+0x24]
0x0000000045b65f84 <+164>:   mov    edx,eax
0x0000000045b65f86 <+166>:   add    r11,0x4
0x0000000045b65f8a <+170>:   mov    ecx,r10d
0x0000000045b65f8d <+173>:   mov    BYTE PTR [rbx+rdx*1],r10b

In the last line rbx points to the data and 64 should be subtracted from this address. This is done by adding -64. In the first line of the assembler snippet 0xffffffc0 (-64 (Int32)) saved into eax and then moved to edx. But in the last line

0x0000000045b65f8d <+173>:   mov    BYTE PTR [rbx+rdx*1],r10b

the rdx register instead of the edx register used. At this point the top part of the rdx is 0, so +64 would work just fine. But adding -64 requires the top part of the rdx register to hold 0xFFFFFFFF. Adding &data+0xffffffc0 in 64bit mode produces an invalid address and crashes.

The problem is fixed by changing the assignment (adding (int) cast):

       data[i * 4 + 0 - (int)kBlockSize] = (Byte)(d);
       data[i * 4 + 1 - (int)kBlockSize] = (Byte)(d >>  8);
       data[i * 4 + 2 - (int)kBlockSize] = (Byte)(d >> 16);
       data[i * 4 + 3 - (int)kBlockSize] = (Byte)(d >> 24);

My questions are:

UPDATE: It turn out, that this behavior doesn't depend on optimization flags and is only observed on Windows

Upvotes: 2

Views: 238

Answers (2)

Ross Ridge
Ross Ridge

Reputation: 39581

It's standard required behaviour when int and unsigned int are 32 bits wide. The expression i * 4 + 0 - kBlockSize is evaluated as (i * 4 + 0) - kBlockSize. The type on the left side of the minus operator is int while on the right side it's unsigned int. These need to converted to a common type and standard C says that type is unsigned int. So during the first iteration of the loop this becomes (unsigned) 0 - 64U which is 0xffffffc0. If pointers were 32 bits then data[0xffffffc0] would "wrap around" and be the same as data[-64], but they're 64 bits so you get a crash instead.

Upvotes: 3

David Schwartz
David Schwartz

Reputation: 182763

   data[i * 4 + 0 - kBlockSize] = (Byte)(d);      // <<= DISASSEMBLED

This is wrong. When i is zero, the subtraction tries to produce a negative, unsigned number. There is no such thing.

Upvotes: 2

Related Questions