From 6aa4e6466a60532199efce5eef3f66da2f74856f Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Mon, 21 Aug 2023 20:28:11 +0000 Subject: [PATCH] c++20 serialization endian changes POC - use of std::endian means that we no longer have to rely on configure checks and compiler macros for compile-time endianness checks. - countl_zero replaces the need for __builtin_clz* - use macros to check for byteswap builtins --- configure.ac | 14 +- src/Makefile.am | 1 - src/compat/byteswap.h | 59 --------- src/compat/endian.h | 275 +++++++++++---------------------------- src/crypto/common.h | 50 +++---- src/i2p.cpp | 2 +- src/serialize.h | 29 +++-- src/test/bswap_tests.cpp | 8 +- 8 files changed, 116 insertions(+), 322 deletions(-) delete mode 100644 src/compat/byteswap.h diff --git a/configure.ac b/configure.ac index adc862d251459..9984052a99ea3 100644 --- a/configure.ac +++ b/configure.ac @@ -977,7 +977,7 @@ if test "$TARGET_OS" = "darwin"; then AX_CHECK_LINK_FLAG([-Wl,-fixup_chains], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-fixup_chains"], [], [$LDFLAG_WERROR]) fi -AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h]) +AC_CHECK_HEADERS([sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h]) AC_CHECK_DECLS([getifaddrs, freeifaddrs],[CHECK_SOCKET],, [#include @@ -992,18 +992,6 @@ AC_CHECK_DECLS([pipe2]) AC_CHECK_FUNCS([timingsafe_bcmp]) -AC_CHECK_DECLS([le16toh, le32toh, le64toh, htole16, htole32, htole64, be16toh, be32toh, be64toh, htobe16, htobe32, htobe64],,, - [#if HAVE_ENDIAN_H - #include - #elif HAVE_SYS_ENDIAN_H - #include - #endif]) - -AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64],,, - [#if HAVE_BYTESWAP_H - #include - #endif]) - AC_MSG_CHECKING([for __builtin_clzl]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ (void) __builtin_clzl(0); diff --git a/src/Makefile.am b/src/Makefile.am index 780f547b4b838..9a5f668880208 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -141,7 +141,6 @@ BITCOIN_CORE_H = \ common/run_command.h \ common/url.h \ compat/assumptions.h \ - compat/byteswap.h \ compat/compat.h \ compat/cpuid.h \ compat/endian.h \ diff --git a/src/compat/byteswap.h b/src/compat/byteswap.h deleted file mode 100644 index 9ee71ef267d3a..0000000000000 --- a/src/compat/byteswap.h +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) 2014-2022 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_COMPAT_BYTESWAP_H -#define BITCOIN_COMPAT_BYTESWAP_H - -#if defined(HAVE_CONFIG_H) -#include -#endif - -#include - -#if defined(HAVE_BYTESWAP_H) -#include -#endif - -#if defined(MAC_OSX) - -#include -#define bswap_16(x) OSSwapInt16(x) -#define bswap_32(x) OSSwapInt32(x) -#define bswap_64(x) OSSwapInt64(x) - -#else -// Non-MacOS / non-Darwin - -#if HAVE_DECL_BSWAP_16 == 0 -inline uint16_t bswap_16(uint16_t x) -{ - return (x >> 8) | (x << 8); -} -#endif // HAVE_DECL_BSWAP16 == 0 - -#if HAVE_DECL_BSWAP_32 == 0 -inline uint32_t bswap_32(uint32_t x) -{ - return (((x & 0xff000000U) >> 24) | ((x & 0x00ff0000U) >> 8) | - ((x & 0x0000ff00U) << 8) | ((x & 0x000000ffU) << 24)); -} -#endif // HAVE_DECL_BSWAP32 == 0 - -#if HAVE_DECL_BSWAP_64 == 0 -inline uint64_t bswap_64(uint64_t x) -{ - return (((x & 0xff00000000000000ull) >> 56) - | ((x & 0x00ff000000000000ull) >> 40) - | ((x & 0x0000ff0000000000ull) >> 24) - | ((x & 0x000000ff00000000ull) >> 8) - | ((x & 0x00000000ff000000ull) << 8) - | ((x & 0x0000000000ff0000ull) << 24) - | ((x & 0x000000000000ff00ull) << 40) - | ((x & 0x00000000000000ffull) << 56)); -} -#endif // HAVE_DECL_BSWAP64 == 0 - -#endif // defined(MAC_OSX) - -#endif // BITCOIN_COMPAT_BYTESWAP_H diff --git a/src/compat/endian.h b/src/compat/endian.h index 882de2dbf0e49..aefaf6471935b 100644 --- a/src/compat/endian.h +++ b/src/compat/endian.h @@ -5,237 +5,120 @@ #ifndef BITCOIN_COMPAT_ENDIAN_H #define BITCOIN_COMPAT_ENDIAN_H -#if defined(HAVE_CONFIG_H) -#include -#endif - -#include - +#include #include -#if defined(HAVE_ENDIAN_H) -#include -#elif defined(HAVE_SYS_ENDIAN_H) -#include -#endif - -#ifndef HAVE_CONFIG_H -// While not technically a supported configuration, defaulting to defining these -// DECLs when we were compiled without autotools makes it easier for other build -// systems to build things like libbitcoinconsensus for strange targets. -#ifdef htobe16 -#define HAVE_DECL_HTOBE16 1 -#endif -#ifdef htole16 -#define HAVE_DECL_HTOLE16 1 -#endif -#ifdef be16toh -#define HAVE_DECL_BE16TOH 1 -#endif -#ifdef le16toh -#define HAVE_DECL_LE16TOH 1 -#endif - -#ifdef htobe32 -#define HAVE_DECL_HTOBE32 1 -#endif -#ifdef htole32 -#define HAVE_DECL_HTOLE32 1 -#endif -#ifdef be32toh -#define HAVE_DECL_BE32TOH 1 -#endif -#ifdef le32toh -#define HAVE_DECL_LE32TOH 1 -#endif - -#ifdef htobe64 -#define HAVE_DECL_HTOBE64 1 -#endif -#ifdef htole64 -#define HAVE_DECL_HTOLE64 1 +#ifndef DISABLE_BUILTIN_BSWAPS +# if defined __has_builtin +# if __has_builtin(__builtin_bswap16) +# define bitcoin_builtin_bswap16(x) __builtin_bswap16(x) +# endif +# if __has_builtin(__builtin_bswap32) +# define bitcoin_builtin_bswap32(x) __builtin_bswap32(x) +# endif +# if __has_builtin(__builtin_bswap64) +# define bitcoin_builtin_bswap64(x) __builtin_bswap64(x) +# endif +# elif defined(_MSC_VER) +# define bitcoin_builtin_bswap16(x) _byteswap_ushort(x) +# define bitcoin_builtin_bswap32(x) _byteswap_ulong(x) +# define bitcoin_builtin_bswap64(x) _byteswap_uint64(x) +# endif #endif -#ifdef be64toh -#define HAVE_DECL_BE64TOH 1 -#endif -#ifdef le64toh -#define HAVE_DECL_LE64TOH 1 -#endif - -#endif // HAVE_CONFIG_H - -#if defined(WORDS_BIGENDIAN) - -#if HAVE_DECL_HTOBE16 == 0 -inline uint16_t htobe16(uint16_t host_16bits) -{ - return host_16bits; -} -#endif // HAVE_DECL_HTOBE16 - -#if HAVE_DECL_HTOLE16 == 0 -inline uint16_t htole16(uint16_t host_16bits) -{ - return bswap_16(host_16bits); -} -#endif // HAVE_DECL_HTOLE16 - -#if HAVE_DECL_BE16TOH == 0 -inline uint16_t be16toh(uint16_t big_endian_16bits) -{ - return big_endian_16bits; -} -#endif // HAVE_DECL_BE16TOH - -#if HAVE_DECL_LE16TOH == 0 -inline uint16_t le16toh(uint16_t little_endian_16bits) -{ - return bswap_16(little_endian_16bits); -} -#endif // HAVE_DECL_LE16TOH -#if HAVE_DECL_HTOBE32 == 0 -inline uint32_t htobe32(uint32_t host_32bits) +static constexpr uint16_t internal_bswap_16(uint16_t x) { - return host_32bits; -} -#endif // HAVE_DECL_HTOBE32 - -#if HAVE_DECL_HTOLE32 == 0 -inline uint32_t htole32(uint32_t host_32bits) -{ - return bswap_32(host_32bits); -} -#endif // HAVE_DECL_HTOLE32 - -#if HAVE_DECL_BE32TOH == 0 -inline uint32_t be32toh(uint32_t big_endian_32bits) -{ - return big_endian_32bits; -} -#endif // HAVE_DECL_BE32TOH - -#if HAVE_DECL_LE32TOH == 0 -inline uint32_t le32toh(uint32_t little_endian_32bits) -{ - return bswap_32(little_endian_32bits); -} -#endif // HAVE_DECL_LE32TOH - -#if HAVE_DECL_HTOBE64 == 0 -inline uint64_t htobe64(uint64_t host_64bits) -{ - return host_64bits; -} -#endif // HAVE_DECL_HTOBE64 - -#if HAVE_DECL_HTOLE64 == 0 -inline uint64_t htole64(uint64_t host_64bits) -{ - return bswap_64(host_64bits); +#ifdef bitcoin_builtin_bswap16 + return bitcoin_builtin_bswap16(x); +#else + return (x >> 8) | (x << 8); +#endif } -#endif // HAVE_DECL_HTOLE64 -#if HAVE_DECL_BE64TOH == 0 -inline uint64_t be64toh(uint64_t big_endian_64bits) +static constexpr uint32_t internal_bswap_32(uint32_t x) { - return big_endian_64bits; +#ifdef bitcoin_builtin_bswap32 + return bitcoin_builtin_bswap32(x); +#else + return (((x & 0xff000000U) >> 24) | ((x & 0x00ff0000U) >> 8) | + ((x & 0x0000ff00U) << 8) | ((x & 0x000000ffU) << 24)); +#endif } -#endif // HAVE_DECL_BE64TOH -#if HAVE_DECL_LE64TOH == 0 -inline uint64_t le64toh(uint64_t little_endian_64bits) +static constexpr uint64_t internal_bswap_64(uint64_t x) { - return bswap_64(little_endian_64bits); +#ifdef bitcoin_builtin_bswap64 + return bitcoin_builtin_bswap64(x); +#else + return (((x & 0xff00000000000000ULL) >> 56) + | ((x & 0x00ff000000000000ULL) >> 40) + | ((x & 0x0000ff0000000000ULL) >> 24) + | ((x & 0x000000ff00000000ULL) >> 8) + | ((x & 0x00000000ff000000ULL) << 8) + | ((x & 0x0000000000ff0000ULL) << 24) + | ((x & 0x000000000000ff00ULL) << 40) + | ((x & 0x00000000000000ffULL) << 56)); +#endif } -#endif // HAVE_DECL_LE64TOH - -#else // WORDS_BIGENDIAN - -#if HAVE_DECL_HTOBE16 == 0 -inline uint16_t htobe16(uint16_t host_16bits) +static uint16_t constexpr htobe16_internal(uint16_t host_16bits) { - return bswap_16(host_16bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_16(host_16bits); + else return host_16bits; } -#endif // HAVE_DECL_HTOBE16 - -#if HAVE_DECL_HTOLE16 == 0 -inline uint16_t htole16(uint16_t host_16bits) +static uint16_t constexpr htole16_internal(uint16_t host_16bits) { - return host_16bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_16(host_16bits); + else return host_16bits; } -#endif // HAVE_DECL_HTOLE16 - -#if HAVE_DECL_BE16TOH == 0 -inline uint16_t be16toh(uint16_t big_endian_16bits) +static uint16_t constexpr be16toh_internal(uint16_t big_endian_16bits) { - return bswap_16(big_endian_16bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_16(big_endian_16bits); + else return big_endian_16bits; } -#endif // HAVE_DECL_BE16TOH - -#if HAVE_DECL_LE16TOH == 0 -inline uint16_t le16toh(uint16_t little_endian_16bits) +static uint16_t constexpr le16toh_internal(uint16_t little_endian_16bits) { - return little_endian_16bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_16(little_endian_16bits); + else return little_endian_16bits; } -#endif // HAVE_DECL_LE16TOH - -#if HAVE_DECL_HTOBE32 == 0 -inline uint32_t htobe32(uint32_t host_32bits) +static uint32_t constexpr htobe32_internal(uint32_t host_32bits) { - return bswap_32(host_32bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_32(host_32bits); + else return host_32bits; } -#endif // HAVE_DECL_HTOBE32 - -#if HAVE_DECL_HTOLE32 == 0 -inline uint32_t htole32(uint32_t host_32bits) +static uint32_t constexpr htole32_internal(uint32_t host_32bits) { - return host_32bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_32(host_32bits); + else return host_32bits; } -#endif // HAVE_DECL_HTOLE32 - -#if HAVE_DECL_BE32TOH == 0 -inline uint32_t be32toh(uint32_t big_endian_32bits) +static uint32_t constexpr be32toh_internal(uint32_t big_endian_32bits) { - return bswap_32(big_endian_32bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_32(big_endian_32bits); + else return big_endian_32bits; } -#endif // HAVE_DECL_BE32TOH - -#if HAVE_DECL_LE32TOH == 0 -inline uint32_t le32toh(uint32_t little_endian_32bits) +static uint32_t constexpr le32toh_internal(uint32_t little_endian_32bits) { - return little_endian_32bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_32(little_endian_32bits); + else return little_endian_32bits; } -#endif // HAVE_DECL_LE32TOH - -#if HAVE_DECL_HTOBE64 == 0 -inline uint64_t htobe64(uint64_t host_64bits) +static uint64_t constexpr htobe64_internal(uint64_t host_64bits) { - return bswap_64(host_64bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_64(host_64bits); + else return host_64bits; } -#endif // HAVE_DECL_HTOBE64 - -#if HAVE_DECL_HTOLE64 == 0 -inline uint64_t htole64(uint64_t host_64bits) +static uint64_t constexpr htole64_internal(uint64_t host_64bits) { - return host_64bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_64(host_64bits); + else return host_64bits; } -#endif // HAVE_DECL_HTOLE64 - -#if HAVE_DECL_BE64TOH == 0 -inline uint64_t be64toh(uint64_t big_endian_64bits) +static uint64_t constexpr be64toh_internal(uint64_t big_endian_64bits) { - return bswap_64(big_endian_64bits); + if constexpr (std::endian::native == std::endian::little) return internal_bswap_64(big_endian_64bits); + else return big_endian_64bits; } -#endif // HAVE_DECL_BE64TOH - -#if HAVE_DECL_LE64TOH == 0 -inline uint64_t le64toh(uint64_t little_endian_64bits) +static uint64_t constexpr le64toh_internal(uint64_t little_endian_64bits) { - return little_endian_64bits; + if constexpr (std::endian::native == std::endian::big) return internal_bswap_64(little_endian_64bits); + else return little_endian_64bits; } -#endif // HAVE_DECL_LE64TOH - -#endif // WORDS_BIGENDIAN #endif // BITCOIN_COMPAT_ENDIAN_H diff --git a/src/crypto/common.h b/src/crypto/common.h index 6ae5d4cd241d9..667e748f9d5b2 100644 --- a/src/crypto/common.h +++ b/src/crypto/common.h @@ -5,51 +5,48 @@ #ifndef BITCOIN_CRYPTO_COMMON_H #define BITCOIN_CRYPTO_COMMON_H -#if defined(HAVE_CONFIG_H) -#include -#endif - -#include -#include - #include +#include +#include +#include + uint16_t static inline ReadLE16(const unsigned char* ptr) { uint16_t x; memcpy(&x, ptr, 2); - return le16toh(x); + return le16toh_internal(x); } uint32_t static inline ReadLE32(const unsigned char* ptr) { uint32_t x; memcpy(&x, ptr, 4); - return le32toh(x); + return le32toh_internal(x); } uint64_t static inline ReadLE64(const unsigned char* ptr) { uint64_t x; memcpy(&x, ptr, 8); - return le64toh(x); + return le64toh_internal(x); } void static inline WriteLE16(unsigned char* ptr, uint16_t x) { - uint16_t v = htole16(x); + uint16_t v = htole16_internal(x); memcpy(ptr, &v, 2); } void static inline WriteLE32(unsigned char* ptr, uint32_t x) { - uint32_t v = htole32(x); + uint32_t v = htole32_internal(x); memcpy(ptr, &v, 4); } void static inline WriteLE64(unsigned char* ptr, uint64_t x) { - uint64_t v = htole64(x); + uint64_t v = htole64_internal(x); memcpy(ptr, &v, 8); } @@ -57,54 +54,39 @@ uint16_t static inline ReadBE16(const unsigned char* ptr) { uint16_t x; memcpy(&x, ptr, 2); - return be16toh(x); + return be16toh_internal(x); } uint32_t static inline ReadBE32(const unsigned char* ptr) { uint32_t x; memcpy(&x, ptr, 4); - return be32toh(x); + return be32toh_internal(x); } uint64_t static inline ReadBE64(const unsigned char* ptr) { uint64_t x; memcpy(&x, ptr, 8); - return be64toh(x); + return be64toh_internal(x); } void static inline WriteBE32(unsigned char* ptr, uint32_t x) { - uint32_t v = htobe32(x); + uint32_t v = htobe32_internal(x); memcpy(ptr, &v, 4); } void static inline WriteBE64(unsigned char* ptr, uint64_t x) { - uint64_t v = htobe64(x); + uint64_t v = htobe64_internal(x); memcpy(ptr, &v, 8); } /** Return the smallest number n such that (x >> n) == 0 (or 64 if the highest bit in x is set. */ uint64_t static inline CountBits(uint64_t x) { -#if HAVE_BUILTIN_CLZL - if (sizeof(unsigned long) >= sizeof(uint64_t)) { - return x ? 8 * sizeof(unsigned long) - __builtin_clzl(x) : 0; - } -#endif -#if HAVE_BUILTIN_CLZLL - if (sizeof(unsigned long long) >= sizeof(uint64_t)) { - return x ? 8 * sizeof(unsigned long long) - __builtin_clzll(x) : 0; - } -#endif - int ret = 0; - while (x) { - x >>= 1; - ++ret; - } - return ret; + return (sizeof(x) * 8 )- std::countl_zero(x); } #endif // BITCOIN_CRYPTO_COMMON_H diff --git a/src/i2p.cpp b/src/i2p.cpp index c891562d00bfd..4b79a6826bd0b 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -392,7 +392,7 @@ Binary Session::MyDestination() const } memcpy(&cert_len, &m_private_key.at(CERT_LEN_POS), sizeof(cert_len)); - cert_len = be16toh(cert_len); + cert_len = be16toh_internal(cert_len); const size_t dest_len = DEST_LEN_BASE + cert_len; diff --git a/src/serialize.h b/src/serialize.h index 19585c630ab2a..02b9341256aaf 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -56,27 +57,27 @@ template inline void ser_writedata8(Stream &s, uint8_t obj) } template inline void ser_writedata16(Stream &s, uint16_t obj) { - obj = htole16(obj); + obj = htole16_internal(obj); s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata16be(Stream &s, uint16_t obj) { - obj = htobe16(obj); + obj = htobe16_internal(obj); s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata32(Stream &s, uint32_t obj) { - obj = htole32(obj); + obj = htole32_internal(obj); s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata32be(Stream &s, uint32_t obj) { - obj = htobe32(obj); + obj = htobe32_internal(obj); s.write(AsBytes(Span{&obj, 1})); } template inline void ser_writedata64(Stream &s, uint64_t obj) { - obj = htole64(obj); + obj = htole64_internal(obj); s.write(AsBytes(Span{&obj, 1})); } template inline uint8_t ser_readdata8(Stream &s) @@ -89,31 +90,31 @@ template inline uint16_t ser_readdata16(Stream &s) { uint16_t obj; s.read(AsWritableBytes(Span{&obj, 1})); - return le16toh(obj); + return le16toh_internal(obj); } template inline uint16_t ser_readdata16be(Stream &s) { uint16_t obj; s.read(AsWritableBytes(Span{&obj, 1})); - return be16toh(obj); + return be16toh_internal(obj); } template inline uint32_t ser_readdata32(Stream &s) { uint32_t obj; s.read(AsWritableBytes(Span{&obj, 1})); - return le32toh(obj); + return le32toh_internal(obj); } template inline uint32_t ser_readdata32be(Stream &s) { uint32_t obj; s.read(AsWritableBytes(Span{&obj, 1})); - return be32toh(obj); + return be32toh_internal(obj); } template inline uint64_t ser_readdata64(Stream &s) { uint64_t obj; s.read(AsWritableBytes(Span{&obj, 1})); - return le64toh(obj); + return le64toh_internal(obj); } @@ -544,10 +545,10 @@ struct CustomUintFormatter { if (v < 0 || v > MAX) throw std::ios_base::failure("CustomUintFormatter value out of range"); if (BigEndian) { - uint64_t raw = htobe64(v); + uint64_t raw = htobe64_internal(v); s.write(AsBytes(Span{&raw, 1}).last(Bytes)); } else { - uint64_t raw = htole64(v); + uint64_t raw = htole64_internal(v); s.write(AsBytes(Span{&raw, 1}).first(Bytes)); } } @@ -559,10 +560,10 @@ struct CustomUintFormatter uint64_t raw = 0; if (BigEndian) { s.read(AsWritableBytes(Span{&raw, 1}).last(Bytes)); - v = static_cast(be64toh(raw)); + v = static_cast(be64toh_internal(raw)); } else { s.read(AsWritableBytes(Span{&raw, 1}).first(Bytes)); - v = static_cast(le64toh(raw)); + v = static_cast(le64toh_internal(raw)); } } }; diff --git a/src/test/bswap_tests.cpp b/src/test/bswap_tests.cpp index 2be7122fc16d7..7a3b99a9ac444 100644 --- a/src/test/bswap_tests.cpp +++ b/src/test/bswap_tests.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include @@ -16,9 +16,9 @@ BOOST_AUTO_TEST_CASE(bswap_tests) uint16_t e1 = 0x3412; uint32_t e2 = 0xbc9a7856; uint64_t e3 = 0xbc9a78563412f0de; - BOOST_CHECK(bswap_16(u1) == e1); - BOOST_CHECK(bswap_32(u2) == e2); - BOOST_CHECK(bswap_64(u3) == e3); + BOOST_CHECK(internal_bswap_16(u1) == e1); + BOOST_CHECK(internal_bswap_32(u2) == e2); + BOOST_CHECK(internal_bswap_64(u3) == e3); } BOOST_AUTO_TEST_SUITE_END()