Reputation: 8695
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
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
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