From afbd3e60f68ff8d9ae1b90d9c3c4eb576f518dbd Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Thu, 23 Aug 2018 14:42:29 -0400 Subject: [PATCH] Fix alignment on Win32 and Solaris Sparc (PR #709) These fixes were interesting in a morbid sort of way. I thought the FixedSizeAllocatorWithCleanup specializations faithfully reproduced semantics but I was wrong on Win32 and Sparc. Also see Commit e054d36dc88d. It seems there was another requirement or dependency that we missed, but it was not readily apparent. If I am parsing results correctly (which I may not be), it appears the bit twiddling using 8 byte alignment had more influence on alignment than I originally thought based on use of CRYPTOPP_BOOL_ALIGN16 and T_Align16. Or maybe the alignment attributes specified by CRYPTOPP_ALIGN_DATA are not being honored like they should for stack allocations. This check-in avoids some uses of x86 movdqa (aligned) in favor of movdqu (unaligned). The uses were concentrated on memory operands which were 8-byte aligned instead of 16-byte aligned. It is not clear to me how the specializations lost 8-bytes of alignment. The check-in also enlists CRYPTOPP_ASSERT to tell us when there's a problem so we don't need to go hunting for bugs. --- iterhash.h | 4 ++-- misc.h | 8 ++++---- panama.cpp | 56 ++++++++++++++++++++++++++------------------------- salsa.cpp | 50 ++++++++++++++++++++++++--------------------- secblock.h | 11 ++++++++-- sha.cpp | 5 +++-- sosemanuk.cpp | 24 +++++++++++----------- 7 files changed, 86 insertions(+), 72 deletions(-) diff --git a/iterhash.h b/iterhash.h index edaa18e2..f642ab47 100644 --- a/iterhash.h +++ b/iterhash.h @@ -152,8 +152,8 @@ public: { CRYPTOPP_ASSERT(in != NULLPTR); CRYPTOPP_ASSERT(out != NULLPTR); - CRYPTOPP_ASSERT(IsAligned(in)); - CRYPTOPP_ASSERT(IsAligned(out)); + CRYPTOPP_ASSERT(IsAligned(in)); + CRYPTOPP_ASSERT(IsAligned(out)); ConditionalByteReverse(T_Endianness::ToEnum(), out, in, byteCount); } diff --git a/misc.h b/misc.h index 377f9fd7..36101e7d 100644 --- a/misc.h +++ b/misc.h @@ -2043,7 +2043,7 @@ inline T ConditionalByteReverse(ByteOrder order, T value) /// not part of a full element. If T is int (and int is 4 bytes), then /// byteCount = 10 means only the first 2 elements or 8 bytes are /// reversed. -/// \details The follwoing program should help illustrate the behavior. +/// \details The following program should help illustrate the behavior. /// 
vector v1, v2;
 ///
 /// v1.push_back(1);
@@ -2063,7 +2063,7 @@ inline T ConditionalByteReverse(ByteOrder order, T value)
 /// for(unsigned int i = 0; i < v2.size(); i++)
 ///   cout << std::hex << v2[i] << " ";
 /// cout << endl;
-/// The program above results in the follwoing output. +/// The program above results in the following output. /// 
V1: 00000001 00000002 00000003 00000004
 /// V2: 01000000 02000000 03000000 04000000
/// \sa ConditionalByteReverse @@ -2072,8 +2072,8 @@ void ByteReverse(T *out, const T *in, size_t byteCount) { // Alignment check due to Issues 690 CRYPTOPP_ASSERT(byteCount % sizeof(T) == 0); - CRYPTOPP_ASSERT(IsAligned(in)); - CRYPTOPP_ASSERT(IsAligned(out)); + //CRYPTOPP_ASSERT(IsAligned(in)); + //CRYPTOPP_ASSERT(IsAligned(out)); size_t count = byteCount/sizeof(T); for (size_t i=0; i