Increase use of ptrdiff_t when performing pointer math

This fixes the incorrect result with CFB mode on A-32 in CFB_CipherTemplate<BASE>::ProcessData

strciphr.cpp

@@ -59,8 +59,8 @@ void AdditiveCipherTemplate<S>::GenerateBlock(byte *outString, size_t length)
 		size_t bufferByteSize = RoundUpToMultipleOf(length, bytesPerIteration);
 		size_t bufferIterations = bufferByteSize / bytesPerIteration;
 
-		policy.WriteKeystream(KeystreamBufferEnd()-bufferByteSize, bufferIterations);
+		policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bufferByteSize), bufferIterations);
-		memcpy(outString, KeystreamBufferEnd()-bufferByteSize, length);
+		memcpy(outString, PtrSub(KeystreamBufferEnd(), bufferByteSize), length);
 		m_leftOver = bufferByteSize - length;
 	}
 }
@@ -71,7 +71,7 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
 	if (m_leftOver > 0)
 	{
 		const size_t len = STDMIN(m_leftOver, length);
-		xorbuf(outString, inString, KeystreamBufferEnd()-m_leftOver, len);
+		xorbuf(outString, inString, PtrSub(KeystreamBufferEnd(), m_leftOver), len);
 
 		length -= len; m_leftOver -= len;
 		inString = PtrAdd(inString, len);
@@ -115,8 +115,8 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
 		bufferByteSize = RoundUpToMultipleOf(length, bytesPerIteration);
 		bufferIterations = bufferByteSize / bytesPerIteration;
 
-		policy.WriteKeystream(KeystreamBufferEnd()-bufferByteSize, bufferIterations);
+		policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bufferByteSize), bufferIterations);
-		xorbuf(outString, inString, KeystreamBufferEnd()-bufferByteSize, length);
+		xorbuf(outString, inString, PtrSub(KeystreamBufferEnd(), bufferByteSize), length);
 		m_leftOver = bufferByteSize - length;
 	}
 }
@@ -141,7 +141,7 @@ void AdditiveCipherTemplate<BASE>::Seek(lword position)
 
 	if (position > 0)
 	{
-		policy.WriteKeystream(KeystreamBufferEnd()-bytesPerIteration, 1);
+		policy.WriteKeystream(PtrSub(KeystreamBufferEnd(), bytesPerIteration), 1);
 		m_leftOver = bytesPerIteration - (unsigned int)position;
 	}
 	else
@@ -195,16 +195,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 		if (!length) {return;}
 	}
 
-	// TODO: Figure out what is happening on ARM A-32. x86, Aarch64 and PowerPC are OK.
-	//       The issue surfaced for CFB mode when we cut-in Cryptogams AES ARMv7 asm.
-	//       Using 'outString' for both input and output leads to incorrect results.
-	//
-	//       Benchmarking on Cortex-A7 and Cortex-A9 indicates removing the block
-	//       below does not have a material effect on performance.
-	//
-	//       Also see https://github.com/weidai11/cryptopp/issues/683.
-	//
-#if !defined(__arm__)
 	if (policy.CanIterate() && length >= bytesPerIteration && IsAlignedOn(outString, alignment))
 	{
 		const CipherDir cipherDir = GetCipherDir(*this);
@@ -212,32 +202,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 			policy.Iterate(outString, inString, cipherDir, length / bytesPerIteration);
 		else
 		{
-			// GCC and Clang does not like this on ARM. The incorrect result is a string
-			// of 0's instead of ciphertext (or plaintext if decrypting). The 0's trace
-			// back to the allocation for the std::string in datatest.cpp. Elements in the
-			// string are initialized to their default value, which is 0.
-			//
-			// It almost feels as if the compiler does not see the string is transformed
-			// in-place so it short-circuits the transform. However, if we use a stand-alone
-			// reproducer with the same data then the issue is _not_ present.
-			//
-			// When working on this issue we introduced PtrAdd and PtrSub to ensure we were
-			// not running afoul of pointer arithmetic rules of the language. Namely we need
-			// to use ptrdiff_t when subtracting pointers. We believe the relevant code paths
-			// are clean.
-			//
-			// There are two remaining open questions. The first is aliasing rules. Char-types
-			// are not bound by aliasing rules so we are OK. The second is array const-ness.
-			// The arrays are created in datatest.cpp and they are non-const. Since the original
-			// objects are non-const we are OK casting const-ness away as buffers are twiddled.
-			//
-			// One workaround is a distinct and aligned temporary buffer. It [mostly] works
-			// as expected but requires an extra allocation (casts not shown):
-			//
-			//   std::string temp(length);
-			//   std::memcpy(&temp[0], inString, length);
-			//   policy.Iterate(outString, &temp[0], cipherDir, length / bytesPerIteration);
-			//
 			memcpy(outString, inString, length);
 			policy.Iterate(outString, outString, cipherDir, length / bytesPerIteration);
 		}
@@ -245,7 +209,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
 		outString = PtrAdd(outString, length - length % bytesPerIteration);
 		length %= bytesPerIteration;
 	}
-#endif
 
 	while (length >= bytesPerIteration)
 	{

strciphr.h

@@ -376,7 +376,7 @@ protected:
 	unsigned int GetBufferByteSize(const PolicyInterface &policy) const {return policy.GetBytesPerIteration() * policy.GetIterationsToBuffer();}
 
 	inline byte * KeystreamBufferBegin() {return this->m_buffer.data();}
-	inline byte * KeystreamBufferEnd() {return (this->m_buffer.data() + this->m_buffer.size());}
+	inline byte * KeystreamBufferEnd() {return (PtrAdd(this->m_buffer.data(), this->m_buffer.size()));}
 
 	SecByteBlock m_buffer;
 	size_t m_leftOver;