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

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.
pull/757/head
Jeffrey Walton 2018-12-03 06:33:15 -05:00
parent 318d53f6f9
commit 6729b29410
No known key found for this signature in database
GPG Key ID: B36AB348921B1838
2 changed files with 40 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
cryptlib.cpp
View File

@ -986,6 +986,42 @@ int LibraryVersion(CRYPTOPP_NOINLINE_DOTDOTDOT)
return CRYPTOPP_BUILD_VERSION;
}
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;}
};
#if HAVE_GCC_INIT_PRIORITY
CRYPTOPP_COMPILE_ASSERT(CRYPTOPP_INIT_PRIORITY >= 101);
const std::string DEFAULT_CHANNEL __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 25))) = "";
const std::string AAD_CHANNEL __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 26))) = "AAD";
const NullNameValuePairs s_nullNameValuePairs __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 27)));
const NameValuePairs& g_nullNameValuePairs = s_nullNameValuePairs;
#elif HAVE_MSC_INIT_PRIORITY
#pragma warning(disable: 4073)
#pragma init_seg(lib)
const std::string DEFAULT_CHANNEL = "";
const std::string AAD_CHANNEL = "AAD";
const NullNameValuePairs s_nullNameValuePairs;
const NameValuePairs& g_nullNameValuePairs = s_nullNameValuePairs;
#pragma warning(default: 4073)
#elif HAVE_XLC_INIT_PRIORITY
#pragma priority(260)
const std::string DEFAULT_CHANNEL = "";
const std::string AAD_CHANNEL = "AAD";
const NullNameValuePairs s_nullNameValuePairs;
const NameValuePairs& g_nullNameValuePairs = s_nullNameValuePairs;
#else
static const std::string s1(""), s2("AAD");
const std::string DEFAULT_CHANNEL = s1;
const std::string AAD_CHANNEL = s2;
const simple_ptr<NullNameValuePairs> s_pNullNameValuePairs(new NullNameValuePairs);
const NameValuePairs &g_nullNameValuePairs = *s_pNullNameValuePairs.m_p;
#endif
NAMESPACE_END // CryptoPP
#endif // CRYPTOPP_IMPORTS

38
cryptlib.h
View File

@ -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.