Remove volatile cast gyrations in strciphr.cpp (GH #1231)

It turns out we went down a rabbit hole when we added the volatile cast gyrations in an attempt to change the compiler behavior. We are seeing the same failures from AES, Rabbit, HIGHT, HC-128 and HC-256 with and without the gyrations.
We were able to work out the problems with Rabbit, HIGHT, HC-128 and HC-256. See GH #1231 and GH #1234.
We are also not able to successfully cut-in Cryptogams AES on ARMv7, so it is now disabled. See GH #1236.
Since the volatile casts were not a solution, we are backing it out along with associated comments.
This commit is contained in:
Jeffrey Walton 2023-09-29 15:40:14 -04:00
parent d4b9fa1165
commit 2e23f6482a
No known key found for this signature in database
GPG Key ID: B36AB348921B1838

View File

@ -1,15 +1,4 @@
// strciphr.cpp - originally written and placed in the public domain by Wei Dai
// TODO: Figure out what is happening in ProcessData. The issue surfaced
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
// inString == outString leads to incorrect results. We think it relates
// to aliasing violations because inString == outString.
//
// Also see https://github.com/weidai11/cryptopp/issues/683,
// https://github.com/weidai11/cryptopp/issues/1010 and
// https://github.com/weidai11/cryptopp/issues/1088.
// strciphr.cpp - originally written and placed in the public domain by Wei Dai.
#include "pch.h"
@ -76,17 +65,6 @@ void AdditiveCipherTemplate<S>::GenerateBlock(byte *outString, size_t length)
}
}
// TODO: Figure out what is happening in ProcessData. The issue surfaced
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
// inString == outString leads to incorrect results. We think it relates
// to aliasing violations because inString == outString.
//
// Also see https://github.com/weidai11/cryptopp/issues/683,
// https://github.com/weidai11/cryptopp/issues/1010 and
// https://github.com/weidai11/cryptopp/issues/1088.
template <class S>
void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inString, size_t length)
{
@ -121,10 +99,6 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
KeystreamOperation operation = KeystreamOperation(flags);
policy.OperateKeystream(operation, outString, inString, iterations);
// Try to tame the optimizer. This is GH #683, #1010, and #1088.
volatile byte* unused = const_cast<volatile byte*>(outString);
CRYPTOPP_UNUSED(unused);
inString = PtrAdd(inString, iterations * bytesPerIteration);
outString = PtrAdd(outString, iterations * bytesPerIteration);
length -= iterations * bytesPerIteration;
@ -206,17 +180,6 @@ void CFB_CipherTemplate<BASE>::Resynchronize(const byte *iv, int length)
m_leftOver = policy.GetBytesPerIteration();
}
// TODO: Figure out what is happening in ProcessData. The issue surfaced
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
// inString == outString leads to incorrect results. We think it relates
// to aliasing violations because inString == outString.
//
// Also see https://github.com/weidai11/cryptopp/issues/683,
// https://github.com/weidai11/cryptopp/issues/1010 and
// https://github.com/weidai11/cryptopp/issues/1088.
template <class BASE>
void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString, size_t length)
{
@ -249,10 +212,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
CipherDir cipherDir = GetCipherDir(*this);
policy.Iterate(outString, inString, cipherDir, length / bytesPerIteration);
// Try to tame the optimizer. This is GH #683, #1010, and #1088.
volatile byte* unused = const_cast<volatile byte*>(outString);
CRYPTOPP_UNUSED(unused);
const size_t remainder = length % bytesPerIteration;
inString = PtrAdd(inString, length - remainder);
outString = PtrAdd(outString, length - remainder);