all: more assorted cleanups

.github/workflows/ci.yml

@@ -455,8 +455,8 @@ jobs:
           - { zig_target: i386-linux-musl, qemu: qemu-i386 }
           # { zig_target: i386-linux-musl, qemu: qemu-i386, zig_pic: -fPIE }
           - { zig_target: i386-windows-gnu }
-          - { zig_target: mips-linux-musl }
+          - { zig_target: mips-linux-musl }   # TODO: qemu
-          - { zig_target: mipsel-linux-musl }
+          - { zig_target: mipsel-linux-musl } # TODO: qemu
           - { zig_target: powerpc-linux-musl, qemu: qemu-ppc }
           - { zig_target: powerpc64-linux-musl, qemu: qemu-ppc64 }
           - { zig_target: powerpc64le-linux-musl, qemu: qemu-ppc64le }

src/check/dt_cxxlib.cpp

@@ -131,6 +131,7 @@ TEST_CASE("libc++") {
     CHECK(v.end() - v.begin() == N);
     CHECK(&v[0] == &(*(v.begin())));
     // CHECK(&v[0] + N == &(*(v.end()))); // TODO later: is this legal??
+    // TODO later
 #if defined(_LIBCPP_HARDENING_MODE_DEBUG) &&                                                       \
     (_LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG)
     CHECK_THROWS((void) &v[N]);
@@ -259,6 +260,41 @@ struct Z2_X2 : public X2 {
 // util
 **************************************************************************/
 
+TEST_CASE("Deleter") {
+    LE16 *o = {}; // object
+    LE32 *a = {}; // array
+    {
+        const upx::ObjectDeleter<LE16 **> o_deleter{&o, 1};
+        o = new LE16;
+        assert(o != nullptr);
+        const upx::ArrayDeleter<LE32 **> a_deleter{&a, 1};
+        a = New(LE32, 1);
+        assert(a != nullptr);
+    }
+    assert(o == nullptr);
+    assert(a == nullptr);
+}
+
+TEST_CASE("Deleter") {
+    constexpr size_t N = 2;
+    BE16 *o[N]; // multiple objects
+    BE32 *a[N]; // multiple arrays
+    {
+        upx::ObjectDeleter<BE16 **> o_deleter{o, 0};
+        upx::ArrayDeleter<BE32 **> a_deleter{a, 0};
+        for (size_t i = 0; i < N; i++) {
+            o[i] = new BE16;
+            o_deleter.count += 1;
+            a[i] = New(BE32, 1 + i);
+            a_deleter.count += 1;
+        }
+    }
+    for (size_t i = 0; i < N; i++) {
+        assert(o[i] == nullptr);
+        assert(a[i] == nullptr);
+    }
+}
+
 TEST_CASE("ptr_static_cast") {
     // check that we don't trigger any -Wcast-align warnings
     using upx::ptr_static_cast;

src/compress/compress_bzip2.cpp

@@ -24,6 +24,10 @@
    <markus@oberhumer.com>
  */
 
+#include "../headers.h"
+#if WITH_BZIP2
+#include <bzip2/bzlib.h>
+#endif
 #include "../conf.h"
 
 void bzip2_compress_config_t::reset() noexcept { mem_clear(this); }
@@ -31,7 +35,6 @@ void bzip2_compress_config_t::reset() noexcept { mem_clear(this); }
 #if WITH_BZIP2
 #include "compress.h"
 #include "../util/membuffer.h"
-#include <bzip2/bzlib.h>
 
 #if defined(BZ_NO_STDIO) || 1
 // we need to supply bz_internal_error() when building with BZ_NO_STDIO

src/compress/compress_zstd.cpp

@@ -24,6 +24,12 @@
    <markus@oberhumer.com>
  */
 
+#include "../headers.h"
+#if WITH_ZSTD
+#include <zstd/lib/zstd.h>
+#include <zstd/lib/zstd_errors.h>
+#include <zstd/lib/compress/hist.h>
+#endif
 #include "../conf.h"
 
 void zstd_compress_config_t::reset() noexcept { mem_clear(this); }
@@ -31,9 +37,6 @@ void zstd_compress_config_t::reset() noexcept { mem_clear(this); }
 #if WITH_ZSTD
 #include "compress.h"
 #include "../util/membuffer.h"
-#include <zstd/lib/zstd.h>
-#include <zstd/lib/zstd_errors.h>
-#include <zstd/lib/compress/hist.h>
 
 static int convert_errno_from_zstd(size_t zr) {
     const ZSTD_ErrorCode ze = ZSTD_getErrorCode(zr);

src/conf.h

@@ -190,7 +190,7 @@ typedef upx_int64_t upx_off_t;
 #define unlikely              __acc_unlikely
 #define very_likely           __acc_very_likely
 #define very_unlikely         __acc_very_unlikely
-// cosmetic: explictly annotate some functions which may throw exceptions
+// cosmetic: explicitly annotate some functions which may throw exceptions
 // note: noexcept(false) is the default for all C++ functions anyway
 #define may_throw             noexcept(false)
 

src/except.cpp

@@ -41,8 +41,8 @@ Throwable::Throwable(const char *m, int e, bool w) noexcept : super(),
         msg = strdup(m);
         assert_noexcept(msg != nullptr);
     }
-    NO_fprintf(stderr, "construct exception: %zu %zu %s\n", stats.counter_current,
+    NO_fprintf(stderr, "construct exception: %zu %zu %s\n", size_t(stats.counter_current),
-               stats.counter_total, (const char *) msg);
+               size_t(stats.counter_total), (const char *) msg);
     stats.counter_current += 1;
     stats.counter_total += 1;
 }
@@ -55,16 +55,16 @@ Throwable::Throwable(const Throwable &other) noexcept : super(other),
         msg = strdup(other.msg);
         assert_noexcept(msg != nullptr);
     }
-    NO_fprintf(stderr, "copy construct exception: %zu %zu %s\n", stats.counter_current,
+    NO_fprintf(stderr, "copy construct exception: %zu %zu %s\n", size_t(stats.counter_current),
-               stats.counter_total, (const char *) msg);
+               size_t(stats.counter_total), (const char *) msg);
     stats.counter_current += 1;
     stats.counter_total += 1;
 }
 
 Throwable::~Throwable() noexcept {
     stats.counter_current -= 1;
-    NO_fprintf(stderr, "destruct exception: %zu %zu %s\n", stats.counter_current,
+    NO_fprintf(stderr, "destruct exception: %zu %zu %s\n", size_t(stats.counter_current),
-               stats.counter_total, (const char *) msg);
+               size_t(stats.counter_total), (const char *) msg);
     upx::owner_free(msg);
 }
 

src/headers.h

@@ -106,12 +106,14 @@ static_assert(sizeof(void *) == 8);
 #include <intrin.h>
 #endif
 
-// C++ system headers
+// C++ freestanding headers
 #include <cstddef>
 #include <exception>
 #include <new>
 #include <type_traits>
-
+#include <utility>
+// C++ system headers
+#include <memory> // std::unique_ptr
 // C++ multithreading (UPX currently does not use multithreading)
 #if __STDC_NO_ATOMICS__
 #undef WITH_THREADS

src/p_lx_elf.h

@@ -44,7 +44,7 @@ public:
     PackLinuxElf(InputFile *f);
     virtual ~PackLinuxElf();
     /*virtual void buildLoader(const Filter *);*/
-    virtual int getVersion() const override { return 14; } // upx-3.96 cannot upack, for instance
+    virtual int getVersion() const override { return 14; } // upx-3.96 cannot unpack, for instance
     virtual bool canUnpackVersion(int version) const override { return (version >= 11); }
     virtual tribool canUnpack() override { return super::canUnpack(); } // bool, except -1: format known, but not packed
 

src/packmast.cpp

@@ -25,8 +25,6 @@
    <markus@oberhumer.com>               <ezerotven+github@gmail.com>
  */
 
-#include "headers.h"
-#include <memory> // std::unique_ptr
 #include "conf.h"
 #include "file.h"
 #include "packmast.h"

src/pefile.cpp

@@ -282,19 +282,6 @@ void PeFile::Interval::dump() const {
 // relocation handling
 **************************************************************************/
 
-namespace {
-struct FixDeleter final { // helper so we don't leak memory on exceptions
-    LE32 **fix;
-    size_t count;
-    ~FixDeleter() noexcept {
-        for (size_t i = 0; i < count; i++) {
-            delete[] fix[i];
-            fix[i] = nullptr;
-        }
-    }
-};
-} // namespace
-
 void PeFile::Reloc::RelocationBlock::reset() noexcept {
     rel = nullptr;  // SPAN_0
     rel1 = nullptr; // SPAN_0
@@ -489,7 +476,7 @@ void PeFile32::processRelocs() // pass1
             infoWarning("skipping unsupported relocation type %d (%d)", ic, counts[ic]);
 
     LE32 *fix[4];
-    FixDeleter fixdel{fix, 0}; // don't leak memory
+    upx::ArrayDeleter<LE32 **> fixdel{fix, 0}; // don't leak memory
     for (ic = 0; ic < 4; ic++) {
         fix[ic] = New(LE32, counts[ic]);
         fixdel.count += 1;
@@ -591,7 +578,7 @@ void PeFile64::processRelocs() // pass1
             infoWarning("skipping unsupported relocation type %d (%d)", ic, counts[ic]);
 
     LE32 *fix[16];
-    FixDeleter fixdel{fix, 0}; // don't leak memory
+    upx::ArrayDeleter<LE32 **> fixdel{fix, 0}; // don't leak memory
     for (ic = 0; ic < 16; ic++) {
         fix[ic] = New(LE32, counts[ic]);
         fixdel.count += 1;
@@ -1919,11 +1906,14 @@ void PeFile::processResources(Resource *res) {
     SPAN_S_VAR(byte, ores, oresources + res->dirsize());
 
     char *keep_icons = nullptr; // icon ids in the first icon group
+    upx::ArrayDeleter<char **> keep_icons_deleter{&keep_icons, 1}; // don't leak memory
     unsigned iconsin1stdir = 0;
     if (opt->win32_pe.compress_icons == 2)
         while (res->next()) // there is no rewind() in Resource
             if (res->itype() == RT_GROUP_ICON && iconsin1stdir == 0) {
                 iconsin1stdir = get_le16(ibuf.subref("bad resoff %#x", res->offs() + 4, 2));
+                delete[] keep_icons;
+                keep_icons = nullptr;
                 keep_icons = New(char, 1 + iconsin1stdir * 9);
                 *keep_icons = 0;
                 for (unsigned ic = 0; ic < iconsin1stdir; ic++)
@@ -2007,8 +1997,6 @@ void PeFile::processResources(Resource *res) {
     }
     soresources = ptr_diff_bytes(ores, oresources);
 
-    delete[] keep_icons;
-    keep_icons = nullptr;
     if (!res->clear()) {
         // The area occupied by the resource directory is not continuous
         // so to still support uncompression, I can't zero this area.

src/util/cxxlib.h

@@ -179,6 +179,38 @@ forceinline Result ptr_static_cast(const From *ptr) noexcept {
     return static_cast<Result>(static_cast<const void *>(ptr));
 }
 
+// helper classes so we don't leak memory on exceptions
+template <class T> // T is "Type **"
+struct ObjectDeleter final {
+    static_assert(std::is_pointer_v<T>);
+    static_assert(std::is_pointer_v<std::remove_pointer_t<T> >);
+    T items;      // public
+    size_t count; // public
+    ~ObjectDeleter() noexcept { delete_items(); }
+    void delete_items() noexcept {
+        for (size_t i = 0; i < count; i++) {
+            auto item = items[i];
+            items[i] = nullptr;
+            delete item; // single object delete
+        }
+    }
+};
+template <class T> // T is "Type **"
+struct ArrayDeleter final {
+    static_assert(std::is_pointer_v<T>);
+    static_assert(std::is_pointer_v<std::remove_pointer_t<T> >);
+    T items;      // public
+    size_t count; // public
+    ~ArrayDeleter() noexcept { delete_items(); }
+    void delete_items() noexcept {
+        for (size_t i = 0; i < count; i++) {
+            auto item = items[i];
+            items[i] = nullptr;
+            delete[] item; // array delete
+        }
+    }
+};
+
 class noncopyable {
 protected:
     forceinline constexpr noncopyable() noexcept {}

src/util/xspan_impl_common.h

@@ -457,7 +457,7 @@ public: // raw access
     // like C++20 std::span
     pointer data() const noexcept { return ptr; }
     pointer data(size_t bytes) const { return raw_bytes(bytes); } // UPX extra
-    size_type size() const noexcept { return size_bytes() / sizeof(element_type); }
+    size_type size() const { return size_bytes() / sizeof(element_type); }
     size_type size_bytes() const {
         assertInvariants();
         if __acc_cte (!configRequirePtr && ptr == nullptr)