diff --git a/config.h b/config.h index 0636667a..05058a10 100644 --- a/config.h +++ b/config.h @@ -374,7 +374,7 @@ NAMESPACE_END // 4786: identifier was truncated in debug information // 4355: 'this' : used in base member initializer list // 4910: '__declspec(dllexport)' and 'extern' are incompatible on an explicit instantiation -# pragma warning(disable: 4127 4512 4661) +# pragma warning(disable: 4127 4512 4661 4910) // Security related, possible defects // http://blogs.msdn.com/b/vcblog/archive/2010/12/14/off-by-default-compiler-warnings-in-visual-c.aspx # pragma warning(once: 4191 4242 4263 4264 4266 4302 4826 4905 4906 4928) diff --git a/cpu.cpp b/cpu.cpp index edd0c192..5a7b5ca6 100644 --- a/cpu.cpp +++ b/cpu.cpp @@ -113,7 +113,6 @@ bool CpuId(word32 func, word32 subfunc, word32 output[4]) mov eax, func mov ecx, subfunc cpuid - mov edi, output mov [a], eax mov [b], ebx mov [c], ecx @@ -130,7 +129,9 @@ bool CpuId(word32 func, word32 subfunc, word32 output[4]) return false; } - // function 0 returns the highest basic function understood in EAX + // func = 0 returns the highest basic function understood in EAX. If the CPU does + // not return non-0, then it is mostly useless. The code below converts basic + // function value to a true/false return value. if(func == 0) return !!output[0]; @@ -748,9 +749,11 @@ NAMESPACE_END // *************************** C++ Static Initialization *************************** ANONYMOUS_NAMESPACE_BEGIN -struct InitializeCpu + +class InitCpu { - InitializeCpu() +public: + InitCpu() { #if CRYPTOPP_BOOL_X86 || CRYPTOPP_BOOL_X32 || CRYPTOPP_BOOL_X64 CryptoPP::DetectX86Features(); @@ -762,16 +765,21 @@ struct InitializeCpu } }; +// This is not really needed because HasSSE() and friends can dynamically initialize. +// Everything depends on CPU features so we initialize it once at load time. +// Dynamic initialization will be used if init priorities are not available. + #if HAVE_GCC_INIT_PRIORITY -const InitializeCpu s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 20))) = InitializeCpu(); + const InitCpu s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 10))) = InitCpu(); #elif HAVE_MSC_INIT_PRIORITY -#pragma warning(disable: 4075) -#pragma init_seg(".CRT$XCU-020") -const InitializeCpu s_init; -#pragma warning(default: 4075) + #pragma warning(disable: 4075) + #pragma init_seg(".CRT$XCU") + const InitCpu s_init; + #pragma warning(default: 4075) #else -const InitializeCpu& s_init = CryptoPP::Singleton().Ref(); + const InitCpu s_init; #endif + ANONYMOUS_NAMESPACE_END #endif // CRYPTOPP_IMPORTS diff --git a/cryptlib.cpp b/cryptlib.cpp index 3de5c02b..b6e91f07 100644 --- a/cryptlib.cpp +++ b/cryptlib.cpp @@ -42,6 +42,7 @@ CRYPTOPP_COMPILE_ASSERT(sizeof(word64) == 8); CRYPTOPP_COMPILE_ASSERT(sizeof(dword) == 2*sizeof(word)); #endif +#if 0 class NullNameValuePairs : public NameValuePairs { public: @@ -49,6 +50,7 @@ public: bool GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const {CRYPTOPP_UNUSED(name); CRYPTOPP_UNUSED(valueType); CRYPTOPP_UNUSED(pValue); return false;} }; +#endif BufferedTransformation & TheBitBucket() { @@ -945,27 +947,13 @@ int LibraryVersion(CRYPTOPP_NOINLINE_DOTDOTDOT) } // ***************** C++ Static Initialization ******************** -// We can't put these in the anonymous namespace. DEFAULT_CHANNEL, -// AAD_CHANNEL and g_nullNameValuePairs must be defined in CryptoPP. -#if HAVE_GCC_INIT_PRIORITY -const std::string DEFAULT_CHANNEL __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 10))) = ""; -const std::string AAD_CHANNEL __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 11))) = "AAD"; -const NullNameValuePairs s_nullNameValuePairs __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 12))); -const NameValuePairs &g_nullNameValuePairs = dynamic_cast(s_nullNameValuePairs); -#elif HAVE_MSC_INIT_PRIORITY -#pragma warning(disable: 4075) -#pragma init_seg(".CRT$XCU-010") -const std::string DEFAULT_CHANNEL(""); +#if 0 +const std::string DEFAULT_CHANNEL; const std::string AAD_CHANNEL("AAD"); -const NullNameValuePairs s_nullNameValuePairs; -const NameValuePairs &g_nullNameValuePairs = dynamic_cast(s_nullNameValuePairs); -#pragma warning(default: 4075) -#else -const std::string DEFAULT_CHANNEL = ""; -const std::string AAD_CHANNEL = "AAD"; -const simple_ptr s_pNullNameValuePairs(new NullNameValuePairs); -const NameValuePairs &g_nullNameValuePairs = *s_pNullNameValuePairs.m_p; + +NullNameValuePairs s_nullNameValuePairs; +const NameValuePairs&g_nullNameValuePairs = dynamic_cast(s_nullNameValuePairs); #endif NAMESPACE_END // CryptoPP diff --git a/cryptlib.h b/cryptlib.h index 37be7fea..1d9677a7 100644 --- a/cryptlib.h +++ b/cryptlib.h @@ -282,7 +282,9 @@ struct CRYPTOPP_DLL DecodingResult //! then look at the Name namespace documentation to see what the type of each value is, or //! alternatively, call GetIntValue() with the value name, and if the type is not int, a //! ValueTypeMismatch exception will be thrown and you can get the actual type from the exception object. -class CRYPTOPP_NO_VTABLE NameValuePairs +//! \sa NullNameValuePairs, g_nullNameValuePairs, +//! NameValuePairs on the Crypto++ wiki +class NameValuePairs { public: virtual ~NameValuePairs() {} @@ -445,8 +447,52 @@ public: CRYPTOPP_DLL virtual bool GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const =0; }; -#if CRYPTOPP_DOXYGEN_PROCESSING +//! \class NullNameValuePairs +//! \brief Interface for retrieving values given their names +//! \details This class is used when no names or values are present. Typically a program uses +//! g_nullNameValuePairs rather than creating its own NullNameValuePairs object. +//! \details NullNameValuePairs always existed in cryptlib.cpp. Crypto++ 6.0 moved NullNameValuePairs +//! into the header. This allowed the library to define g_nullNameValuePairs in the header rather +//! than declaring it as extern and placing the definition in the source file. As an external definition +//! the string g_nullNameValuePairs was subject to static initialization order fiasco problems. +//! \sa NameValuePairs, g_nullNameValuePairs, +//! NameValuePairs on the Crypto++ wiki +class NullNameValuePairs : public NameValuePairs +{ +public: + NullNameValuePairs() {} // Clang complains a default ctor must be avilable + bool GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const + {CRYPTOPP_UNUSED(name); CRYPTOPP_UNUSED(valueType); CRYPTOPP_UNUSED(pValue); return false;} +}; +// More static initialization order fiasco workarounds. These definitions cannot be extern and +// cannot be static class members because they require a single definition in a source file. +ANONYMOUS_NAMESPACE_BEGIN +const NullNameValuePairs s_nullNameValuePairs; +ANONYMOUS_NAMESPACE_END + +//! \brief Default channel for BufferedTransformation +//! \details DEFAULT_CHANNEL is equal to an empty string +//! \details Crypto++ 6.0 placed DEFAULT_CHANNEL in the header, rather than declaring it as extern and +//! placing the definition in the source file. As an external definition the string DEFAULT_CHANNEL +//! was subject to static initialization order fiasco problems. +static const std::string DEFAULT_CHANNEL; + +//! \brief Channel for additional authenticated data +//! \details AAD_CHANNEL is equal to "AAD" +//! \details Crypto++ 6.0 placed AAD_CHANNEL in the header, rather than declaring it as extern and +//! placing the definition in the source file. As an external definition the string AAD_CHANNEL +//! was subject to static initialization order fiasco problems. +static const std::string AAD_CHANNEL("AAD"); + +//! \brief An empty set of name-value pairs +//! \details Crypto++ 6.0 placed g_nullNameValuePairs in the header, rather than declaring it as extern +//! and placing the definition in the source file. As an external definition the g_nullNameValuePairs +//! was subject to static initialization order fiasco problems. +static const NameValuePairs& g_nullNameValuePairs = s_nullNameValuePairs; + +// Document additional name spaces which show up elsewhere in the sources. +#if CRYPTOPP_DOXYGEN_PROCESSING //! \brief Namespace containing value name definitions. //! \details Name is part of the CryptoPP namespace. //! \details The semantics of value names, types are: @@ -470,7 +516,6 @@ DOCUMENTED_NAMESPACE_END //! ... //! CryptoPP::Weak::MD5 md5; //! - DOCUMENTED_NAMESPACE_BEGIN(Weak) // weak and wounded algorithms DOCUMENTED_NAMESPACE_END @@ -483,9 +528,6 @@ DOCUMENTED_NAMESPACE_BEGIN(Test) // testing and benchmark classes DOCUMENTED_NAMESPACE_END -//! \brief An empty set of name-value pairs -extern CRYPTOPP_DLL const NameValuePairs &g_nullNameValuePairs; - // ******************************************************** //! \class Clonable @@ -1388,6 +1430,7 @@ public: bool Wait(unsigned long milliseconds, CallStack const& callStack); }; +#if 0 //! \brief Default channel for BufferedTransformation //! \details DEFAULT_CHANNEL is equal to an empty string extern CRYPTOPP_DLL const std::string DEFAULT_CHANNEL; @@ -1395,6 +1438,7 @@ extern CRYPTOPP_DLL const std::string DEFAULT_CHANNEL; //! \brief Channel for additional authenticated data //! \details AAD_CHANNEL is equal to "AAD" extern CRYPTOPP_DLL const std::string AAD_CHANNEL; +#endif //! \brief Interface for buffered transformations //! \details BufferedTransformation is a generalization of BlockTransformation, diff --git a/integer.cpp b/integer.cpp index 8eadc47e..52644ffc 100644 --- a/integer.cpp +++ b/integer.cpp @@ -4,33 +4,25 @@ // Notes by JW: The Integer class needs to do two things. First, it needs to set function // 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 Integer::Two(). The function pointers are initialized in the class InitializeInteger. -// Wei's original code was much simpler. It uses the Singleton pattern, but it always produced -// memory findings. The Singleton generates memory findings because it used for a Create on -// First Use pattern. Resource destruction effectivley requires running resource acquisition -// with dependencies in reverse. For resources provided through the Singletons, there is no way -// to express the dependency order to safely destroy resources. -// The difference in the changes below is we use platform and language specific remediations -// if they are available. If not available, then we fall back to Wei's original code. If -// NO_OS_DEPENDENCE is defined, then the library uses Wei's original code. -// Under all versions of C++ on Linux and Microsoft platforms, we can use GCC's init_priority -// or MSVC's init_seg(lib) to initialize the function pointers and create the Integers 0, 1 and 2 -// after CRT startup. This avoids the Singletons and clears over half the reports of memory -// leaks. However, it does not apply to Apple or Sun platforms. -// C++11 allows us to use call_once to set the function pointers, and Integer does so when -// init_priority and init_seg(lib) are not available. The class also uses the Singleton pattern -// to ensure integers 0, 1 and 2 are available. The Singleton will produce memory findings, but -// we don't have anything else to use in this case. -// C++03 on platforms like Apple and Sun, we use a boolean flag to track when the function pointers -// have been set based on the cpu. Its just a Nifty Counter in disguise, and its similar to using -// the g_pAssignToInteger to track initialization. It has concurrency issues, but a race is not a -// problem. It does not matter if two threads both set the same pointers. The Singleton pattern -// is also used to ensure integers 0, 1 and 2 are available. The Singleton will produce memory -// findings, but we don't have anything else to use in this case. -// While not readily apparent, Integer does not need to inherit from InitializeInteger when -// init_priority and init_seg(lib) are available. They just create an InitializePointers object -// at the right time after CRT initialization. The additional class avoids the small runtime -// overhead associated with checking the flags, and hides the detail from the interface. +// and Integer::Two(). +// The function pointers are initialized in the InitializeInteger class by calling +// SetFunctionPointers(). The call to SetFunctionPointers() is guarded to run once. If C++11 +// dynamic initialization is available, then a standard run_once is used. Otherwise, and simple +// flag is used. The flag suffers a race, but the worse case is the same function pointers +// get written twice without leaking memory. +// For Integer::Zero(), Integer::One() and Integer::Two(), we use one of two strategies. First, +// if C++11 dynamic initialization is available, then we use a static variable. Second, if +// C++11 dynamic initialization is not available, then we fall back to Wei's original code of +// a Singleton. +// Wei's original code was much simpler. It simply used the Singleton pattern, but it always +// produced memory findings on some platforms. The Singleton generates memory findings because +// it uses a Create On First Use pattern (a dumb Nifty Counter) and the compiler had to be smart +// enough to fold them to return the same object. Unix and Linux compilers do a good job of 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 dynamic +// intitialization with concurrent execution is supposed to solve). #include "pch.h" #include "config.h" @@ -105,31 +97,16 @@ NAMESPACE_BEGIN(CryptoPP) static void SetFunctionPointers(); -#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) -// Add InitializePointers to perform the work of setting pointers once. -struct InitializePointers -{ - InitializePointers() - { - SetFunctionPointers(); - } -}; -// Leave InitializeInteger empty so no work is done. -InitializeInteger::InitializeInteger() -{ -} -#elif defined(CRYPTOPP_CXX11_SYNCHRONIZATION) && defined(CRYPTOPP_CXX11_DYNAMIC_INIT) -std::once_flag s_flag; 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; -InitializeInteger::InitializeInteger() -{ + static bool s_flag; MEMORY_BARRIER(); if (s_flag == false) { @@ -137,8 +114,9 @@ InitializeInteger::InitializeInteger() s_flag = true; MEMORY_BARRIER(); } +#endif // C++11 or C++03 flag +#endif // not GCC and MSC init priorities } -#endif template struct NewInteger { @@ -147,28 +125,9 @@ struct NewInteger return new Integer(i); } }; -NAMESPACE_END - -ANONYMOUS_NAMESPACE_BEGIN -#if defined(HAVE_GCC_INIT_PRIORITY) -const CryptoPP::InitializePointers s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 30))) = CryptoPP::InitializePointers(); -const CryptoPP::Integer s_zero __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 31))) = CryptoPP::Integer(0L); -const CryptoPP::Integer s_one __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 32))) = CryptoPP::Integer(1L); -const CryptoPP::Integer s_two __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 33))) = CryptoPP::Integer(2L); -#elif defined(HAVE_MSC_INIT_PRIORITY) -#pragma warning(disable: 4075) -#pragma init_seg(".CRT$XCU-030") -const CryptoPP::InitializePointers s_init; -const CryptoPP::Integer s_zero(0L); -const CryptoPP::Integer s_one(1L); -const CryptoPP::Integer s_two(2L); -#pragma warning(default: 4075) -#endif -ANONYMOUS_NAMESPACE_END // ***************** Library code ******************** -NAMESPACE_BEGIN(CryptoPP) inline static int Compare(const word *A, const word *B, size_t N) { while (N--) @@ -3086,7 +3045,8 @@ Integer Integer::Power2(size_t e) const Integer &Integer::Zero() { -#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) +#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) + static Integer s_zero(0L); return s_zero; #else return Singleton >().Ref(); @@ -3095,7 +3055,8 @@ const Integer &Integer::Zero() const Integer &Integer::One() { -#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) +#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) + static Integer s_one(1L); return s_one; #else return Singleton >().Ref(); @@ -3104,7 +3065,8 @@ const Integer &Integer::One() const Integer &Integer::Two() { -#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) +#if defined(CRYPTOPP_CXX11_DYNAMIC_INIT) + static Integer s_two(2L); return s_two; #else return Singleton >().Ref(); @@ -4812,8 +4774,38 @@ bool AssignIntToInteger(const std::type_info &valueType, void *pInteger, const v *reinterpret_cast(pInteger) = *reinterpret_cast(pInt); return true; } +#endif // CRYPTOPP_NO_ASSIGN_TO_INTEGER + +// *************************** C++ Static Initialization *************************** + +ANONYMOUS_NAMESPACE_BEGIN + +class InitInteger +{ +public: + InitInteger() + { + SetFunctionPointers(); + } +}; + +// This is not really needed because each Integer can dynamically initialize itself, +// but we take a peephole optimization and initialize the class once if init priorities are +// available. Dynamic initialization will be used if init priorities are not available. + +#if HAVE_GCC_INIT_PRIORITY + const InitInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 10))) = InitInteger(); +#elif HAVE_MSC_INIT_PRIORITY + #pragma warning(disable: 4075) + #pragma init_seg(".CRT$XCU") + const InitInteger s_init; + #pragma warning(default: 4075) +#else + const InitInteger s_init; #endif +ANONYMOUS_NAMESPACE_END + NAMESPACE_END -#endif +#endif // CRYPTOPP_IMPORTS