Move DEFAULT_CHANNEL and AAD_CHANNEL defs into cryptlib.cpp (GH #751)

The library used to provide DEFAULT_CHANNEL and AAD_CHANNEL this way. We experienced Static Initialization Order Fiasco crashes on occassion, so we moved them into cryptlib.h with internal linkage. The cost was, each translation unit got a copy of the strings which contributed to bloat. Issue 751 shows Clang compiles the global constructors for DEFAULT_CHANNEL and AAD_CHANNEL above the base ISA so we caught crashes on OS X with down-level hardware.

We are now at a "pick your poison" point. We selected Static Initialization Order Fiasco because it seems to be less prevalent.

Hat tip to the C++ Committee for allowing this problem to fester for three decades.

1 files changed, 4 insertions, 34 deletions

diff --git a/cryptlib.h b/cryptlib.h
index ebe14c4c..2f36c14a 100644
--- a/cryptlib.h
+++ b/cryptlib.h
@@ -468,28 +468,6 @@ public:
 	CRYPTOPP_DLL virtual bool GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const =0;

 };

 

-/// \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,

-///   <A HREF="http://www.cryptopp.com/wiki/NameValuePairs">NameValuePairs</A> 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.

-// User programs should use g_nullNameValuePairs rather than s_nullNameValuePairs.

-static const NullNameValuePairs s_nullNameValuePairs;

-

 // Doxygen cannot handle initialization

 #if CRYPTOPP_DOXYGEN_PROCESSING

 /// \brief Default channel for BufferedTransformation

@@ -510,21 +488,13 @@ const std::string AAD_CHANNEL;
 /// \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.

-const NameValuePairs g_nullNameValuePairs;

+const NameValuePairs& g_nullNameValuePairs;

 

 // Sun Studio 12.3 and earlier can't handle NameValuePairs initialization

-#elif defined(__SUNPRO_CC) && (__SUNPRO_CC < 0x5130)

-static const std::string DEFAULT_CHANNEL;

-static const std::string AAD_CHANNEL = "AAD";

-static const NameValuePairs& g_nullNameValuePairs = s_nullNameValuePairs;

-

-// We don't really want static here since it detracts from public symbol visibility, but the Windows

-// DLL fails to compile when the symbols are only const. Apparently Microsoft compilers don't treat

-// const the same as static in a translation unit for visibility under C++.

 #else

-static const std::string DEFAULT_CHANNEL;

-static const std::string AAD_CHANNEL("AAD");

-static const NameValuePairs& g_nullNameValuePairs(s_nullNameValuePairs);

+extern const std::string DEFAULT_CHANNEL;

+extern const std::string AAD_CHANNEL;

+extern const NameValuePairs& g_nullNameValuePairs;

 #endif

 

 // Document additional name spaces which show up elsewhere in the sources.