arne
/
cryptopp
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Avoid std::call_once (GH #707) 

This commit also favors init priorities over C++ dynamic initialization. After the std::call_once problems on Sparc and PowerPC I'm worried about problems with Dynamic Initialization and Destruction with Concurrency.
We also do away with supressing warnings and use CRYPTOPP_UNUSED instead.
pull/709/head
Jeffrey Walton 2018-08-21 12:53:10 -04:00
parent 6b93c284fe
commit 81f8c48faf
No known key found for this signature in database
GPG Key ID: B36AB348921B1838
2 changed files with 76 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
blake2.cpp
View File

@ -235,7 +235,7 @@ void BLAKE2_Base<word32, false>::UncheckedSetKey(const byte *key, unsigned int l
AlignedSecByteBlock temp(BLOCKSIZE); AlignedSecByteBlock temp(BLOCKSIZE);
memcpy_s(temp, BLOCKSIZE, key, length); memcpy_s(temp, BLOCKSIZE, key, length);
size_t rem = SaturatingSubtract(BLOCKSIZE, length); size_t rem = SaturatingSubtract((unsigned int)BLOCKSIZE, length);
if (rem) if (rem)
std::memset(temp+length, 0x00, rem); std::memset(temp+length, 0x00, rem);
@ -291,7 +291,7 @@ void BLAKE2_Base<word64, true>::UncheckedSetKey(const byte *key, unsigned int le
AlignedSecByteBlock temp(BLOCKSIZE); AlignedSecByteBlock temp(BLOCKSIZE);
memcpy_s(temp, BLOCKSIZE, key, length); memcpy_s(temp, BLOCKSIZE, key, length);
size_t rem = SaturatingSubtract(BLOCKSIZE, length); size_t rem = SaturatingSubtract((unsigned int)BLOCKSIZE, length);
if (rem) if (rem)
std::memset(temp+length, 0x00, rem); std::memset(temp+length, 0x00, rem);

140
integer.cpp
View File

@ -5,51 +5,33 @@
// pointers on some platforms, like X86 and X64. The function pointers select a fast multiply // pointers on some platforms, like X86 and X64. The function pointers select a fast multiply
// and addition based on the cpu. Second, it wants to create Integer::Zero(), Integer::One() // and addition based on the cpu. Second, it wants to create Integer::Zero(), Integer::One()
// and Integer::Two(). // and Integer::Two().
// The function pointers are initialized in the InitializeInteger class by calling // The function pointers are initialized in the InitializeInteger class by
// SetFunctionPointers(). The call to SetFunctionPointers() is guarded to run once. If C++11 // calling SetFunctionPointers(). The call to SetFunctionPointers() is
// dynamic initialization is available, then a standard run_once is used. Otherwise, and simple // guarded to run once using a double-checked pattern. We don't use C++
// flag is used. The flag suffers a race, but the worse case is the same function pointers // std::call_once due to bad interactions between libstdc++, glibc and
// get written twice without leaking memory. // pthreads. Since they are only function pointers we don't have to worry
// For Integer::Zero(), Integer::One() and Integer::Two(), we use one of two strategies. First, // about leaking memory. The worst case seems to be the pointer gets written
// if C++11 dynamic initialization is available, then we use a static variable. Second, if // twice.
// C++11 dynamic initialization is not available, then we fall back to Wei's original code of // For Integer::Zero(), Integer::One() and Integer::Two(), we use one of two
// a Singleton. // strategies. First, if C++11 dynamic initialization is available, then we
// Wei's original code was much simpler. It simply used the Singleton pattern, but it always // use a static variable. Second, if C++11 dynamic initialization is not
// produced memory findings on some platforms. The Singleton generates memory findings because // available, then we fall back to Wei's original code of a Singleton.
// it uses a Create On First Use pattern (a dumb Nifty Counter) and the compiler had to be smart // Wei's original code was much simpler. It simply used the Singleton pattern,
// enough to fold them to return the same object. Unix and Linux compilers do a good job of folding // but it always produced memory findings on some platforms. The Singleton
// objects, but Microsoft compilers do a rather poor job for some versions of the compilers. // generates memory findings because it uses a Create On First Use pattern
// Another problem with the Singleton is resource destruction requires running resource acquisition // (a dumb Nifty Counter) and the compiler had to be smart enough to fold
// in reverse. For resources provided through the Singletons, there is no way to express the // them to return the same object. Unix and Linux compilers do a good job of
// dependency order to safely destroy resources. (That's one of the problems C++11 dynamic // folding objects, but Microsoft compilers do a rather poor job for some
// versions of the compilers.
// Another problem with the Singleton is resource destruction requires running
// resource acquisition in reverse. For resources provided through the
// Singletons, there is no way to express the dependency order to safely
// destroy resources. (That's one of the problems C++11 dynamic00
// intitialization with concurrent execution is supposed to solve). // intitialization with concurrent execution is supposed to solve).
#include "pch.h" #include "pch.h"
#include "config.h" #include "config.h"
#if CRYPTOPP_MSC_VERSION
# pragma warning(disable: 4100)
#endif
#if CRYPTOPP_GCC_DIAGNOSTIC_AVAILABLE
# pragma GCC diagnostic ignored "-Wunused"
#if !defined(__clang__)
# pragma GCC diagnostic ignored "-Wunused-but-set-variable"
#endif
#endif
// Issue 340
#if CRYPTOPP_GCC_DIAGNOSTIC_AVAILABLE
# pragma GCC diagnostic ignored "-Wconversion"
# pragma GCC diagnostic ignored "-Wsign-conversion"
#endif
// Define this to statically initialize Integer Zero(), One()
// and Two() using Microsoft init_seg(). This is useful for
// testing Integer code for leaks when the MSC compiler
// does not fold use of the Singletons.
// #define USE_MSC_INIT_PRIORITY 1
#ifndef CRYPTOPP_IMPORTS #ifndef CRYPTOPP_IMPORTS
#include "integer.h" #include "integer.h"
@ -102,16 +84,20 @@
// ***************** C++ Static Initialization ******************** // ***************** C++ Static Initialization ********************
NAMESPACE_BEGIN(CryptoPP) NAMESPACE_BEGIN(CryptoPP)
// Function body near the middle of the file
static void SetFunctionPointers(); static void SetFunctionPointers();
// We used to jump through some hoops to set the function pointers once,
// including std::call_once. Later we learned std::call_once was fairly
// buggy due to interactions between libstdc++, glibc and pthreads.
// Now we use a double-checked pattern. We are not leaking anything so
// it does not matter if a pointer is written twice during a race.
// Also see GCC Issue 66146, "call_once not C++11-compliant" (on many
// platforms), http://gcc.gnu.org/bugzilla/show_bug.cgi?id=66146 and
// http://github.com/weidai11/cryptopp/issues/707.
InitializeInteger::InitializeInteger() InitializeInteger::InitializeInteger()
{ {
#if !(HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY)
#if defined(CRYPTOPP_CXX11_SYNCHRONIZATION) && defined(CRYPTOPP_CXX11_DYNAMIC_INIT)
static std::once_flag s_flag;
std::call_once(s_flag, []() {
SetFunctionPointers();
});
#else
static bool s_flag; static bool s_flag;
MEMORY_BARRIER(); MEMORY_BARRIER();
if (s_flag == false) if (s_flag == false)
@ -120,8 +106,6 @@ InitializeInteger::InitializeInteger()
s_flag = true; s_flag = true;
MEMORY_BARRIER(); MEMORY_BARRIER();
} }
#endif // C++11 or C++03 flag
#endif // not GCC and MSC init priorities
} }
template <long i> template <long i>
@ -195,6 +179,7 @@ static word AtomicInverseModPower2(word A)
// ******************************************************** // ********************************************************
#if !defined(CRYPTOPP_NATIVE_DWORD_AVAILABLE) || (defined(__x86_64__) && defined(CRYPTOPP_WORD128_AVAILABLE)) #if !defined(CRYPTOPP_NATIVE_DWORD_AVAILABLE) || (defined(__x86_64__) && defined(CRYPTOPP_WORD128_AVAILABLE))
#define TWO_64_BIT_WORDS 1
#define Declare2Words(x) word x##0, x##1; #define Declare2Words(x) word x##0, x##1;
#define AssignWord(a, b) a##0 = b; a##1 = 0; #define AssignWord(a, b) a##0 = b; a##1 = 0;
#define Add2WordsBy1(a, b, c) a##0 = b##0 + c; a##1 = b##1 + (a##0 < c); #define Add2WordsBy1(a, b, c) a##0 = b##0 + c; a##1 = b##1 + (a##0 < c);
@ -744,6 +729,10 @@ CRYPTOPP_NAKED int CRYPTOPP_FASTCALL Baseline_Add(size_t N, word *C, const word
AS1( setc al) // store carry into eax (return result register) AS1( setc al) // store carry into eax (return result register)
AddEpilogue AddEpilogue
// http://github.com/weidai11/cryptopp/issues/340
CRYPTOPP_UNUSED(A); CRYPTOPP_UNUSED(B);
CRYPTOPP_UNUSED(C); CRYPTOPP_UNUSED(N);
} }
CRYPTOPP_NAKED int CRYPTOPP_FASTCALL Baseline_Sub(size_t N, word *C, const word *A, const word *B) CRYPTOPP_NAKED int CRYPTOPP_FASTCALL Baseline_Sub(size_t N, word *C, const word *A, const word *B)
@ -785,6 +774,10 @@ CRYPTOPP_NAKED int CRYPTOPP_FASTCALL Baseline_Sub(size_t N, word *C, const word
AS1( setc al) // store carry into eax (return result register) AS1( setc al) // store carry into eax (return result register)
AddEpilogue AddEpilogue
// http://github.com/weidai11/cryptopp/issues/340
CRYPTOPP_UNUSED(A); CRYPTOPP_UNUSED(B);
CRYPTOPP_UNUSED(C); CRYPTOPP_UNUSED(N);
} }
#if CRYPTOPP_INTEGER_SSE2 #if CRYPTOPP_INTEGER_SSE2
@ -843,6 +836,10 @@ CRYPTOPP_NAKED int CRYPTOPP_FASTCALL SSE2_Add(size_t N, word *C, const word *A,
AS1( emms) AS1( emms)
AddEpilogue AddEpilogue
// http://github.com/weidai11/cryptopp/issues/340
CRYPTOPP_UNUSED(A); CRYPTOPP_UNUSED(B);
CRYPTOPP_UNUSED(C); CRYPTOPP_UNUSED(N);
} }
CRYPTOPP_NAKED int CRYPTOPP_FASTCALL SSE2_Sub(size_t N, word *C, const word *A, const word *B) CRYPTOPP_NAKED int CRYPTOPP_FASTCALL SSE2_Sub(size_t N, word *C, const word *A, const word *B)
{ {
@ -899,6 +896,10 @@ CRYPTOPP_NAKED int CRYPTOPP_FASTCALL SSE2_Sub(size_t N, word *C, const word *A,
AS1( emms) AS1( emms)
AddEpilogue AddEpilogue
// http://github.com/weidai11/cryptopp/issues/340
CRYPTOPP_UNUSED(A); CRYPTOPP_UNUSED(B);
CRYPTOPP_UNUSED(C); CRYPTOPP_UNUSED(N);
} }
#endif // CRYPTOPP_INTEGER_SSE2 #endif // CRYPTOPP_INTEGER_SSE2
#else // CRYPTOPP_SSE2_ASM_AVAILABLE #else // CRYPTOPP_SSE2_ASM_AVAILABLE
@ -1295,6 +1296,11 @@ void Baseline_MultiplyBottom2(word *R, const word *AA, const word *BB)
MAYBE_CONST word* B = MAYBE_UNCONST_CAST(BB); MAYBE_CONST word* B = MAYBE_UNCONST_CAST(BB);
Bot_2 Bot_2
// http://github.com/weidai11/cryptopp/issues/340
#if defined(TWO_64_BIT_WORDS)
CRYPTOPP_UNUSED(d0); CRYPTOPP_UNUSED(d1);
#endif
} }
void Baseline_MultiplyBottom4(word *R, const word *AA, const word *BB) void Baseline_MultiplyBottom4(word *R, const word *AA, const word *BB)
@ -4787,21 +4793,23 @@ public:
} }
}; };
// This is not really needed because each Integer can dynamically initialize itself, // This is not really needed because each Integer can dynamically initialize
// but we take a peephole optimization and initialize the class once if init priorities are // itself, but we take a peephole optimization and initialize the class once
// available. Dynamic initialization will be used if init priorities are not available. // if init priorities are available. Dynamic initialization will be used if
// init priorities are not available.
#if HAVE_GCC_INIT_PRIORITY #if HAVE_GCC_INIT_PRIORITY
const InitInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 10))) = InitInteger(); const InitInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 10))) = InitInteger();
const Integer g_zero __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 11))) = Integer(0L);
const Integer g_one __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 12))) = Integer(1L);
const Integer g_two __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 13))) = Integer(2L);
#elif defined(HAVE_MSC_INIT_PRIORITY) #elif defined(HAVE_MSC_INIT_PRIORITY)
#pragma warning(disable: 4075) #pragma warning(disable: 4075)
#pragma init_seg(".CRT$XCU") #pragma init_seg(".CRT$XCU")
const InitInteger s_init; const InitInteger s_init;
# if defined(USE_MSC_INIT_PRIORITY)
const Integer g_zero(0L); const Integer g_zero(0L);
const Integer g_one(1L); const Integer g_one(1L);
const Integer g_two(2L); const Integer g_two(2L);
# endif
#pragma warning(default: 4075) #pragma warning(default: 4075)
#else #else
const InitInteger s_init; const InitInteger s_init;
@ -4811,36 +4819,36 @@ public:
const Integer &Integer::Zero() const Integer &Integer::Zero()
{ {
#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) #if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY)
return g_zero;
#elif defined(CRYPTOPP_CXX11_DYNAMIC_INIT)
static Integer s_zero(0L); static Integer s_zero(0L);
return s_zero; return s_zero;
#elif defined(HAVE_MSC_INIT_PRIORITY) && defined(USE_MSC_INIT_PRIORITY) #else // Potential memory leak. Avoid if possible.
return g_zero;
#else
return Singleton<Integer, NewInteger<0L> >().Ref(); return Singleton<Integer, NewInteger<0L> >().Ref();
#endif #endif
} }
const Integer &Integer::One() const Integer &Integer::One()
{ {
#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) #if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY)
return g_one;
#elif defined(CRYPTOPP_CXX11_DYNAMIC_INIT)
static Integer s_one(1L); static Integer s_one(1L);
return s_one; return s_one;
#elif defined(HAVE_MSC_INIT_PRIORITY) && defined(USE_MSC_INIT_PRIORITY) #else // Potential memory leak. Avoid if possible.
return g_one;
#else
return Singleton<Integer, NewInteger<1L> >().Ref(); return Singleton<Integer, NewInteger<1L> >().Ref();
#endif #endif
} }
const Integer &Integer::Two() const Integer &Integer::Two()
{ {
#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) #if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY)
return g_two;
#elif defined(CRYPTOPP_CXX11_DYNAMIC_INIT)
static Integer s_two(2L); static Integer s_two(2L);
return s_two; return s_two;
#elif defined(HAVE_MSC_INIT_PRIORITY) && defined(USE_MSC_INIT_PRIORITY) #else // Potential memory leak. Avoid if possible.
return g_two;
#else
return Singleton<Integer, NewInteger<2L> >().Ref(); return Singleton<Integer, NewInteger<2L> >().Ref();
#endif #endif
} }