From 76ac35f36d87078da62f95b4a1167ec296e37363 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 5 Jan 2016 15:58:48 -0500 Subject: [PATCH 1/4] c++11: detect and correct for boost builds with an incompatible abi This is ugly, but temporary. boost::filesystem will likely be dropped soon after c++11 is enabled. Otherwise, we could simply roll our own copy_file. I've fixed this at the buildsystem level for now in order to avoid mixing in functional changes. Explanation: If boost (prior to 1.57) was built without c++11, it emulated scoped enums using c++98 constructs. Unfortunately, this implementation detail leaked into the abi. This was fixed in 1.57. When building against that installed version using c++11, the headers pick up on the native c++11 scoped enum support and enable it, however it will fail to link. This can be worked around by disabling c++11 scoped enums if linking will fail. Add an autoconf test to determine incompatibility. At build-time, if native enums are being used (a c++11 build), and force-disabling them causes a successful link, we can be sure that there's an incompatibility and enable the work-around. --- configure.ac | 25 +++++++++++++++++++++++++ src/wallet/walletdb.cpp | 6 ++++++ 2 files changed, 31 insertions(+) diff --git a/configure.ac b/configure.ac index 9161e2b2c..07f9a4a6f 100644 --- a/configure.ac +++ b/configure.ac @@ -619,6 +619,31 @@ if test x$use_boost = xyes; then BOOST_LIBS="$BOOST_LDFLAGS $BOOST_SYSTEM_LIB $BOOST_FILESYSTEM_LIB $BOOST_PROGRAM_OPTIONS_LIB $BOOST_THREAD_LIB $BOOST_CHRONO_LIB" +TEMP_LIBS="$LIBS" +LIBS="$BOOST_LIBS $LIBS" +TEMP_CPPFLAGS="$CPPFLAGS" +CPPFLAGS="$CPPFLAGS $BOOST_CPPFLAGS" +AC_MSG_CHECKING([for mismatched boost c++11 scoped enums]) +AC_LINK_IFELSE([AC_LANG_PROGRAM([[ + #include "boost/config.hpp" + #include "boost/version.hpp" + #if !defined(BOOST_NO_SCOPED_ENUMS) && !defined(BOOST_NO_CXX11_SCOPED_ENUMS) && BOOST_VERSION < 105700 + #define BOOST_NO_SCOPED_ENUMS + #define BOOST_NO_CXX11_SCOPED_ENUMS + #define CHECK + #endif + #include "boost/filesystem.hpp" + ]],[[ + #if defined(CHECK) + boost::filesystem::copy_file("foo", "bar"); + #else + choke; + #endif + ]])], + [AC_MSG_RESULT(mismatched); AC_DEFINE(FORCE_BOOST_EMULATED_SCOPED_ENUMS, 1, [Define this symbol if boost scoped enums are emulated])], [AC_MSG_RESULT(ok)]) +LIBS="$TEMP_LIBS" +CPPFLAGS="$TEMP_CPPFLAGS" + dnl Boost >= 1.50 uses sleep_for rather than the now-deprecated sleep, however dnl it was broken from 1.50 to 1.52 when backed by nanosleep. Use sleep_for if dnl a working version is available, else fall back to sleep. sleep was removed diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 88dc3102d..f0e177695 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -15,6 +15,12 @@ #include "utiltime.h" #include "wallet/wallet.h" +#if defined(FORCE_BOOST_EMULATED_SCOPED_ENUMS) +#define BOOST_NO_SCOPED_ENUMS +#define BOOST_NO_CXX11_SCOPED_ENUMS +#endif + +#include #include #include #include From 89f71c68c0fecf63059f6055ebdd25f1eba4c982 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 5 Jan 2016 16:10:13 -0500 Subject: [PATCH 2/4] c++11: don't throw from the reverselock destructor noexcept is default for destructors as of c++11. By throwing in reverselock's destructor if it's lock has been tampered with, the likely result is std::terminate being called. Indeed that happened before this change. Once reverselock has taken another lock (its ctor didn't throw), it makes no sense to try to grab or lock the parent lock. That is be broken/undefined behavior depending on the parent lock's implementation, but it shouldn't cause the reverselock to fail to re-lock when destroyed. To avoid those problems, simply swap the parent lock's contents with a dummy for the duration of the lock. That will ensure that any undefined behavior is caught at the call-site rather than the reverse lock's destruction. Barring a failed mutex unlock which would be indicative of a larger problem, the destructor should now never throw. --- src/reverselock.h | 5 ++++- src/test/reverselock_tests.cpp | 16 ++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/reverselock.h b/src/reverselock.h index 567636e16..fac1ccb79 100644 --- a/src/reverselock.h +++ b/src/reverselock.h @@ -15,10 +15,12 @@ public: explicit reverse_lock(Lock& lock) : lock(lock) { lock.unlock(); + lock.swap(templock); } ~reverse_lock() { - lock.lock(); + templock.lock(); + templock.swap(lock); } private: @@ -26,6 +28,7 @@ private: reverse_lock& operator=(reverse_lock const&); Lock& lock; + Lock templock; }; #endif // BITCOIN_REVERSELOCK_H diff --git a/src/test/reverselock_tests.cpp b/src/test/reverselock_tests.cpp index e7e627ae0..8bdff9700 100644 --- a/src/test/reverselock_tests.cpp +++ b/src/test/reverselock_tests.cpp @@ -42,22 +42,18 @@ BOOST_AUTO_TEST_CASE(reverselock_errors) BOOST_CHECK(failed); BOOST_CHECK(!lock.owns_lock()); - // Make sure trying to lock a lock after it has been reverse locked fails - failed = false; - bool locked = false; + // Locking the original lock after it has been taken by a reverse lock + // makes no sense. Ensure that the original lock no longer owns the lock + // after giving it to a reverse one. lock.lock(); BOOST_CHECK(lock.owns_lock()); - - try { + { reverse_lock > rlock(lock); - lock.lock(); - locked = true; - } catch(...) { - failed = true; + BOOST_CHECK(!lock.owns_lock()); } - BOOST_CHECK(locked && failed); + BOOST_CHECK(failed); BOOST_CHECK(lock.owns_lock()); } From 57d2f62c99e7ec2c1c95809f0f8f7441c3201423 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 5 Jan 2016 16:25:42 -0500 Subject: [PATCH 3/4] c++11: CAccountingEntry must be defined before use in a list c++11ism. This fixes builds against libc++. --- src/wallet/wallet.h | 164 +++++++++++++++++++++----------------------- 1 file changed, 80 insertions(+), 84 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 53a2b9669..a9d0c3f60 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -59,7 +59,6 @@ static const CAmount nHighTransactionMaxFeeWarning = 100 * nHighTransactionFeeWa static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; static const bool DEFAULT_WALLETBROADCAST = true; -class CAccountingEntry; class CBlockIndex; class CCoinControl; class COutput; @@ -445,6 +444,86 @@ public: } }; +/** + * Internal transfers. + * Database key is acentry. + */ +class CAccountingEntry +{ +public: + std::string strAccount; + CAmount nCreditDebit; + int64_t nTime; + std::string strOtherAccount; + std::string strComment; + mapValue_t mapValue; + int64_t nOrderPos; //! position in ordered transaction list + uint64_t nEntryNo; + + CAccountingEntry() + { + SetNull(); + } + + void SetNull() + { + nCreditDebit = 0; + nTime = 0; + strAccount.clear(); + strOtherAccount.clear(); + strComment.clear(); + nOrderPos = -1; + nEntryNo = 0; + } + + ADD_SERIALIZE_METHODS; + + template + inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { + if (!(nType & SER_GETHASH)) + READWRITE(nVersion); + //! Note: strAccount is serialized as part of the key, not here. + READWRITE(nCreditDebit); + READWRITE(nTime); + READWRITE(LIMITED_STRING(strOtherAccount, 65536)); + + if (!ser_action.ForRead()) + { + WriteOrderPos(nOrderPos, mapValue); + + if (!(mapValue.empty() && _ssExtra.empty())) + { + CDataStream ss(nType, nVersion); + ss.insert(ss.begin(), '\0'); + ss << mapValue; + ss.insert(ss.end(), _ssExtra.begin(), _ssExtra.end()); + strComment.append(ss.str()); + } + } + + READWRITE(LIMITED_STRING(strComment, 65536)); + + size_t nSepPos = strComment.find("\0", 0, 1); + if (ser_action.ForRead()) + { + mapValue.clear(); + if (std::string::npos != nSepPos) + { + CDataStream ss(std::vector(strComment.begin() + nSepPos + 1, strComment.end()), nType, nVersion); + ss >> mapValue; + _ssExtra = std::vector(ss.begin(), ss.end()); + } + ReadOrderPos(nOrderPos, mapValue); + } + if (std::string::npos != nSepPos) + strComment.erase(nSepPos); + + mapValue.erase("n"); + } + +private: + std::vector _ssExtra; +}; /** @@ -840,87 +919,4 @@ public: } }; - - -/** - * Internal transfers. - * Database key is acentry. - */ -class CAccountingEntry -{ -public: - std::string strAccount; - CAmount nCreditDebit; - int64_t nTime; - std::string strOtherAccount; - std::string strComment; - mapValue_t mapValue; - int64_t nOrderPos; //! position in ordered transaction list - uint64_t nEntryNo; - - CAccountingEntry() - { - SetNull(); - } - - void SetNull() - { - nCreditDebit = 0; - nTime = 0; - strAccount.clear(); - strOtherAccount.clear(); - strComment.clear(); - nOrderPos = -1; - nEntryNo = 0; - } - - ADD_SERIALIZE_METHODS; - - template - inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { - if (!(nType & SER_GETHASH)) - READWRITE(nVersion); - //! Note: strAccount is serialized as part of the key, not here. - READWRITE(nCreditDebit); - READWRITE(nTime); - READWRITE(LIMITED_STRING(strOtherAccount, 65536)); - - if (!ser_action.ForRead()) - { - WriteOrderPos(nOrderPos, mapValue); - - if (!(mapValue.empty() && _ssExtra.empty())) - { - CDataStream ss(nType, nVersion); - ss.insert(ss.begin(), '\0'); - ss << mapValue; - ss.insert(ss.end(), _ssExtra.begin(), _ssExtra.end()); - strComment.append(ss.str()); - } - } - - READWRITE(LIMITED_STRING(strComment, 65536)); - - size_t nSepPos = strComment.find("\0", 0, 1); - if (ser_action.ForRead()) - { - mapValue.clear(); - if (std::string::npos != nSepPos) - { - CDataStream ss(std::vector(strComment.begin() + nSepPos + 1, strComment.end()), nType, nVersion); - ss >> mapValue; - _ssExtra = std::vector(ss.begin(), ss.end()); - } - ReadOrderPos(nOrderPos, mapValue); - } - if (std::string::npos != nSepPos) - strComment.erase(nSepPos); - - mapValue.erase("n"); - } - -private: - std::vector _ssExtra; -}; - #endif // BITCOIN_WALLET_WALLET_H From 3968922b9623af9da9959adc49a779d6837e1f0c Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Tue, 5 Jan 2016 16:27:42 -0500 Subject: [PATCH 4/4] c++11: fix libbdb build against libc++ in c++11 mode atomic_init clashes with --- depends/packages/bdb.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index 68841afdb..e2f85ad4f 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -12,7 +12,8 @@ $(package)_config_opts_linux=--with-pic endef define $(package)_preprocess_cmds - sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' dbinc/atomic.h + sed -i.old 's/__atomic_compare_exchange/__atomic_compare_exchange_db/' dbinc/atomic.h && \ + sed -i.old 's/atomic_init/atomic_init_db/' dbinc/atomic.h mp/mp_region.c mp/mp_mvcc.c mp/mp_fget.c mutex/mut_method.c mutex/mut_tas.c endef define $(package)_config_cmds