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) diff --git a/src/minisketch/.cirrus.yml b/src/minisketch/.cirrus.yml index 4a5353f1373c9..5ceefee2cffa3 100644 --- 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 diff --git a/src/minisketch/ci/cirrus.sh b/src/minisketch/ci/cirrus.sh index 02f737ca7f2eb..36250d165195d 100755 --- 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 diff --git a/src/minisketch/ci/linux-debian.Dockerfile b/src/minisketch/ci/linux-debian.Dockerfile index 63e5412ee79cc..122af36e1f044 100644 --- 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 diff --git a/src/minisketch/configure.ac b/src/minisketch/configure.ac index 83910448a2c2a..65a47b45c28ca 100644 --- 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) diff --git a/src/minisketch/include/minisketch.h b/src/minisketch/include/minisketch.h index 24d6b4e1c0a83..b0571d2788e31 100644 --- a/src/minisketch/include/minisketch.h +++ b/src/minisketch/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/minisketch/src/false_positives.h b/src/minisketch/src/false_positives.h index 44ebb3e94c3f1..9d0358997f75d 100644 --- 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(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/minisketch/src/int_utils.h b/src/minisketch/src/int_utils.h index d21ba56f336c0..a6b89cd63c688 100644 --- 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 #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; @@ -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::digits >= std::numeric_limits::digits) { @@ -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); } @@ -190,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) { @@ -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); diff --git a/src/minisketch/src/minisketch.cpp b/src/minisketch/src/minisketch.cpp index d003fdf755632..2e45409243038 100644 --- 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(max_elements), output); } void minisketch_set_seed(minisketch* sketch, uint64_t seed) { diff --git a/src/minisketch/src/sketch.h b/src/minisketch/src/sketch.h index 3e9bad793d521..662b4e982f59e 100644 --- a/src/minisketch/src/sketch.h +++ b/src/minisketch/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/minisketch/src/sketch_impl.h b/src/minisketch/src/sketch_impl.h index 4547b742f239f..c357f0e823558 100644 --- a/src/minisketch/src/sketch_impl.h +++ b/src/minisketch/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