Skip to content

Commit

Permalink
Try fix ProcessData in CFB_CipherTemplate and AdditiveCipherTemplate
Browse files Browse the repository at this point in the history
Also see GH #683, GH #1010, GH #1088, GH #1103
  • Loading branch information
noloader committed Feb 14, 2022
1 parent 5baf6c5 commit 0fccd24
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 64 deletions.
2 changes: 1 addition & 1 deletion arm_simd.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,9 @@ inline uint64x2_t VEXT_U8(uint64x2_t a, uint64x2_t b)
:"=w" (r) : "w" (a), "w" (b), "I" (C) );
return r;
#endif
//@}
}

//@}
#endif // CRYPTOPP_ARM_PMULL_AVAILABLE

#if CRYPTOPP_ARM_SHA3_AVAILABLE || defined(CRYPTOPP_DOXYGEN_PROCESSING)
Expand Down
1 change: 1 addition & 0 deletions config_asm.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@
# define CRYPTOPP_ARM_ASIMD_AVAILABLE 1
# endif // Compilers
# endif // Platforms

#endif

// ARMv8 and ASIMD. -march=armv8-a+crc or above must be present
Expand Down
4 changes: 2 additions & 2 deletions misc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
# include <immintrin.h>
# endif

# if defined(__aarch64__) || defined(__aarch32__) || defined(_M_ARM64)
# if defined(CRYPTOPP_ARM_NEON_HEADER)
# if defined(__aarch32__) || defined(__aarch64__) || defined(_M_ARM64)
# if (CRYPTOPP_ARM_NEON_HEADER) || (CRYPTOPP_ARM_ASIMD_AVAILABLE)
# include <arm_neon.h>
# endif
# endif
Expand Down
70 changes: 14 additions & 56 deletions strciphr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand All @@ -276,13 +237,9 @@ 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 word32 alignment = policy.GetAlignment();
const bool inAligned = IsAlignedOn(inString, alignment);
const bool outAligned = IsAlignedOn(outString, alignment);
CRYPTOPP_UNUSED(inAligned); CRYPTOPP_UNUSED(outAligned);
Expand All @@ -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);
Expand All @@ -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>
Expand Down
5 changes: 1 addition & 4 deletions strciphr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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;
};

Expand Down
2 changes: 1 addition & 1 deletion xts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#endif

#if defined(__aarch32__) || defined(__aarch64__) || defined(_M_ARM64)
# if (CRYPTOPP_ARM_NEON_HEADER)
# if (CRYPTOPP_ARM_NEON_HEADER) || (CRYPTOPP_ARM_ASIMD_AVAILABLE)
# include <arm_neon.h>
# endif
#endif
Expand Down

0 comments on commit 0fccd24

Please sign in to comment.