Skip to content

Commit 2e23f64

Browse files
committed
Remove volatile cast gyrations in strciphr.cpp (GH weidai11#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 weidai11#1231 and GH weidai11#1234. We are also not able to successfully cut-in Cryptogams AES on ARMv7, so it is now disabled. See GH weidai11#1236. Since the volatile casts were not a solution, we are backing it out along with associated comments.
1 parent d4b9fa1 commit 2e23f64

File tree

1 file changed

+1
-42
lines changed

1 file changed

+1
-42
lines changed

strciphr.cpp

+1-42
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,4 @@
1-
// strciphr.cpp - originally written and placed in the public domain by Wei Dai
2-
3-
// TODO: Figure out what is happening in ProcessData. The issue surfaced
4-
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
5-
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
6-
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
7-
// inString == outString leads to incorrect results. We think it relates
8-
// to aliasing violations because inString == outString.
9-
//
10-
// Also see https://github.com/weidai11/cryptopp/issues/683,
11-
// https://github.com/weidai11/cryptopp/issues/1010 and
12-
// https://github.com/weidai11/cryptopp/issues/1088.
1+
// strciphr.cpp - originally written and placed in the public domain by Wei Dai.
132

143
#include "pch.h"
154

@@ -76,17 +65,6 @@ void AdditiveCipherTemplate<S>::GenerateBlock(byte *outString, size_t length)
7665
}
7766
}
7867

79-
// TODO: Figure out what is happening in ProcessData. The issue surfaced
80-
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
81-
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
82-
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
83-
// inString == outString leads to incorrect results. We think it relates
84-
// to aliasing violations because inString == outString.
85-
//
86-
// Also see https://github.com/weidai11/cryptopp/issues/683,
87-
// https://github.com/weidai11/cryptopp/issues/1010 and
88-
// https://github.com/weidai11/cryptopp/issues/1088.
89-
9068
template <class S>
9169
void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inString, size_t length)
9270
{
@@ -121,10 +99,6 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
12199
KeystreamOperation operation = KeystreamOperation(flags);
122100
policy.OperateKeystream(operation, outString, inString, iterations);
123101

124-
// Try to tame the optimizer. This is GH #683, #1010, and #1088.
125-
volatile byte* unused = const_cast<volatile byte*>(outString);
126-
CRYPTOPP_UNUSED(unused);
127-
128102
inString = PtrAdd(inString, iterations * bytesPerIteration);
129103
outString = PtrAdd(outString, iterations * bytesPerIteration);
130104
length -= iterations * bytesPerIteration;
@@ -206,17 +180,6 @@ void CFB_CipherTemplate<BASE>::Resynchronize(const byte *iv, int length)
206180
m_leftOver = policy.GetBytesPerIteration();
207181
}
208182

209-
// TODO: Figure out what is happening in ProcessData. The issue surfaced
210-
// for CFB_CipherTemplate<BASE>::ProcessData when we cut-in Cryptogams
211-
// AES ARMv7 asm. Then again in AdditiveCipherTemplate<S>::ProcessData
212-
// for CTR mode with HIGHT, which is a 64-bit block cipher. In both cases,
213-
// inString == outString leads to incorrect results. We think it relates
214-
// to aliasing violations because inString == outString.
215-
//
216-
// Also see https://github.com/weidai11/cryptopp/issues/683,
217-
// https://github.com/weidai11/cryptopp/issues/1010 and
218-
// https://github.com/weidai11/cryptopp/issues/1088.
219-
220183
template <class BASE>
221184
void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString, size_t length)
222185
{
@@ -249,10 +212,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
249212
CipherDir cipherDir = GetCipherDir(*this);
250213
policy.Iterate(outString, inString, cipherDir, length / bytesPerIteration);
251214

252-
// Try to tame the optimizer. This is GH #683, #1010, and #1088.
253-
volatile byte* unused = const_cast<volatile byte*>(outString);
254-
CRYPTOPP_UNUSED(unused);
255-
256215
const size_t remainder = length % bytesPerIteration;
257216
inString = PtrAdd(inString, length - remainder);
258217
outString = PtrAdd(outString, length - remainder);

0 commit comments

Comments
 (0)