From c305e8812772582edeb6e373b6e34feb59f3799f Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Mon, 27 Mar 2017 06:06:12 -0400 Subject: [PATCH] Fix runtime crash when CRYPTOPP_INIT_PRIORITY=0 Couple use of initialization priorities to no NO_OS_DEPENDENCE Add comments explaining what integer does, how it does it, and why we want to inprove on the Singleton pattern as a resource manager. Update documentation. --- config.h | 5 ++-- integer.cpp | 79 +++++++++++++++++++++++++++++++++++++++++++++++------ integer.h | 13 +++++---- 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/config.h b/config.h index f3f37d3e..1d96fbd4 100644 --- a/config.h +++ b/config.h @@ -596,13 +596,14 @@ NAMESPACE_END # define CRYPTOPP_USER_PRIORITY 350 #endif -#if (CRYPTOPP_INIT_PRIORITY > 0) && !(defined(__APPLE__) || defined(__sun__)) +// Most platforms allow us to specify when to create C++ objects. Apple and Sun do not. +#if (CRYPTOPP_INIT_PRIORITY > 0) && !(defined(NO_OS_DEPENDENCE) || defined(__APPLE__) || defined(__sun__)) # if (CRYPTOPP_GCC_VERSION >= 30000) || (CRYPTOPP_LLVM_CLANG_VERSION >= 20900) || (_INTEL_COMPILER >= 800) # define HAVE_GCC_INIT_PRIORITY 1 # elif (CRYPTOPP_MSC_VERSION >= 1310) # define HAVE_MSC_INIT_PRIORITY 1 # endif -#endif // CRYPTOPP_INIT_PRIORITY, Sun, Darwin +#endif // CRYPTOPP_INIT_PRIORITY, NO_OS_DEPENDENCE, Apple, Sun // ***************** determine availability of OS features ******************** diff --git a/integer.cpp b/integer.cpp index 8df0b567..317ad89f 100644 --- a/integer.cpp +++ b/integer.cpp @@ -1,6 +1,37 @@ // integer.cpp - originally written and placed in the public domain by Wei Dai // contains public domain code contributed by Alister Lee and Leonard Janke +// 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. + #include "pch.h" #include "config.h" @@ -28,6 +59,7 @@ #include "smartptr.h" #include "algparam.h" #include "filters.h" +#include "stdcpp.h" #include "asn.h" #include "oids.h" #include "words.h" @@ -73,11 +105,40 @@ 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() { - SetFunctionPointers(); } - +#elif defined(CRYPTOPP_CXX11_SYNCHRONIZATION) && defined(CRYPTOPP_CXX11_DYNAMIC_INIT) +std::once_flag s_flag; +InitializeInteger::InitializeInteger() +{ + std::call_once(s_flag, []() { + SetFunctionPointers(); + }); +} +#else +static bool s_flag; +InitializeInteger::InitializeInteger() +{ + MEMORY_BARRIER(); + if (s_flag == false) + { + SetFunctionPointers(); + s_flag = true; + MEMORY_BARRIER(); + } +} +#endif template struct NewInteger { @@ -89,15 +150,15 @@ struct NewInteger NAMESPACE_END ANONYMOUS_NAMESPACE_BEGIN -#if HAVE_GCC_INIT_PRIORITY -const CryptoPP::InitializeInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 30))) = CryptoPP::InitializeInteger(); +#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 HAVE_MSC_INIT_PRIORITY +#elif defined(HAVE_MSC_INIT_PRIORITY) #pragma warning(disable: 4075) #pragma init_seg(".CRT$XCU-030") -const CryptoPP::InitializeInteger s_init; +const CryptoPP::InitializePointers s_init; const CryptoPP::Integer s_zero(0L); const CryptoPP::Integer s_one(1L); const CryptoPP::Integer s_two(2L); @@ -3025,7 +3086,7 @@ Integer Integer::Power2(size_t e) const Integer &Integer::Zero() { -#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY +#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) return s_zero; #else return Singleton >().Ref(); @@ -3034,7 +3095,7 @@ const Integer &Integer::Zero() const Integer &Integer::One() { -#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY +#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) return s_one; #else return Singleton >().Ref(); @@ -3043,7 +3104,7 @@ const Integer &Integer::One() const Integer &Integer::Two() { -#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY +#if defined(HAVE_GCC_INIT_PRIORITY) || defined(HAVE_MSC_INIT_PRIORITY) return s_two; #else return Singleton >().Ref(); diff --git a/integer.h b/integer.h index 6e639b2b..41700447 100644 --- a/integer.h +++ b/integer.h @@ -8,6 +8,9 @@ //! has two data members. The first is a IntegerSecBlock (a SecBlock) and it is //! used to hold the representation. The second is a Sign (an enumeration), and it is //! used to track the sign of the Integer. +//! \details For details on how the Integer class initializes its function pointers using +//! InitializeInteger and how it creates Integer::Zero(), Integer::One(), and +//! Integer::Two(), then see the comments at the top of integer.cpp. //! \since Crypto++ 1.0 #ifndef CRYPTOPP_INTEGER_H @@ -38,14 +41,12 @@ typedef SecBlock > IntegerSecBlock; //! has two data members. The first is a IntegerSecBlock (a SecBlock) and it is //! used to hold the representation. The second is a Sign (an enumeration), and it is //! used to track the sign of the Integer. +//! \details For details on how the Integer class initializes its function pointers using +//! InitializeInteger and how it creates Integer::Zero(), Integer::One(), and +//! Integer::Two(), then see the comments at the top of integer.cpp. //! \since Crypto++ 1.0 //! \nosubgrouping -class CRYPTOPP_DLL Integer -#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY - : private InitializeInteger, public ASN1Object -#else - : public ASN1Object -#endif +class CRYPTOPP_DLL Integer : private InitializeInteger, public ASN1Object { public: //! \name ENUMS, EXCEPTIONS, and TYPEDEFS