From 9540ecbc346255b52eae1edacf28d40c333371eb Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:03:14 +0000 Subject: [PATCH 1/4] Squashed 'src/minisketch/' changes from a571ba20f9..3472e2f5ec 3472e2f5ec Merge sipa/minisketch#81: Avoid overflowing shift by special casing inverse of 1 653d8b2e26 Avoid overflowing shift by special casing inverse of 1 33b7c200b9 Merge sipa/minisketch#80: Add c++20 version of CountBits 4a48f31a37 Merge sipa/minisketch#83: ci: Fix "s390x (big-endian)" task 82b6488acb Add c++20 version of CountBits 0498084d31 ci: Fix "s390x (big-endian)" task 71709dca9e Merge sipa/minisketch#82: ci: Fix `x86_64-w64-mingw32` task 9e6127fa98 Merge sipa/minisketch#74: Avoid >> above type width in BitWriter ed420bc170 ci: Fix `x86_64-w64-mingw32` task fe1040f227 Drop -Wno-shift-count-overflow compile flag 154bcd43bd Avoid >> above type width in BitWriter 67b87acdb6 Merge sipa/minisketch#78: ci: Update macOS image for CI 7de7250416 ci: Update macOS image for CI 83d812ea9f Merge sipa/minisketch#73: ci: Use correct variable to designate C++ compiler e051a7d690 ci: Install wine32 package for Windows tests 2d2c695d78 build: Drop unused `CC` variable 1810fcbd11 ci: Use correct variable to designate C++ compiler 022b959049 Merge sipa/minisketch#77: Add missing include 08443c4892 Add missing include git-subtree-dir: src/minisketch git-subtree-split: 3472e2f5ec75ace39ce9243af6b3fee233a67492 --- .cirrus.yml | 57 ++++++++++++++++++++------------------ ci/cirrus.sh | 8 +++--- ci/linux-debian.Dockerfile | 6 ++-- configure.ac | 6 ---- src/int_utils.h | 34 ++++++++++++++++++----- 5 files changed, 64 insertions(+), 47 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 4a5353f1373c9..5ceefee2cffa3 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -36,17 +36,6 @@ env_matrix_snippet: &ENV_MATRIX_VALGRIND TESTRUNS: 1 BUILD: -env_matrix_snippet: &ENV_MATRIX_SAN - - env: - ENABLE_FIELDS: 28 - - env: - BUILD: distcheck - - env: - CXXFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" - LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" - UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1" - BENCH: no - env_matrix_snippet: &ENV_MATRIX_SAN_VALGRIND - env: ENABLE_FIELDS: "11,64,37" @@ -72,9 +61,9 @@ task: << : *ENV_MATRIX_SAN_VALGRIND matrix: - env: - CC: gcc + CXX: g++ - env: - CC: clang + CXX: clang++ -gdwarf-4 << : *MERGE_BASE test_script: - ./ci/cirrus.sh @@ -92,30 +81,45 @@ task: << : *ENV_MATRIX_VALGRIND matrix: - env: - CC: i686-linux-gnu-gcc + CXX: i686-linux-gnu-g++ - env: - CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include + CXX: clang++ --target=i686-linux-gnu -gdwarf-4 + CXXFLAGS: -g -O2 -isystem /usr/i686-linux-gnu/include -isystem /usr/i686-linux-gnu/include/c++/10/i686-linux-gnu test_script: - ./ci/cirrus.sh << : *CAT_LOGS task: - name: "x86_64: macOS Catalina" + name: "arm64: macOS Monterey" macos_instance: - image: catalina-base + image: ghcr.io/cirruslabs/macos-monterey-base:latest env: - # Cirrus gives us a fixed number of 12 virtual CPUs. - MAKEFLAGS: -j13 - matrix: - << : *ENV_MATRIX_SAN + # Cirrus gives us a fixed number of 4 virtual CPUs. + MAKEFLAGS: -j5 matrix: - env: - CC: gcc-9 + CXX: g++-11 + # Homebrew's gcc for arm64 has no libubsan. + matrix: + - env: + ENABLE_FIELDS: 28 + - env: + BUILD: distcheck - env: - CC: clang + CXX: clang++ + matrix: + - env: + ENABLE_FIELDS: 28 + - env: + BUILD: distcheck + - env: + CXXFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" + LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer" + UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1" + BENCH: no brew_script: - brew update - - brew install automake libtool gcc@9 + - brew install automake libtool gcc@11 << : *MERGE_BASE test_script: - ./ci/cirrus.sh @@ -128,13 +132,11 @@ task: cpu: 4 memory: 2G env: - EXEC_CMD: qemu-s390x -L /usr/s390x-linux-gnu + EXEC_CMD: qemu-s390x HOST: s390x-linux-gnu BUILD: << : *MERGE_BASE test_script: - # https://sourceware.org/bugzilla/show_bug.cgi?id=27008 - - rm /etc/ld.so.cache - ./ci/cirrus.sh << : *CAT_LOGS @@ -146,6 +148,7 @@ task: memory: 2G env: EXEC_CMD: wine + EXEC_EXT: .exe HOST: x86_64-w64-mingw32 BUILD: << : *MERGE_BASE diff --git a/ci/cirrus.sh b/ci/cirrus.sh index 02f737ca7f2eb..36250d165195d 100755 --- a/ci/cirrus.sh +++ b/ci/cirrus.sh @@ -7,7 +7,7 @@ export LC_ALL=C env >> test_env.log -$CC -v || true +$CXX -v || true valgrind --version || true ./autogen.sh @@ -32,10 +32,10 @@ then fi if [ -n "$EXEC_CMD" ]; then - $EXEC_CMD ./test $TESTRUNS - $EXEC_CMD ./test-verify $TESTRUNS + $EXEC_CMD "./test$EXEC_EXT" $TESTRUNS + $EXEC_CMD "./test-verify$EXEC_EXT" $TESTRUNS fi if [ "$BENCH" = "yes" ]; then - $EXEC_CMD ./bench + $EXEC_CMD "./bench$EXEC_EXT" fi diff --git a/ci/linux-debian.Dockerfile b/ci/linux-debian.Dockerfile index 63e5412ee79cc..122af36e1f044 100644 --- a/ci/linux-debian.Dockerfile +++ b/ci/linux-debian.Dockerfile @@ -8,10 +8,10 @@ RUN apt-get update RUN apt-get install --no-install-recommends --no-upgrade -y \ git ca-certificates \ make automake libtool pkg-config dpkg-dev valgrind qemu-user \ - gcc g++ clang libc6-dbg \ + gcc g++ clang libclang-rt-dev libc6-dbg \ gcc-i686-linux-gnu g++-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \ - g++-s390x-linux-gnu gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ - wine g++-mingw-w64-x86-64 + g++-s390x-linux-gnu libstdc++6:s390x gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ + wine wine64 g++-mingw-w64-x86-64 # Run a dummy command in wine to make it set up configuration RUN wine true || true diff --git a/configure.ac b/configure.ac index 83910448a2c2a..cd52d7f412a69 100644 --- a/configure.ac +++ b/configure.ac @@ -104,11 +104,6 @@ esac AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-fvisibility=hidden],[CXXFLAGS="$CXXFLAGS -fvisibility=hidden"],[],[$CXXFLAG_WERROR]) -## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all -## unknown options if any other warning is produced. Test the -Wfoo case, and -## set the -Wno-foo case if it works. -AX_CHECK_COMPILE_FLAG([-Wshift-count-overflow],[NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-shift-count-overflow"],,[[$CXXFLAG_WERROR]]) - if test "x$use_ccache" != "xno"; then AC_MSG_CHECKING(if ccache should be used) if test x$CCACHE = x; then @@ -119,7 +114,6 @@ if test "x$use_ccache" != "xno"; then fi else use_ccache=yes - CC="$ac_cv_path_CCACHE $CC" CXX="$ac_cv_path_CCACHE $CXX" fi AC_MSG_RESULT($use_ccache) diff --git a/src/int_utils.h b/src/int_utils.h index d21ba56f336c0..2b3d8cb402668 100644 --- a/src/int_utils.h +++ b/src/int_utils.h @@ -7,13 +7,16 @@ #ifndef _MINISKETCH_INT_UTILS_H_ #define _MINISKETCH_INT_UTILS_H_ +#include #include #include #include #include -#ifdef _MSC_VER +#if defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L +# include +#elif defined(_MSC_VER) # include #endif @@ -54,11 +57,10 @@ class BitWriter { int offset = 0; unsigned char* out; -public: - BitWriter(unsigned char* output) : out(output) {} - template - inline void Write(I val) { + inline void WriteInner(I val) { + // We right shift by up to 8 bits below. Verify that's well defined for the type I. + static_assert(std::numeric_limits::digits > 8, "BitWriter::WriteInner needs I > 8 bits"); int bits = BITS; if (bits + offset >= 8) { state |= ((val & ((I(1) << (8 - offset)) - 1)) << offset); @@ -77,6 +79,19 @@ class BitWriter { offset += bits; } + +public: + BitWriter(unsigned char* output) : out(output) {} + + template + inline void Write(I val) { + // If I is smaller than an unsigned int, invoke WriteInner with argument converted to unsigned. + using compute_type = typename std::conditional< + (std::numeric_limits::digits < std::numeric_limits::digits), + unsigned, I>::type; + return WriteInner(val); + } + inline void Flush() { if (offset) { *(out++) = state; @@ -129,7 +144,11 @@ constexpr inline I Mask() { return ((I((I(-1)) << (std::numeric_limits::digit /** Compute the smallest power of two that is larger than val. */ template static inline int CountBits(I val, int max) { -#ifdef _MSC_VER +#if defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L + // c++20 impl + (void)max; + return std::bit_width(val); +#elif defined(_MSC_VER) (void)max; unsigned long index; unsigned char ret; @@ -175,6 +194,7 @@ class BitsInt { } static constexpr inline bool IsZero(I a) { return a == 0; } + static constexpr inline bool IsOne(I a) { return a == 1; } static constexpr inline I Mask(I val) { return val & MASK; } static constexpr inline I Shift(I val, int bits) { return ((val << bits) & MASK); } static constexpr inline I UnsafeShift(I val, int bits) { return (val << bits); } @@ -233,7 +253,7 @@ template inline constexpr I GFMul(con template inline I InvExtGCD(I x) { - if (F::IsZero(x)) return x; + if (F::IsZero(x) || F::IsOne(x)) return x; I t(0), newt(1); I r(MOD), newr = x; int rlen = BITS + 1, newrlen = F::Bits(newr, BITS); From 94dca7f9aeb27bbf36c9dc05ece0a1585d92e8cc Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:05:24 +0000 Subject: [PATCH 2/4] merge bitcoin#29823: update subtree to 3472e2f5e excludes: - 1eea10a6 (subtree manipulation done in previous commits) - e58e1323 (see above) --- configure.ac | 18 ------------------ src/Makefile.minisketch.include | 4 ---- 2 files changed, 22 deletions(-) diff --git a/configure.ac b/configure.ac index 10a5a83026491..4081edea605fd 100644 --- a/configure.ac +++ b/configure.ac @@ -1081,23 +1081,6 @@ AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64],,, #include #endif]) -AC_MSG_CHECKING(for __builtin_clzl) - -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ - (void) __builtin_clzl(0); - ]])], - [ AC_MSG_RESULT(yes); have_clzl=yes; AC_DEFINE(HAVE_BUILTIN_CLZL, 1, [Define this symbol if you have __builtin_clzl])], - [ AC_MSG_RESULT(no); have_clzl=no;] -) - -AC_MSG_CHECKING(for __builtin_clzll) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ - (void) __builtin_clzll(0); - ]])], - [ AC_MSG_RESULT(yes); have_clzll=yes; AC_DEFINE(HAVE_BUILTIN_CLZLL, 1, [Define this symbol if you have __builtin_clzll])], - [ AC_MSG_RESULT(no); have_clzll=no;] -) - dnl Check for mallopt(M_ARENA_MAX) (to set glibc arenas) AC_MSG_CHECKING(for mallopt M_ARENA_MAX) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], @@ -1870,7 +1853,6 @@ AM_CONDITIONAL([USE_UPNP],[test x$use_upnp = xyes]) dnl for minisketch AM_CONDITIONAL([ENABLE_CLMUL],[test x$enable_clmul = xyes]) -AM_CONDITIONAL([HAVE_CLZ],[test x$have_clzl$have_clzll = xyesyes]) AC_DEFINE(CLIENT_VERSION_MAJOR, _CLIENT_VERSION_MAJOR, [Major version]) AC_DEFINE(CLIENT_VERSION_MINOR, _CLIENT_VERSION_MINOR, [Minor version]) diff --git a/src/Makefile.minisketch.include b/src/Makefile.minisketch.include index 1363bec34eac2..c6f894f0ca018 100644 --- a/src/Makefile.minisketch.include +++ b/src/Makefile.minisketch.include @@ -12,10 +12,6 @@ LIBMINISKETCH_CPPFLAGS += -DHAVE_CLMUL MINISKETCH_LIBS += $(LIBMINISKETCH_CLMUL) endif -if HAVE_CLZ -LIBMINISKETCH_CPPFLAGS += -DHAVE_CLZ -endif - EXTRA_LIBRARIES += $(MINISKETCH_LIBS) minisketch_libminisketch_clmul_a_SOURCES = $(MINISKETCH_FIELD_CLMUL_SOURCES_INT) $(MINISKETCH_FIELD_CLMUL_HEADERS_INT) From ef10e83d4d56b4c6942aab18095daf9d2be53908 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:16:07 +0000 Subject: [PATCH 3/4] Squashed 'src/minisketch/' changes from 3472e2f5ec..eb37a9b8e7 eb37a9b8e7 Merge sipa/minisketch#87: Avoid copy in self-assign fe6557642e Merge sipa/minisketch#88: build: Add `-Wundef` 8ea298bfa7 Avoid copy in self-assign 978a3d8869 build: Add `-Wundef` 3387044179 Merge sipa/minisketch#86: doc: fix typo in sketch_impl.h 15c2d13b60 doc: fix typo in sketch_impl.h 7be08b8a46 Merge sipa/minisketch#85: Fixes for integer precision loss 00fb4a4d83 Avoid or make integer precision conversion explicit 9d62a4d27c Avoid the need to cast/convert to size_t for vector operations 19e06cc7af Prevent overflows from large capacity/max_elements git-subtree-dir: src/minisketch git-subtree-split: eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c --- configure.ac | 1 + include/minisketch.h | 2 +- src/false_positives.h | 4 +++- src/int_utils.h | 4 ++-- src/minisketch.cpp | 2 +- src/sketch.h | 2 +- src/sketch_impl.h | 11 ++++++----- 7 files changed, 15 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index cd52d7f412a69..65a47b45c28ca 100644 --- a/configure.ac +++ b/configure.ac @@ -102,6 +102,7 @@ case $host in esac AX_CHECK_COMPILE_FLAG([-Wall],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"],,[[$CXXFLAG_WERROR]]) +AX_CHECK_COMPILE_FLAG([-Wundef], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wundef"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-fvisibility=hidden],[CXXFLAGS="$CXXFLAGS -fvisibility=hidden"],[],[$CXXFLAG_WERROR]) if test "x$use_ccache" != "xno"; then diff --git a/include/minisketch.h b/include/minisketch.h index 24d6b4e1c0a83..b0571d2788e31 100644 --- a/include/minisketch.h +++ b/include/minisketch.h @@ -239,7 +239,7 @@ class Minisketch /** Make this Minisketch a clone of the specified one. */ Minisketch& operator=(const Minisketch& sketch) noexcept { - if (sketch.m_minisketch) { + if (this != &sketch && sketch.m_minisketch) { m_minisketch = std::unique_ptr(minisketch_clone(sketch.m_minisketch.get())); } return *this; diff --git a/src/false_positives.h b/src/false_positives.h index 44ebb3e94c3f1..9d0358997f75d 100644 --- a/src/false_positives.h +++ b/src/false_positives.h @@ -81,7 +81,8 @@ uint64_t BaseFPBits(uint32_t bits, uint32_t capacity) { size_t ComputeCapacity(uint32_t bits, size_t max_elements, uint32_t fpbits) { if (bits == 0) return 0; - uint64_t base_fpbits = BaseFPBits(bits, max_elements); + if (max_elements > 0xffffffff) return max_elements; + uint64_t base_fpbits = BaseFPBits(bits, static_cast(max_elements)); // The fpbits provided by the base max_elements==capacity case are sufficient. if (base_fpbits >= fpbits) return max_elements; // Otherwise, increment capacity by ceil(fpbits / bits) beyond that. @@ -90,6 +91,7 @@ size_t ComputeCapacity(uint32_t bits, size_t max_elements, uint32_t fpbits) { size_t ComputeMaxElements(uint32_t bits, size_t capacity, uint32_t fpbits) { if (bits == 0) return 0; + if (capacity > 0xffffffff) return capacity; // Start with max_elements=capacity, and decrease max_elements until the corresponding capacity is capacity. size_t max_elements = capacity; while (true) { diff --git a/src/int_utils.h b/src/int_utils.h index 2b3d8cb402668..a6b89cd63c688 100644 --- a/src/int_utils.h +++ b/src/int_utils.h @@ -159,7 +159,7 @@ static inline int CountBits(I val, int max) { } if (!ret) return 0; return index + 1; -#elif HAVE_CLZ +#elif defined(HAVE_CLZ) (void)max; if (val == 0) return 0; if (std::numeric_limits::digits >= std::numeric_limits::digits) { @@ -210,7 +210,7 @@ class BitsInt { static constexpr inline int TopBits(I val) { static_assert(Count > 0, "BitsInt::TopBits needs Count > 0"); static_assert(Count <= BITS, "BitsInt::TopBits needs Offset <= BITS"); - return val >> (BITS - Count); + return static_cast(val >> (BITS - Count)); } static inline constexpr I CondXorWith(I val, bool cond, I v) { diff --git a/src/minisketch.cpp b/src/minisketch.cpp index d003fdf755632..2e45409243038 100644 --- a/src/minisketch.cpp +++ b/src/minisketch.cpp @@ -468,7 +468,7 @@ size_t minisketch_merge(minisketch* sketch, const minisketch* other_sketch) { ssize_t minisketch_decode(const minisketch* sketch, size_t max_elements, uint64_t* output) { const Sketch* s = (const Sketch*)sketch; s->Check(); - return s->Decode(max_elements, output); + return s->Decode(static_cast(max_elements), output); } void minisketch_set_seed(minisketch* sketch, uint64_t seed) { diff --git a/src/sketch.h b/src/sketch.h index 3e9bad793d521..662b4e982f59e 100644 --- a/src/sketch.h +++ b/src/sketch.h @@ -29,7 +29,7 @@ class Sketch virtual ~Sketch() {} virtual size_t Syndromes() const = 0; - virtual void Init(int syndromes) = 0; + virtual void Init(size_t syndromes) = 0; virtual void Add(uint64_t element) = 0; virtual void Serialize(unsigned char*) const = 0; virtual void Deserialize(const unsigned char*) = 0; diff --git a/src/sketch_impl.h b/src/sketch_impl.h index 4547b742f239f..c357f0e823558 100644 --- a/src/sketch_impl.h +++ b/src/sketch_impl.h @@ -92,7 +92,8 @@ template void Sqr(std::vector& poly, const F& field) { if (poly.size() == 0) return; poly.resize(poly.size() * 2 - 1); - for (int x = poly.size() - 1; x >= 0; --x) { + for (size_t i = 0; i < poly.size(); ++i) { + auto x = poly.size() - i - 1; poly[x] = (x & 1) ? 0 : field.Sqr(poly[x / 2]); } } @@ -217,7 +218,7 @@ bool RecFindRoots(std::vector>& stack, size_t pos, } if (fully_factorizable) { - // Every succesful iteration of this algorithm splits the input + // Every successful iteration of this algorithm splits the input // polynomial further into buckets, each corresponding to a subset // of 2^(BITS-depth) roots. If after depth splits the degree of // the polynomial is >= 2^(BITS-depth), something is wrong. @@ -297,7 +298,7 @@ std::vector BerlekampMassey(const std::vector(n + 1 - (current.size() - 1) - (prev.size() - 1)); if (!b_have_inv) { b_inv = field.Inv(b); b_have_inv = true; @@ -366,7 +367,7 @@ class SketchImpl final : public Sketch } size_t Syndromes() const override { return m_syndromes.size(); } - void Init(int count) override { m_syndromes.assign(count, 0); } + void Init(size_t count) override { m_syndromes.assign(count, 0); } void Add(uint64_t val) override { @@ -405,7 +406,7 @@ class SketchImpl final : public Sketch for (const auto& root : roots) { *(out++) = m_field.ToUint64(root); } - return roots.size(); + return static_cast(roots.size()); } size_t Merge(const Sketch* other_sketch) override From 5e65bb45a7296461fb86c65193c8281de82497e9 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 29 Oct 2024 10:17:28 +0000 Subject: [PATCH 4/4] merge bitcoin#30270: update subtree to eb37a9b8 excludes: - cb59af38 (subtree manipulation done in previous commits) - 89464ad5 (see above)