merimax
asked on
problems moving bit shifting from 32bit to 64bit linux
I have some code that works in 32 bit linux but when it is moved to 64 bit linux it core dumps. Here is the offending code...
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc )(unsigned long *))
{
unsigned long uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | (size_t)p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&( uintTemp[0 ]));
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0] >> 24) & 0xff);
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
The p_ptrIn and p_ptrOut arrays are the ones that are getting overrun when I run in 64 bit but I don't understand why as they are not getting modified.
I am obviously missing something here. Increasing the size of uintTemp did not solve the problem.
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc
{
unsigned long uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | (size_t)p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&(
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0]
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
The p_ptrIn and p_ptrOut arrays are the ones that are getting overrun when I run in 64 bit but I don't understand why as they are not getting modified.
I am obviously missing something here. Increasing the size of uintTemp did not solve the problem.
Have you recompiled your code in the 64 machine or did you just run your 32 compiled binary?
Instead of using unsigned long, try using use fixed width integer types (like uint32_t) everywhere where the code relies on a given width.
ASKER
I tried changing the mask to uint32_t (see below) but it still core dumps on
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
Obviously I am missing something basic here. p_ptrIn[0] is a character so it is 8 bits wide. It is getting shifted over 24 bits into the 64 bit of the original uintTemp[0] (before I changed to uing32_t) . How can I be getting overflow on p_ptrIn?
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc )(unsigned long *))
{
uint32_t uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | (size_t)p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&( uintTemp[0 ]));
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0] >> 24) & 0xff);
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
Obviously I am missing something basic here. p_ptrIn[0] is a character so it is 8 bits wide. It is getting shifted over 24 bits into the 64 bit of the original uintTemp[0] (before I changed to uing32_t) . How can I be getting overflow on p_ptrIn?
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc
{
uint32_t uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | (size_t)p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&(
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0]
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
>> void (Blowfish::*p_ptrCryptFunc )(unsigned long *))
I still see unsigned long being used there ;)
I still see unsigned long being used there ;)
ASKER
Yes, but even if I comment out that function call, I still core dump on p_ptrIn being out of bounds.
Then the problem is not caused in the code you showed. So, you'll have to show a bit more code (like the code that calls this function).
ASKER
Here is where it is getting called...
// Make sure the number of bytes is a multiple of 8 if we are decrypting
//
if ( p_blnEncrypt == false && *p_ptrLength % 8 != 0 )
return false;
Cipher(p_ptrKey, strlen(p_ptrKey));
void (Blowfish::*ftnCryptFunc)( unsigned long *) = ( p_blnEncrypt )
? &Blowfish::Encipher
: &Blowfish::Decipher;
unsigned int i;
unsigned char uchrDataIn[8];
unsigned char uchrDataOut[8];
unsigned int uintBytes = (*p_ptrLength) - 8;
for (i=0; i<uintBytes; i+=8)
{
if ( p_ptrIn->Read(&uchrDataIn, 8) == false )
return false;
Crypt(uchrDataIn,
uchrDataOut,
ftnCryptFunc);
if ( p_ptrOut->Write(&uchrDataO ut, 8) == false )
return false;
}
// Make sure the number of bytes is a multiple of 8 if we are decrypting
//
if ( p_blnEncrypt == false && *p_ptrLength % 8 != 0 )
return false;
Cipher(p_ptrKey, strlen(p_ptrKey));
void (Blowfish::*ftnCryptFunc)(
? &Blowfish::Encipher
: &Blowfish::Decipher;
unsigned int i;
unsigned char uchrDataIn[8];
unsigned char uchrDataOut[8];
unsigned int uintBytes = (*p_ptrLength) - 8;
for (i=0; i<uintBytes; i+=8)
{
if ( p_ptrIn->Read(&uchrDataIn,
return false;
Crypt(uchrDataIn,
uchrDataOut,
ftnCryptFunc);
if ( p_ptrOut->Write(&uchrDataO
return false;
}
What does this :
>> unsigned int uintBytes = (*p_ptrLength) - 8;
give you ?
Or in other words, how is p_ptrLength defined, and what value does *p_ptrLength hold ?
>> unsigned int uintBytes = (*p_ptrLength) - 8;
give you ?
Or in other words, how is p_ptrLength defined, and what value does *p_ptrLength hold ?
ASKER
Ok....
We have...
unsigned int uintQueryLength ;
which gets passed to the below function...
clsCrypt.Crypt("Transfer.x ml",
(unsigned char *)(p_ptrData->ptrQueryText ),
&uintQueryLength,
false) == false )
which calls the following...
bool
Blowfish::
Crypt(const char * p_ptrKey,
UTIL::Input * p_ptrIn,
UTIL::Output * p_ptrOut,
unsigned int * p_ptrLength,
const bool p_blnEncrypt)
{
//
// Make sure the number of bytes is a multiple of 8 if we are decrypting
//
if ( p_blnEncrypt == false && *p_ptrLength % 8 != 0 )
return false;
Cipher(p_ptrKey, strlen(p_ptrKey));
void (Blowfish::*ftnCryptFunc)( unsigned long *) = ( p_blnEncrypt )
? &Blowfish::Encipher
: &Blowfish::Decipher;
unsigned int i;
unsigned char uchrDataIn[8];
unsigned char uchrDataOut[8];
unsigned int uintBytes = (*p_ptrLength) - 8;
for (i=0; i<uintBytes; i+=8)
{
if ( p_ptrIn->Read(&uchrDataIn, 8) == false )
return false;
Crypt(uchrDataIn,
uchrDataOut,
ftnCryptFunc);
if ( p_ptrOut->Write(&uchrDataO ut, 8) == false )
return false;
}
which then core dumps when it gets to below code because p_ptrIn and p_ptrOut are out of bounds...
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc )(unsigned long *))
{
uint32_t uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&( uintTemp[0 ]));
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0] >> 24) & 0xff);
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
We have...
unsigned int uintQueryLength ;
which gets passed to the below function...
clsCrypt.Crypt("Transfer.x
(unsigned char *)(p_ptrData->ptrQueryText
&uintQueryLength,
false) == false )
which calls the following...
bool
Blowfish::
Crypt(const char * p_ptrKey,
UTIL::Input * p_ptrIn,
UTIL::Output * p_ptrOut,
unsigned int * p_ptrLength,
const bool p_blnEncrypt)
{
//
// Make sure the number of bytes is a multiple of 8 if we are decrypting
//
if ( p_blnEncrypt == false && *p_ptrLength % 8 != 0 )
return false;
Cipher(p_ptrKey, strlen(p_ptrKey));
void (Blowfish::*ftnCryptFunc)(
? &Blowfish::Encipher
: &Blowfish::Decipher;
unsigned int i;
unsigned char uchrDataIn[8];
unsigned char uchrDataOut[8];
unsigned int uintBytes = (*p_ptrLength) - 8;
for (i=0; i<uintBytes; i+=8)
{
if ( p_ptrIn->Read(&uchrDataIn,
return false;
Crypt(uchrDataIn,
uchrDataOut,
ftnCryptFunc);
if ( p_ptrOut->Write(&uchrDataO
return false;
}
which then core dumps when it gets to below code because p_ptrIn and p_ptrOut are out of bounds...
void
Blowfish::
Crypt(const unsigned char * p_ptrIn,
unsigned char * p_ptrOut,
void (Blowfish::*p_ptrCryptFunc
{
uint32_t uintTemp[2];
uintTemp[0] = (p_ptrIn[0] << 24) | (p_ptrIn[1] << 16) | (p_ptrIn[2] << 8) | p_ptrIn[3];
uintTemp[1] = (p_ptrIn[4] << 24) | (p_ptrIn[5] << 16) | (p_ptrIn[6] << 8) | p_ptrIn[7];
(*this.*p_ptrCryptFunc)(&(
p_ptrOut[0] = (unsigned char)((size_t)(uintTemp[0]
p_ptrOut[1] = (unsigned char)((uintTemp[0] >> 16) & 0xff);
p_ptrOut[2] = (unsigned char)((uintTemp[0] >> 8) & 0xff);
p_ptrOut[3] = (unsigned char)(uintTemp[0] & 0xff);
p_ptrOut[4] = (unsigned char)((uintTemp[1] >> 24) & 0xff);
p_ptrOut[5] = (unsigned char)((uintTemp[1] >> 16) & 0xff);
p_ptrOut[6] = (unsigned char)((uintTemp[1] >> 8) & 0xff);
p_ptrOut[7] = (unsigned char)(uintTemp[1] & 0xff);
}
>> unsigned int uintQueryLength ;
So what value does it have ?
So what value does it have ?
ASKER
It just does a strlen() on the query text to get the value.
Ok, it seems that wandering through the code like this won't give us an answer soon.
Why don't you run the code in the debugger, and it'll point you to exactly the line that has the issue, and will tell you the values that all variables have at that point.
Why don't you run the code in the debugger, and it'll point you to exactly the line that has the issue, and will tell you the values that all variables have at that point.
I don't think that (p_ptrIn[0] << 24) is a valid expression for all compilers if p_ptrIn[0] is a char (though i wouldn't have expected it would crash).
same applies for all other bit shift operations which were shifting beyond 8-bit char boundary.
can you try to use for bitshift a temporary uint32_t array where you copied the values of p_ptrIn array before?
Sara
same applies for all other bit shift operations which were shifting beyond 8-bit char boundary.
can you try to use for bitshift a temporary uint32_t array where you copied the values of p_ptrIn array before?
Sara
>> I don't think that (p_ptrIn[0] << 24) is a valid expression for all compilers if p_ptrIn[0] is a char
It's valid for all standards compliant compilers, due to integer promotion rules.
It's valid for all standards compliant compilers, due to integer promotion rules.
that means it uses an int temporary for the expression?
Sara
Sara
>> that means it uses an int temporary for the expression?
It means that the calculation is done after performing integer promotion, and in this case, it will promote p_ptrIn[0] to an int.
It means that the calculation is done after performing integer promotion, and in this case, it will promote p_ptrIn[0] to an int.
but the 'overrun of p_ptrIn' as stated by the OP would fit to a non-compliancy in that point?
Sara
Sara
>> but the 'overrun of p_ptrIn' as stated by the OP would fit to a non-compliancy in that point?
A non-compliant compiler can do anything it wants, so anything is possible. In other words : I couldn't say if a non-compliant compiler would cause the described behavior, because with non-compliant compilers, all bets are off. Standards are there to avoid these issues (and integer promotion is a pretty basic part of C) though.
A non-compliant compiler can do anything it wants, so anything is possible. In other words : I couldn't say if a non-compliant compiler would cause the described behavior, because with non-compliant compilers, all bets are off. Standards are there to avoid these issues (and integer promotion is a pretty basic part of C) though.
I compiled the following
char c, d = 'a';
c = (d << 24);
with vc9 and warning level 4 (the strongest).
it didn't give me a warning. shouldn't it have done so if integer promotion was done at the right side?
it gives a warning if i do
int i = 17;
c = i;
Sara
char c, d = 'a';
c = (d << 24);
with vc9 and warning level 4 (the strongest).
it didn't give me a warning. shouldn't it have done so if integer promotion was done at the right side?
it gives a warning if i do
int i = 17;
c = i;
Sara
>> shouldn't it have done so if integer promotion was done at the right side?
The standard doesn't require warnings here. It's up to the compiler in which cases it gives a warning.
A compiler with high warning levels enabled might give a warning for this case (due to the potential of a value change), or it might not. gcc eg. gives a warning for this with -Wconversion.
The standard doesn't require warnings here. It's up to the compiler in which cases it gives a warning.
A compiler with high warning levels enabled might give a warning for this case (due to the potential of a value change), or it might not. gcc eg. gives a warning for this with -Wconversion.
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Changing from 32bit to 64bit should have no impact on the choice of encoding between Base32 and Base64, because the two are not related.
Are you sure about your analysis ?
Are you sure about your analysis ?
>> It turns out the problem was with the algorithm that worked on the string not the string where the compiler was telling me the array was out of bounds.
I suspect that you actually meant that the algorithm was interpreting the data as 64bit words rather than 32bit words. And that would have been resolved by the suggested changes (among other things : use the correct Blowfish crypt function by using a fixed width integer type rather than unsigned long).
I suspect that you actually meant that the algorithm was interpreting the data as 64bit words rather than 32bit words. And that would have been resolved by the suggested changes (among other things : use the correct Blowfish crypt function by using a fixed width integer type rather than unsigned long).
ASKER
It turns out the problem was with the algorithm that worked on the string not the string where the compiler was telling me the array was out of bounds.