Try fix ProcessData in CFB_CipherTemplate and AdditiveCipherTemplate

Also see GH #683, GH #1010, GH #1088, GH #1103

strciphr.cpp

@@ -55,7 +55,7 @@ void AdditiveCipherTemplate<S>::GenerateBlock(byte *outString, size_t length)
 	}
 
 	PolicyInterface &policy = this->AccessPolicy();
-	unsigned int bytesPerIteration = policy.GetBytesPerIteration();
+	size_t bytesPerIteration = policy.GetBytesPerIteration();
 
 	if (length >= bytesPerIteration)
 	{
@@ -93,26 +93,8 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
 	CRYPTOPP_ASSERT(outString); CRYPTOPP_ASSERT(inString);
 	CRYPTOPP_ASSERT(length % this->MandatoryBlockSize() == 0);
 
-	// If this assert fires, then outString == inString. You could experience a
-	// performance hit. Also see https://github.com/weidai11/cryptopp/issues/1088'
-	CRYPTOPP_ASSERT(outString != inString);
-
 	PolicyInterface &policy = this->AccessPolicy();
-	unsigned int bytesPerIteration = policy.GetBytesPerIteration();
-
-	byte* savedOutString = outString;
-	size_t savedLength = length;
-	bool copyOut = false;
-
-	if (inString == outString)
-	{
-		// No need to copy inString to outString.
-		// Just allocate the space.
-		m_tempOutString.New(length);
-		m_tempOutString.SetMark(0);
-		outString = m_tempOutString.BytePtr();
-		copyOut = true;
-	}
+	size_t bytesPerIteration = policy.GetBytesPerIteration();
 
 	if (m_leftOver > 0)
 	{
@@ -124,13 +106,9 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
 		length -= len; m_leftOver -= len;
 	}
 
-	if (!length) {
-		if (copyOut)
-			std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
-		return;
-	}
+	if (!length) { return; }
 
-	const unsigned int alignment = policy.GetAlignment();
+	const word32 alignment = policy.GetAlignment();
 	const bool inAligned = IsAlignedOn(inString, alignment);
 	const bool outAligned = IsAlignedOn(outString, alignment);
 	CRYPTOPP_UNUSED(inAligned); CRYPTOPP_UNUSED(outAligned);
@@ -143,6 +121,10 @@ 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;
@@ -171,9 +153,6 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
 
 		m_leftOver = bufferByteSize - length;
 	}
-
-	if (copyOut)
-		std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
 }
 
 template <class S>
@@ -244,28 +223,10 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 	CRYPTOPP_ASSERT(outString); CRYPTOPP_ASSERT(inString);
 	CRYPTOPP_ASSERT(length % this->MandatoryBlockSize() == 0);
 
-	// If this assert fires, then outString == inString. You could experience a
-	// performance hit. Also see https://github.com/weidai11/cryptopp/issues/1088'
-	CRYPTOPP_ASSERT(outString != inString);
-
 	PolicyInterface &policy = this->AccessPolicy();
 	unsigned int bytesPerIteration = policy.GetBytesPerIteration();
 	byte *reg = policy.GetRegisterBegin();
 
-	byte* savedOutString = outString;
-	size_t savedLength = length;
-	bool copyOut = false;
-
-	if (inString == outString)
-	{
-		// No need to copy inString to outString.
-		// Just allocate the space.
-		m_tempOutString.New(length);
-		m_tempOutString.SetMark(0);
-		outString = m_tempOutString.BytePtr();
-		copyOut = true;
-	}
-
 	if (m_leftOver)
 	{
 		const size_t len = STDMIN(m_leftOver, length);
@@ -276,14 +237,10 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 		m_leftOver -= len; length -= len;
 	}
 
-	if (!length) {
-		if (copyOut)
-			std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
-		return;
-	}
+	if (!length) { return; }
 
-	const unsigned int alignment = policy.GetAlignment();
-	const bool inAligned = IsAlignedOn(inString, alignment);
+	const word32 alignment = policy.GetAlignment();
+	const bool inAligned = grep IsAlignedOn(inString, alignment);
 	const bool outAligned = IsAlignedOn(outString, alignment);
 	CRYPTOPP_UNUSED(inAligned); CRYPTOPP_UNUSED(outAligned);
 
@@ -292,6 +249,10 @@ 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);
@@ -314,9 +275,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 		CombineMessageAndShiftRegister(outString, reg, inString, length);
 		m_leftOver = bytesPerIteration - length;
 	}
-
-	if (copyOut)
-		std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
 }
 
 template <class BASE>

strciphr.h

@@ -394,8 +394,7 @@ class CRYPTOPP_NO_VTABLE AdditiveCipherTemplate : public BASE, public RandomNumb
 	inline byte * KeystreamBufferBegin() {return this->m_buffer.data();}
 	inline byte * KeystreamBufferEnd() {return (PtrAdd(this->m_buffer.data(), this->m_buffer.size()));}
 
-	// m_tempOutString added due to GH #1010
-	AlignedSecByteBlock m_buffer, m_tempOutString;
+	AlignedSecByteBlock m_buffer;
 	size_t m_leftOver;
 };
 
@@ -646,8 +645,6 @@ class CRYPTOPP_NO_VTABLE CFB_CipherTemplate : public BASE
 
 	void UncheckedSetKey(const byte *key, unsigned int length, const NameValuePairs &params);
 
-	// m_tempOutString added due to GH #1010
-	AlignedSecByteBlock m_tempOutString;
 	size_t m_leftOver;
 };