From 8622e22be0cfb3c630ce995480ee669aaa865b79 Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Tue, 2 Dec 2025 13:54:23 +0100 Subject: [PATCH] CI updates --- .clang-tidy | 1 + .github/workflows/ci.yml | 6 ++ misc/testsuite/mimic_ctest.sh | 1 + src/check/dt_impl.cpp | 2 + src/check/dt_xspan.cpp | 112 +++++++++++++++++++++++++++++ src/conf.h | 3 +- src/p_lx_elf.cpp | 1 + src/util/membuffer.cpp | 132 +++++++++++++++++----------------- src/util/membuffer.h | 8 +-- src/util/xspan_fwd.h | 4 +- src/util/xspan_impl_common.h | 4 +- src/util/xspan_impl_ptr.h | 1 + 12 files changed, 201 insertions(+), 74 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index e6f5f359..0ab1ec1e 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -20,6 +20,7 @@ Checks: > -bugprone-suspicious-memory-comparison, -bugprone-suspicious-string-compare, -bugprone-switch-missing-default-case, + -bugprone-unintended-char-ostream-output, clang-analyzer-*, -clang-analyzer-optin.performance.Padding, -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c13694fc..d3a670a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -148,6 +148,8 @@ jobs: jobs="gcc/debug gcc/release clang/release" echo "===== parallel jobs: $jobs" parallel -kv --lb 'cd build/extra/{} && bash ../../../../misc/testsuite/mimic_ctest.sh' ::: $jobs + ls -ld ./build/*/*/*/upx-* || true + rm -rf ./build/*/*/*/upx-* job-linux-cmake: # uses cmake + make if: true @@ -267,6 +269,8 @@ jobs: test "${{ matrix.use_extra }}" = "true" && jobs="$jobs gcc-m32/debug gcc-m32/release" echo "===== parallel jobs: $jobs" parallel -kv --lb 'cd build/extra/{} && bash ../../../../misc/testsuite/mimic_ctest.sh' ::: $jobs + ls -ld ./build/*/*/*/upx-* || true + rm -rf ./build/*/*/*/upx-* fi - name: Run file system tests run: | @@ -292,6 +296,8 @@ jobs: test "${{ matrix.use_extra }}" = "true" && jobs="$jobs gcc-m32/debug gcc-m32/release" echo "===== parallel jobs: $jobs" parallel -kv --lb 'cd build/extra/{} && bash ../../../../misc/testsuite/upx_testsuite_1.sh' ::: $jobs + ls -ld ./build/*/*/*/tmp-upx-testsuite* || true + rm -rf ./build/*/*/*/tmp-upx-testsuite* job-macos-cmake: # uses cmake + make if: true diff --git a/misc/testsuite/mimic_ctest.sh b/misc/testsuite/mimic_ctest.sh index 8cdacd94..ec78ad18 100755 --- a/misc/testsuite/mimic_ctest.sh +++ b/misc/testsuite/mimic_ctest.sh @@ -48,6 +48,7 @@ run_upx=( "${emu[@]}" "$upx_exe" ) echo "run_upx='${run_upx[*]}'" # run_upx sanity check +"${run_upx[@]}" --version-short || true if ! "${run_upx[@]}" --version-short >/dev/null; then echo "UPX-ERROR: FATAL: upx --version-short FAILED"; exit 1; fi if ! "${run_upx[@]}" -L >/dev/null 2>&1; then echo "UPX-ERROR: FATAL: upx -L FAILED"; exit 1; fi if ! "${run_upx[@]}" --help >/dev/null; then echo "UPX-ERROR: FATAL: upx --help FAILED"; exit 1; fi diff --git a/src/check/dt_impl.cpp b/src/check/dt_impl.cpp index 03535f17..c0ec7a7c 100644 --- a/src/check/dt_impl.cpp +++ b/src/check/dt_impl.cpp @@ -68,7 +68,9 @@ #endif #endif +// NOLINTBEGIN(bugprone-unintended-char-ostream-output) #include +// NOLINTEND(bugprone-unintended-char-ostream-output) #endif // DOCTEST_CONFIG_DISABLE diff --git a/src/check/dt_xspan.cpp b/src/check/dt_xspan.cpp index c70c38b7..d9bffd47 100644 --- a/src/check/dt_xspan.cpp +++ b/src/check/dt_xspan.cpp @@ -870,6 +870,118 @@ TEST_CASE("PtrOrSpan int") { CHECK(a == buf + 8); } +/************************************************************************* +// +**************************************************************************/ + +#ifdef UPX_VERSION_HEX + +namespace { +template +static noinline void check_bele(const T &mb, size_t i) { + if (i < 2) { + CHECK_THROWS(get_ne16(mb)); + CHECK_THROWS(get_be16(mb)); + CHECK_THROWS(get_le16(mb)); + CHECK_THROWS(set_ne16(mb, 0)); + CHECK_THROWS(set_be16(mb, 0)); + CHECK_THROWS(set_le16(mb, 0)); + } else { + CHECK_NOTHROW(get_ne16(mb)); + CHECK_NOTHROW(get_be16(mb)); + CHECK_NOTHROW(get_le16(mb)); + CHECK_NOTHROW(set_ne16(mb, 0)); + CHECK_NOTHROW(set_be16(mb, 0)); + CHECK_NOTHROW(set_le16(mb, 0)); + } + if (i < 3) { + CHECK_THROWS(get_ne24(mb)); + CHECK_THROWS(get_be24(mb)); + CHECK_THROWS(get_le24(mb)); + CHECK_THROWS(set_ne24(mb, 0)); + CHECK_THROWS(set_be24(mb, 0)); + CHECK_THROWS(set_le24(mb, 0)); + } else { + CHECK_NOTHROW(get_ne24(mb)); + CHECK_NOTHROW(get_be24(mb)); + CHECK_NOTHROW(get_le24(mb)); + CHECK_NOTHROW(set_ne24(mb, 0)); + CHECK_NOTHROW(set_be24(mb, 0)); + CHECK_NOTHROW(set_le24(mb, 0)); + } + if (i < 4) { + CHECK_THROWS(get_ne32(mb)); + CHECK_THROWS(get_be32(mb)); + CHECK_THROWS(get_le32(mb)); + CHECK_THROWS(set_ne32(mb, 0)); + CHECK_THROWS(set_be32(mb, 0)); + CHECK_THROWS(set_le32(mb, 0)); + } else { + CHECK_NOTHROW(get_ne32(mb)); + CHECK_NOTHROW(get_be32(mb)); + CHECK_NOTHROW(get_le32(mb)); + CHECK_NOTHROW(set_ne32(mb, 0)); + CHECK_NOTHROW(set_be32(mb, 0)); + CHECK_NOTHROW(set_le32(mb, 0)); + } + if (i < 8) { + CHECK_THROWS(get_ne64(mb)); + CHECK_THROWS(get_be64(mb)); + CHECK_THROWS(get_le64(mb)); + CHECK_THROWS(set_ne64(mb, 0)); + CHECK_THROWS(set_be64(mb, 0)); + CHECK_THROWS(set_le64(mb, 0)); + } else { + CHECK_NOTHROW(get_ne64(mb)); + CHECK_NOTHROW(get_be64(mb)); + CHECK_NOTHROW(get_le64(mb)); + CHECK_NOTHROW(set_ne64(mb, 0)); + CHECK_NOTHROW(set_be64(mb, 0)); + CHECK_NOTHROW(set_le64(mb, 0)); + } +} +} // namespace + +#endif // UPX_VERSION_HEX + +TEST_CASE("xspan global overloads") { + byte buf[16] = {}; + + for (size_t i = 0; i < 16; i++) { + XSPAN_0_VAR(byte, a0, buf, i); + XSPAN_P_VAR(byte, ap, buf, i); + XSPAN_S_VAR(byte, as, buf, i); + +#ifdef UPX_VERSION_HEX + check_bele(a0, i); + check_bele(ap, i); + check_bele(as, i); + CHECK_THROWS(upx_adler32(a0, i + 1)); + CHECK_THROWS(upx_adler32(ap, i + 1)); + CHECK_THROWS(upx_adler32(as, i + 1)); + if (i == 0) { + CHECK_THROWS(upx_safe_strlen(a0)); + CHECK_THROWS(upx_safe_strlen(ap)); + CHECK_THROWS(upx_safe_strlen(as)); + } +#endif + + CHECK(memcmp(a0, a0, i) == 0); + CHECK(memcmp(ap, a0, i) == 0); + CHECK(memcmp(as, a0, i) == 0); + CHECK_THROWS(memcmp(a0, a0, i + 1)); // NOLINT(bugprone-unused-return-value) + CHECK_THROWS(memcmp(ap, a0, i + 1)); // NOLINT(bugprone-unused-return-value) + CHECK_THROWS(memcmp(as, a0, i + 1)); // NOLINT(bugprone-unused-return-value) + + CHECK_NOTHROW(memset(a0, 255, i)); + CHECK_NOTHROW(memset(ap, 255, i)); + CHECK_NOTHROW(memset(as, 255, i)); + CHECK_THROWS(memset(a0, 254, i + 1)); + CHECK_THROWS(memset(ap, 254, i + 1)); + CHECK_THROWS(memset(as, 254, i + 1)); + } +} + /************************************************************************* // codegen **************************************************************************/ diff --git a/src/conf.h b/src/conf.h index 34021df3..cdd0d6f8 100644 --- a/src/conf.h +++ b/src/conf.h @@ -100,6 +100,7 @@ static_assert((char) (-1) == 255); // -funsigned-char #endif // UPX_CONFIG_DISABLE_WERROR #endif // UPX_CONFIG_DISABLE_WSTRICT +// upx_is_constant_evaluated #if __cplusplus >= 202002L // C++20 #define upx_is_constant_evaluated std::is_constant_evaluated #elif __has_builtin(__builtin_is_constant_evaluated) // clang-9, gcc-10 @@ -802,7 +803,7 @@ int upx_doctest_check(); // util/membuffer.h class MemBuffer; void *membuffer_get_void_ptr(MemBuffer &mb) noexcept; -unsigned membuffer_get_size(MemBuffer &mb) noexcept; +unsigned membuffer_get_size_in_bytes(MemBuffer &mb) noexcept; // main.cpp extern const char *progname; diff --git a/src/p_lx_elf.cpp b/src/p_lx_elf.cpp index e19f2103..ebb1a78b 100644 --- a/src/p_lx_elf.cpp +++ b/src/p_lx_elf.cpp @@ -6279,6 +6279,7 @@ unsigned PackLinuxElf32::forward_Shdrs(OutputFile *fo, Elf32_Ehdr *const eho) } } } + UNUSED(ptr); fo->seek(0, SEEK_SET); fo->rewrite(eho, sizeof(*eho)); diff --git a/src/util/membuffer.cpp b/src/util/membuffer.cpp index ad9380dc..4af2ef57 100644 --- a/src/util/membuffer.cpp +++ b/src/util/membuffer.cpp @@ -33,7 +33,7 @@ // extra functions to reduce dependency on membuffer.h void *membuffer_get_void_ptr(MemBuffer &mb) noexcept { return mb.getVoidPtr(); } -unsigned membuffer_get_size(MemBuffer &mb) noexcept { return mb.getSize(); } +unsigned membuffer_get_size_in_bytes(MemBuffer &mb) noexcept { return mb.getSizeInBytes(); } /*static*/ MemBuffer::Stats MemBuffer::stats; @@ -61,7 +61,7 @@ static noinline void init_use_simple_mcheck() noexcept { } use_simple_mcheck_flag = r; } -static bool use_simple_mcheck() noexcept { +static noinline bool use_simple_mcheck() noexcept { static upx_std_once_flag init_done; upx_std_call_once(init_done, init_use_simple_mcheck); // NOTE: clang-analyzer-unix.Malloc does not know that this flag is "constant"; see below @@ -83,23 +83,6 @@ MemBuffer::MemBuffer(upx_uint64_t bytes) may_throw : MemBufferBase() { MemBuffer::~MemBuffer() noexcept { this->dealloc(); } -// similar to BoundedPtr, except checks only at creation -// skip == offset, take == size_in_bytes -void *MemBuffer::subref_impl(const char *errfmt, size_t skip, size_t take) { - debug_set(debug.last_return_address_subref, upx_return_address()); - checkState(); - // check overrun and wrap-around - if (skip + take > size_in_bytes || skip + take < skip) { - char buf[100]; - // printf is using unsigned formatting - if (!errfmt || !errfmt[0]) - errfmt = "bad subref %#x %#x"; - upx_safe_snprintf(buf, sizeof(buf), errfmt, (unsigned) skip, (unsigned) take); - throwCantPack(buf); - } - return ptr + skip; -} - /*static*/ unsigned MemBuffer::getSizeForCompression(unsigned uncompressed_size, unsigned extra) { if (uncompressed_size == 0) @@ -138,12 +121,30 @@ void MemBuffer::allocForDecompression(unsigned uncompressed_size, unsigned extra void MemBuffer::fill(size_t off, size_t bytes, int value) { debug_set(debug.last_return_address_fill, upx_return_address()); checkState(); - if (off > size_in_bytes || bytes > size_in_bytes || off + bytes > size_in_bytes) + // check overrun and wrap-around + if very_unlikely (off + bytes > size_in_bytes || off + bytes < off) throwCantPack("MemBuffer::fill out of range; take care!"); if (bytes > 0) memset(ptr + off, value, bytes); } +// similar to BoundedPtr, except checks only at creation +// skip == offset, take == size_in_bytes +void *MemBuffer::subref_impl(const char *errfmt, size_t skip, size_t take) { + debug_set(debug.last_return_address_subref, upx_return_address()); + checkState(); + // check overrun and wrap-around + if very_unlikely (skip + take > size_in_bytes || skip + take < skip) { + char buf[100]; + // printf is using unsigned formatting + if (!errfmt || !errfmt[0]) + errfmt = "bad subref %#x %#x"; + upx_safe_snprintf(buf, sizeof(buf), errfmt, (unsigned) skip, (unsigned) take); + throwCantPack(buf); + } + return ptr + skip; +} + /************************************************************************* // **************************************************************************/ @@ -154,16 +155,16 @@ void MemBuffer::fill(size_t off, size_t bytes, int value) { #define MAGIC2(p) ((PTR_BITS32(p) ^ 0xfefdbeeb ^ 0x88224411) | 1) void MemBuffer::checkState() const may_throw { - if (!ptr) + if very_unlikely (ptr == nullptr) throwInternalError("block not allocated"); assert(size_in_bytes > 0); if (use_simple_mcheck()) { const byte *p = (const byte *) ptr; - if (get_ne32(p - 4) != MAGIC1(p)) + if very_unlikely (get_ne32(p - 4) != MAGIC1(p)) throwInternalError("memory clobbered before allocated block 1"); - if (get_ne32(p - 8) != size_in_bytes) + if very_unlikely (get_ne32(p - 8) != size_in_bytes) throwInternalError("memory clobbered before allocated block 2"); - if (get_ne32(p + size_in_bytes) != MAGIC2(p)) + if very_unlikely (get_ne32(p + size_in_bytes) != MAGIC2(p)) throwInternalError("memory clobbered past end of allocated block"); } } @@ -207,43 +208,43 @@ void MemBuffer::alloc(upx_uint64_t bytes) may_throw { } void MemBuffer::dealloc() noexcept { - if (ptr != nullptr) { - debug_set(debug.last_return_address_dealloc, upx_return_address()); -#if DEBUG || 1 - // info: calling checkState() here violates "noexcept", so we need a try block - bool shall_check = true; - // bool shall_check = (std::uncaught_exceptions() == 0); // only if not unwinding - // TODO later: add a priority() method to class Throwable - if (shall_check) { - try { - checkState(); - } catch (const Throwable &e) { - printErr("unknown", e); - std::terminate(); - } catch (...) { - std::terminate(); - } - } -#endif - stats.global_dealloc_counter += 1; - stats.global_total_active_bytes -= size_in_bytes; - if (use_simple_mcheck()) { - byte *p = (byte *) ptr; - // clear magic constants - set_ne32(p - 8, 0); - set_ne32(p - 4, 0); - set_ne32(p + size_in_bytes, 0); - set_ne32(p + size_in_bytes + 4, 0); - // - ::free(p - 16); // NOLINT(clang-analyzer-unix.Malloc) // see NOTE above - } else { - ::free(ptr); // NOLINT(clang-analyzer-unix.Malloc) // see NOTE above - } - ptr = nullptr; - size_in_bytes = 0; - } else { + if (ptr == nullptr) { assert_noexcept(size_in_bytes == 0); + return; } + debug_set(debug.last_return_address_dealloc, upx_return_address()); +#if DEBUG || 1 + // info: calling checkState() here violates "noexcept", so we need a try block + bool shall_check = true; + // bool shall_check = (std::uncaught_exceptions() == 0); // only if not unwinding + // TODO later: add a priority() method to class Throwable + if (shall_check) { + try { + checkState(); + } catch (const Throwable &e) { + printErr("unknown", e); + std::terminate(); + } catch (...) { + std::terminate(); + } + } +#endif + stats.global_dealloc_counter += 1; + stats.global_total_active_bytes -= size_in_bytes; + if (use_simple_mcheck()) { + byte *p = (byte *) ptr; + // clear magic constants + set_ne32(p - 8, 0); + set_ne32(p - 4, 0); + set_ne32(p + size_in_bytes, 0); + set_ne32(p + size_in_bytes + 4, 0); + // + ::free(p - 16); // NOLINT(clang-analyzer-unix.Malloc) // see NOTE above + } else { + ::free(ptr); // NOLINT(clang-analyzer-unix.Malloc) // see NOTE above + } + ptr = nullptr; + size_in_bytes = 0; } /************************************************************************* @@ -319,7 +320,6 @@ TEST_CASE("MemBuffer global overloads") { CHECK(mb1[0] == 255); } -#if DEBUG || 0 for (size_t i = 1; i <= 16; i++) { MemBuffer mb(i); mb.clear(); @@ -383,7 +383,7 @@ TEST_CASE("MemBuffer global overloads") { CHECK_NOTHROW(set_be64(mb, 0)); CHECK_NOTHROW(set_le64(mb, 0)); } - // + CHECK_NOTHROW(mb.subref("", 0, 0)); CHECK_NOTHROW(mb.subref("", 0, i)); CHECK_NOTHROW(mb.subref("", i, 0)); @@ -393,7 +393,9 @@ TEST_CASE("MemBuffer global overloads") { CHECK_THROWS(mb.subref("", i, 1)); CHECK_THROWS(mb.subref("", (size_t) -1, 0)); CHECK_THROWS(mb.subref("", (size_t) -1, i)); - // + +#if DEBUG || !(ACC_CC_CLANG && __PPC64__ && ACC_ABI_BIG_ENDIAN) || 0 + // @COMPILER_BUG @CLANG_BUG if (i < 2) { CHECK_THROWS(mb.subref("", 0, sizeof(NE16))); CHECK_THROWS(mb.subref("", 0, sizeof(BE16))); @@ -421,12 +423,12 @@ TEST_CASE("MemBuffer global overloads") { CHECK_NOTHROW(mb.subref("", 0, sizeof(BE64))); CHECK_NOTHROW(mb.subref("", 0, sizeof(LE64))); } - // + CHECK_NOTHROW(mb.subref_u("", 0)); CHECK_NOTHROW(mb.subref_u("", i - 1)); CHECK_THROWS(mb.subref_u("", i)); CHECK_THROWS(mb.subref_u("", (size_t) -1)); - // + if (i < 2) { CHECK_THROWS(mb.subref_u("", 0)); CHECK_THROWS(mb.subref_u("", 0)); @@ -454,8 +456,8 @@ TEST_CASE("MemBuffer global overloads") { CHECK_NOTHROW(mb.subref_u("", 0)); CHECK_NOTHROW(mb.subref_u("", 0)); } - } #endif + } } TEST_CASE("MemBuffer array access") { diff --git a/src/util/membuffer.h b/src/util/membuffer.h index 58366659..1a3f21f9 100644 --- a/src/util/membuffer.h +++ b/src/util/membuffer.h @@ -43,7 +43,7 @@ public: typedef typename std::add_pointer::type pointer; typedef pointer iterator; typedef typename std::add_pointer::type const_iterator; - typedef unsigned size_type; // limited by UPX_RSIZE_MAX + typedef size_t size_type; // limited by UPX_RSIZE_MAX protected: static const size_t element_size = sizeof(element_type); @@ -63,7 +63,7 @@ public: reference operator[](ptrdiff_t i) const may_throw { // NOTE: &array[SIZE] is *not* legal for containers like std::vector and MemBuffer ! if very_unlikely (i < 0 || mem_size(element_size, i) + element_size > size_in_bytes) - throwCantPack("MemBuffer invalid array index %td (%u bytes)", i, size_in_bytes); + throwCantPack("MemBuffer invalid array index %td (%zu bytes)", i, size_in_bytes); return ptr[i]; } // dereference @@ -199,13 +199,13 @@ public: void allocForDecompression(unsigned uncompressed_size, unsigned extra = 0) may_throw; noinline void dealloc() noexcept; - void checkState() const may_throw; + noinline void checkState() const may_throw; // explicit conversion void *getVoidPtr() noexcept { return (void *) ptr; } const void *getVoidPtr() const noexcept { return (const void *) ptr; } unsigned getSizeInBytes() const noexcept { return size_in_bytes; } - unsigned getSize() const noexcept { return size_in_bytes; } // note: element_size == 1 + unsigned getSize() const noexcept { return size_in_bytes / element_size; } // util noinline void fill(size_t off, size_t bytes, int value) may_throw; diff --git a/src/util/xspan_fwd.h b/src/util/xspan_fwd.h index ffe79cde..e08d95a8 100644 --- a/src/util/xspan_fwd.h +++ b/src/util/xspan_fwd.h @@ -92,7 +92,7 @@ XSPAN_FWD_TU_VOIDPTR(int) memcmp(const C &a, const E &b, size_t n) { #endif template -inline void *memcpy(C a, const void *b, size_t n) { +inline void *memcpy(const C a, const void *b, size_t n) { return memcpy(a.raw_bytes(n), b, n); } template @@ -114,7 +114,7 @@ XSPAN_FWD_TU_VOIDPTR(void *) memcpy(const C &a, const E &b, size_t n) { #endif template -inline void *memmove(C a, const void *b, size_t n) { +inline void *memmove(const C a, const void *b, size_t n) { return memmove(a.raw_bytes(n), b, n); } template diff --git a/src/util/xspan_impl_common.h b/src/util/xspan_impl_common.h index 3b517522..fa81b977 100644 --- a/src/util/xspan_impl_common.h +++ b/src/util/xspan_impl_common.h @@ -172,9 +172,9 @@ forceinline ~CSelf() noexcept {} // constructors from MemBuffer CSelf(MemBuffer &mb) : CSelf(makeNotNull((pointer) membuffer_get_void_ptr(mb)), - XSpanSizeInBytes(membuffer_get_size(mb))) {} + XSpanSizeInBytes(membuffer_get_size_in_bytes(mb))) {} CSelf(pointer first, MemBuffer &mb) - : CSelf(first, XSpanSizeInBytes(membuffer_get_size(mb)), + : CSelf(first, XSpanSizeInBytes(membuffer_get_size_in_bytes(mb)), makeNotNull((pointer) membuffer_get_void_ptr(mb))) {} CSelf(std::nullptr_t, MemBuffer &) XSPAN_DELETED_FUNCTION; #endif diff --git a/src/util/xspan_impl_ptr.h b/src/util/xspan_impl_ptr.h index 4e29c325..5a102ac6 100644 --- a/src/util/xspan_impl_ptr.h +++ b/src/util/xspan_impl_ptr.h @@ -81,6 +81,7 @@ public: assertInvariants(); } + // inline CSelf() : ptr(nullptr) { invalidate(); } inline CSelf() { assertInvariants(); } // constructors from pointers