CI updates

.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,

.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

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

src/check/dt_impl.cpp

@@ -68,7 +68,9 @@
 #endif
 #endif
 
+// NOLINTBEGIN(bugprone-unintended-char-ostream-output)
 #include <doctest/doctest/parts/doctest.cpp>
+// NOLINTEND(bugprone-unintended-char-ostream-output)
 
 #endif // DOCTEST_CONFIG_DISABLE
 

src/check/dt_xspan.cpp

@@ -870,6 +870,118 @@ TEST_CASE("PtrOrSpan int") {
     CHECK(a == buf + 8);
 }
 
+/*************************************************************************
+//
+**************************************************************************/
+
+#ifdef UPX_VERSION_HEX
+
+namespace {
+template <class T>
+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
 **************************************************************************/

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;

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));

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<byte>() {
 
 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) {
+    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 {
         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<byte *>("", 0));
         CHECK_NOTHROW(mb.subref_u<byte *>("", i - 1));
         CHECK_THROWS(mb.subref_u<byte *>("", i));
         CHECK_THROWS(mb.subref_u<byte *>("", (size_t) -1));
-        //
+
         if (i < 2) {
             CHECK_THROWS(mb.subref_u<NE16 *>("", 0));
             CHECK_THROWS(mb.subref_u<BE16 *>("", 0));
@@ -454,8 +456,8 @@ TEST_CASE("MemBuffer global overloads") {
             CHECK_NOTHROW(mb.subref_u<BE64 *>("", 0));
             CHECK_NOTHROW(mb.subref_u<LE64 *>("", 0));
         }
-    }
 #endif
+    }
 }
 
 TEST_CASE("MemBuffer array access") {

src/util/membuffer.h

@@ -43,7 +43,7 @@ public:
     typedef typename std::add_pointer<T>::type pointer;
     typedef pointer iterator;
     typedef typename std::add_pointer<const T>::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;

src/util/xspan_fwd.h

@@ -92,7 +92,7 @@ XSPAN_FWD_TU_VOIDPTR(int) memcmp(const C<T> &a, const E<U> &b, size_t n) {
 #endif
 
 template <class T>
-inline void *memcpy(C<T> a, const void *b, size_t n) {
+inline void *memcpy(const C<T> a, const void *b, size_t n) {
     return memcpy(a.raw_bytes(n), b, n);
 }
 template <class T>
@@ -114,7 +114,7 @@ XSPAN_FWD_TU_VOIDPTR(void *) memcpy(const C<T> &a, const E<U> &b, size_t n) {
 #endif
 
 template <class T>
-inline void *memmove(C<T> a, const void *b, size_t n) {
+inline void *memmove(const C<T> a, const void *b, size_t n) {
     return memmove(a.raw_bytes(n), b, n);
 }
 template <class T>

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

src/util/xspan_impl_ptr.h

@@ -81,6 +81,7 @@ public:
         assertInvariants();
     }
 
+    // inline CSelf() : ptr(nullptr) { invalidate(); }
     inline CSelf() { assertInvariants(); }
 
     // constructors from pointers