Merge #6372: backport: merge bitcoin#29823, #30270 (update 'src/minisketch' to sipa/minisketch@eb37a9b8)

5e65bb45a7 merge bitcoin#30270: update subtree to eb37a9b8 (Kittywhiskers Van Gogh) ef10e83d4d Squashed 'src/minisketch/' changes from 3472e2f5ec..eb37a9b8e7 (Kittywhiskers Van Gogh) 94dca7f9ae merge bitcoin#29823: update subtree to 3472e2f5e (Kittywhiskers Van Gogh) 9540ecbc34 Squashed 'src/minisketch/' changes from a571ba20f9..3472e2f5ec (Kittywhiskers Van Gogh) Pull request description: ## Additional information Unexpected failure was found in UBSan unit test run originating from minisketch tests ([build](https://gitlab.com/dashpay/dash/-/jobs/8206813511#L3512)), resolved in subtree with https://github.com/sipa/minisketch/pull/81 and updated upstream in https://github.com/bitcoin/bitcoin/pull/29823. This pull request updates the subtree to latest subtree pulls done on upstream `master` (as of this writing, [`da10e0ba`](da10e0bab4)). Special thanks to knst for reporting this bug! 🎉 ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 5e65bb45a7 Tree-SHA512: be3c75436425c8662d6af46641a6fc744d01043a832884b29aa9767dd4b9090cef93bcb31355032131392a6ccf29cbbcb771a5786c654f26f4fa0a2d5f0e8a5f
2024-12-24 19:42:46 +01:00 · 2024-10-29 13:02:57 -05:00 · 2024-10-29 13:02:57 -05:00 · 02ec0fd284
commit 02ec0fd284
parent e3a0fce5c9 5e65bb45a7
12 changed files with 79 additions and 80 deletions
--- a/configure.ac
+++ b/configure.ac
@ -1081,23 +1081,6 @@ AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64],,,
                 #include <byteswap.h>
                 #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 <malloc.h>]],
@ -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])
--- 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)
--- a/src/minisketch/.cirrus.yml
+++ b/src/minisketch/.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
--- a/src/minisketch/ci/cirrus.sh
+++ b/src/minisketch/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
--- a/src/minisketch/ci/linux-debian.Dockerfile
+++ b/src/minisketch/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
--- a/src/minisketch/configure.ac
+++ b/src/minisketch/configure.ac
@ -102,13 +102,9 @@ 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])

-## 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 +115,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)
--- a/src/minisketch/include/minisketch.h
+++ b/src/minisketch/include/minisketch.h
@ -239,7 +239,7 @@ public:
    /** 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, Deleter>(minisketch_clone(sketch.m_minisketch.get()));
        }
        return *this;
--- a/src/minisketch/src/false_positives.h
+++ b/src/minisketch/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<uint32_t>(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) {
--- a/src/minisketch/src/int_utils.h
+++ b/src/minisketch/src/int_utils.h
@ -7,13 +7,16 @@
 #ifndef _MINISKETCH_INT_UTILS_H_
 #define _MINISKETCH_INT_UTILS_H_

+#include <stdint.h>
 #include <stdlib.h>

 #include <limits>
 #include <algorithm>
 #include <type_traits>

-#ifdef _MSC_VER
+#if defined(__cpp_lib_int_pow2) && __cpp_lib_int_pow2 >= 202002L
+#  include <bit>
+#elif defined(_MSC_VER)
 #  include <intrin.h>
 #endif

@ -54,11 +57,10 @@ class BitWriter {
    int offset = 0;
    unsigned char* out;

-public:
-    BitWriter(unsigned char* output) : out(output) {}
-
    template<int BITS, typename I>
-    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<I>::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 @@ public:
        offset += bits;
    }

+
+public:
+    BitWriter(unsigned char* output) : out(output) {}
+
+    template<int BITS, typename I>
+    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<I>::digits < std::numeric_limits<unsigned>::digits),
+            unsigned, I>::type;
+        return WriteInner<BITS, compute_type>(val);
+    }
+
    inline void Flush() {
        if (offset) {
            *(out++) = state;
@ -129,7 +144,11 @@ constexpr inline I Mask() { return ((I((I(-1)) << (std::numeric_limits<I>::digit
 /** Compute the smallest power of two that is larger than val. */
 template<typename I>
 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;
@ -140,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<unsigned>::digits >= std::numeric_limits<I>::digits) {
@ -175,6 +194,7 @@ public:
    }

    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); }
@ -190,7 +210,7 @@ public:
    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<int>(val >> (BITS - Count));
    }

    static inline constexpr I CondXorWith(I val, bool cond, I v) {
@ -233,7 +253,7 @@ template<typename I, int N, typename L, typename F> inline constexpr I GFMul(con
 template<typename I, typename F, int BITS, uint32_t MOD>
 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);
--- a/src/minisketch/src/minisketch.cpp
+++ b/src/minisketch/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<int>(max_elements), output);
 }

 void minisketch_set_seed(minisketch* sketch, uint64_t seed) {
--- a/src/minisketch/src/sketch.h
+++ b/src/minisketch/src/sketch.h
@ -29,7 +29,7 @@ public:
    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;
--- a/src/minisketch/src/sketch_impl.h
+++ b/src/minisketch/src/sketch_impl.h
@ -92,7 +92,8 @@ template<typename F>
 void Sqr(std::vector<typename F::Elem>& 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<std::vector<typename F::Elem>>& 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<typename F::Elem> BerlekampMassey(const std::vector<typename F::Elem
        auto discrepancy = syndromes[n];
        for (size_t i = 1; i < current.size(); ++i) discrepancy ^= table[n - i](current[i]);
        if (discrepancy != 0) {
-            int x = n + 1 - (current.size() - 1) - (prev.size() - 1);
+            int x = static_cast<int>(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 @@ public:
    }

    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 @@ public:
        for (const auto& root : roots) {
            *(out++) = m_field.ToUint64(root);
        }
-        return roots.size();
+        return static_cast<int>(roots.size());
    }

    size_t Merge(const Sketch* other_sketch) override