all: cleanups

.github/typos_config.toml

@@ -6,8 +6,8 @@
 [files]
 extend-exclude = ["LICENSE", "misc/*podman*/*/packages.txt"]
 
-[default.extend-identifiers]
 # misc variable names & symbols
+[default.extend-identifiers]
 acc_spawnve = "acc_spawnve"
 ba = "ba"
 fo = "fo"

.github/workflows/ci.yml

@@ -12,8 +12,8 @@ env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
   UPX_CMAKE_BUILD_FLAGS: --verbose
-  # 2023-08-10
+  # 2023-08-15
-  ZIG_DIST_VERSION: 0.12.0-dev.47+0461a64a9
+  ZIG_DIST_VERSION: 0.12.0-dev.89+913511557
 
 jobs:
   job-rebuild-and-verify-stubs:

.github/workflows/weekly-ci-llvm-mingw.yml

@@ -24,12 +24,12 @@ jobs:
           - name: llvm-mingw-20230614-ucrt
             llvm_version: 16.0.6
             url: 'https://github.com/mstorsjo/llvm-mingw/releases/download/20230614/llvm-mingw-20230614-ucrt-ubuntu-20.04-x86_64.tar.xz'
-          - name: llvm-mingw-20230730-msvcrt
+          - name: llvm-mingw-20230811-msvcrt
             llvm_version: 17.0.0rc1
-            url: 'https://github.com/mstorsjo/llvm-mingw/releases/download/20230730/llvm-mingw-20230730-msvcrt-ubuntu-20.04-x86_64.tar.xz'
+            url: 'https://github.com/mstorsjo/llvm-mingw/releases/download/20230811/llvm-mingw-20230811-msvcrt-ubuntu-20.04-x86_64.tar.xz'
-          - name: llvm-mingw-20230730-ucrt
+          - name: llvm-mingw-20230811-ucrt
             llvm_version: 17.0.0rc1
-            url: 'https://github.com/mstorsjo/llvm-mingw/releases/download/20230730/llvm-mingw-20230730-ucrt-ubuntu-20.04-x86_64.tar.xz'
+            url: 'https://github.com/mstorsjo/llvm-mingw/releases/download/20230811/llvm-mingw-20230811-ucrt-ubuntu-20.04-x86_64.tar.xz'
     steps:
       - name: 'Check out code'
         uses: actions/checkout@v3

.github/workflows/weekly-ci-zigcc.yml

@@ -8,8 +8,8 @@ on:
 env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
-  # 2023-08-10
+  # 2023-08-15
-  ZIG_DIST_VERSION: 0.12.0-dev.47+0461a64a9
+  ZIG_DIST_VERSION: 0.12.0-dev.89+913511557
 
 jobs:
   job-linux-zigcc: # uses cmake + make

src/check/dt_check.cpp

@@ -577,7 +577,7 @@ void upx_compiler_sanity_check(void) noexcept {
 **************************************************************************/
 
 TEST_CASE("assert_noexcept") {
-    // just to make sure that our own assert macros don't generate any warnings
+    // just to make sure that our own assert() macros don't generate any warnings
     byte dummy = 0;
     byte *ptr1 = &dummy;
     const byte *const ptr2 = &dummy;

src/packer.cpp

@@ -44,16 +44,14 @@ PackerBase::PackerBase(InputFile *f) : fi(f), file_size(f ? f->st.st_size : 0) {
 Packer::Packer(InputFile *f) : PackerBase(f) { uip = new UiPacker(this); }
 
 Packer::~Packer() noexcept {
-    // owner
     owner_delete(uip);
     owner_delete(linker);
     assert_noexcept(linker == nullptr);
-    // references
-    bele = nullptr;
 }
 
 // for PackMaster
 void Packer::assertPacker() const {
+#if DEBUG
     assert(getFormat() > 0);
     assert(getFormat() < 255);
     assert(getVersion() >= 11);
@@ -76,7 +74,6 @@ void Packer::assertPacker() const {
             fprintf(stderr, "%s\n", getName());
         assert(bele == format_bele);
     }
-#if DEBUG
     Linker *l = newLinker();
     assert(l != nullptr);
     if (bele != l->bele)
@@ -509,7 +506,7 @@ unsigned Packer::getRandomId() const {
 
 // this is called directly after the constructor from class PackMaster
 void Packer::initPackHeader() {
-    mem_clear(&ph);
+    ph.reset();
     ph.version = getVersion();
     ph.format = getFormat();
     ph.method = M_NONE;

src/packer.h

@@ -59,11 +59,11 @@ public:
 
     // canPack() should throw a cantPackException eplaining why it cannot pack
     // a recognized format.
-    // canPack() can return -1 to stop early; see class PackMaster
+    // canPack() can return -1 to fail early; see class PackMaster
     virtual tribool canPack() = 0;
     // canUnpack() should throw a cantUnpackException eplaining why it cannot pack
     // a recognized format.
-    // canUnpack() can return -1 to stop early; see class PackMaster
+    // canUnpack() can return -1 to fail early; see class PackMaster
     virtual tribool canUnpack() = 0;
 
     // PackMaster entries
@@ -82,7 +82,7 @@ protected:
         const upx_int64_t file_size;    // must get set by constructor
         const upx_uint64_t file_size_u; // explicitly unsigned
     };
-    PackHeader ph; // must be filled by canUnpack()
+    PackHeader ph; // must be filled by canUnpack(); also used by UiPacker
 };
 
 /*************************************************************************

src/packmast.cpp

@@ -26,7 +26,7 @@
  */
 
 #include "headers.h"
-#include <memory>
+#include <memory> // std::unique_ptr
 #include "conf.h"
 #include "file.h"
 #include "packmast.h"
@@ -102,7 +102,7 @@ static tribool try_can_pack(PackerBase *pb, void *user) may_throw {
             return true; // success
         }
         if (r.isOther()) // aka "-1"
-            return r;    // canPack() says the format is recognized and we should stop early
+            return r;    // canPack() says the format is recognized and we should fail early
     } catch (const IOException &) {
         // ignored
     }
@@ -120,7 +120,7 @@ static tribool try_can_unpack(PackerBase *pb, void *user) may_throw {
             return true; // success
         }
         if (r.isOther()) // aka "-1"
-            return r;    // canUnpack() says the format is recognized and we should stop early
+            return r;    // canUnpack() says the format is recognized and we should fail early
     } catch (const IOException &) {
         // ignored
     }
@@ -138,15 +138,15 @@ PackerBase *PackMaster::visitAllPackers(visit_func_t func, InputFile *f, const O
     ACC_BLOCK_BEGIN                                                                                \
     COMPILE_TIME_ASSERT(std::is_nothrow_destructible_v<Klass>)                                     \
     auto pb = std::unique_ptr<PackerBase>(new Klass(f));                                           \
-    pb->assertPacker();                                                                            \
     if (o->debug.debug_level)                                                                      \
         fprintf(stderr, "visitAllPackers: (ver=%d, fmt=%3d) %s\n", pb->getVersion(),               \
                 pb->getFormat(), #Klass);                                                          \
+    pb->assertPacker();                                                                            \
     tribool r = func(pb.get(), user);                                                              \
     if (r)                                                                                         \
         return pb.release(); /* success */                                                         \
     if (r.isOther())                                                                               \
-        return nullptr; /* stop early */                                                           \
+        return nullptr; /* stop and fail early */                                                  \
     ACC_BLOCK_END
 
     // NOTE: order of tries is important !!!

src/packmast.h

@@ -52,7 +52,7 @@ public:
 
 private:
     OwningPointer(PackerBase) packer = nullptr; // owner
-    InputFile *fi = nullptr;                    // reference
+    InputFile *const fi;                        // reference, required
 
     static PackerBase *getPacker(InputFile *f);
     static PackerBase *getUnpacker(InputFile *f);

src/stub/src/include/header.S

@@ -31,10 +31,11 @@
 
 section         UPX1HEAD
 
-                .byte   85,80,88,33     //  0  UPX_MAGIC_LE32
+                .byte   85,80,88,33     //  0  UPX_MAGIC_LE32 aka "UPX!"
 #if 0
                 .byte   161,216,208,213 //  4  UPX_MAGIC2_LE32
 #else
+                // these will get set to actual values; initialized with some arbitrary numbers
                 .byte   161             //  4  version
                 .byte   216             //  5  format
                 .byte   208             //  6  method

src/ui.cpp

@@ -46,7 +46,7 @@ enum {
     M_CB_SCREEN // 2 line callback using screen
 };
 
-struct UiPacker::State {
+struct UiPacker::State final {
     int mode;
 
     unsigned u_len;

src/util/membuffer.cpp

@@ -62,7 +62,7 @@ static noinline void init_use_simple_mcheck() noexcept {
 static 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"
+    // NOTE: clang-analyzer-unix.Malloc does not know that this flag is "constant"; see below
     return use_simple_mcheck_flag;
 }
 #else
@@ -176,9 +176,10 @@ void MemBuffer::fill(unsigned off, unsigned len, int value) {
 //
 **************************************************************************/
 
-#define PTR_BITS(p) ((unsigned) ((upx_uintptr_t) (p) &0xffffffff))
+// for use_simple_mcheck()
-#define MAGIC1(p) ((PTR_BITS(p) ^ 0xfefdbeeb) | 1)
+#define PTR_BITS32(p) ((unsigned) ((upx_uintptr_t) (p) &0xffffffff))
-#define MAGIC2(p) ((PTR_BITS(p) ^ 0xfefdbeeb ^ 0x80024011) | 1)
+#define MAGIC1(p) ((PTR_BITS32(p) ^ 0xfefdbeeb) | 1)
+#define MAGIC2(p) ((PTR_BITS32(p) ^ 0xfefdbeeb ^ 0x88224411) | 1)
 
 void MemBuffer::checkState() const {
     if (!ptr)
@@ -215,7 +216,7 @@ void MemBuffer::alloc(upx_uint64_t bytes) {
         // store magic constants to detect buffer overruns
         set_ne32(p - 8, size_in_bytes);
         set_ne32(p - 4, MAGIC1(p));
-        set_ne32(p + size_in_bytes, MAGIC2(p));
+        set_ne32(p + size_in_bytes + 0, MAGIC2(p));
         set_ne32(p + size_in_bytes + 4, stats.global_alloc_counter);
     }
     ptr = (pointer) (void *) p;

src/util/util.cpp

@@ -787,6 +787,7 @@ TEST_CASE("get_ratio") {
     CHECK(get_ratio(2 * UPX_RSIZE_MAX, 1024ull * UPX_RSIZE_MAX) == 9999999);
 }
 
+namespace {
 template <class T>
 struct TestTriBool {
     static void test(bool expect_true, int x) noexcept {
@@ -831,9 +832,14 @@ struct TestTriBool {
         CHECK(a.isOther());
     }
 };
+} // namespace
 
 TEST_CASE("TriBool") {
+    static_assert(!tribool(false));
+    static_assert(tribool(true));
+    static_assert(!tribool(tribool::Other));
     TestTriBool<tribool>::test(false, -1);
+    //
     TestTriBool<TriBool<upx_int8_t> >::test(false, -1);
     TestTriBool<TriBool<upx_int64_t> >::test(false, -1);
     //

src/util/util.h

@@ -209,24 +209,24 @@ public:
     static_assert(TOther != 0 && TOther != 1);
     enum value_type : underlying_type { False = 0, True = 1, Other = TOther };
     // constructors
-    forceinline TriBool() noexcept = default;
+    forceinline constexpr TriBool() noexcept = default;
     forceinline ~TriBool() noexcept = default;
     constexpr TriBool(value_type x) noexcept : value(x) {}
     constexpr TriBool(promoted_type x) noexcept : value(x == 0 ? False : (x == 1 ? True : Other)) {}
     // access
-    value_type getValue() const noexcept { return value; }
+    constexpr value_type getValue() const noexcept { return value; }
     // checks for > 0, so TOther determines if Other is false (the default) or true
-    explicit operator bool() const noexcept { return value > False; }
+    explicit constexpr operator bool() const noexcept { return value > False; }
     // query; this is NOT the same as operator bool()
-    bool isStrictFalse() const noexcept { return value == False; }
+    constexpr bool isStrictFalse() const noexcept { return value == False; }
-    bool isStrictTrue() const noexcept { return value == True; }
+    constexpr bool isStrictTrue() const noexcept { return value == True; }
-    bool isStrictBool() const noexcept { return value == False || value == True; }
+    constexpr bool isStrictBool() const noexcept { return value == False || value == True; }
-    bool isOther() const noexcept { return value != False && value != True; }
+    constexpr bool isOther() const noexcept { return value != False && value != True; }
     // "other" can mean many things, depending on usage context, so provide some alternative names:
-    // forceinline bool isDefault() const noexcept { return isOther(); } // might be misleading
+    // constexpr bool isDefault() const noexcept { return isOther(); } // might be misleading
-    forceinline bool isIndeterminate() const noexcept { return isOther(); }
+    constexpr bool isIndeterminate() const noexcept { return isOther(); }
-    forceinline bool isUndecided() const noexcept { return isOther(); }
+    constexpr bool isUndecided() const noexcept { return isOther(); }
-    // forceinline bool isUnset() const noexcept { return isOther(); } // might be misleading
+    // constexpr bool isUnset() const noexcept { return isOther(); } // might be misleading
     constexpr bool operator==(TriBool other) const noexcept { return value == other.value; }
     constexpr bool operator==(value_type other) const noexcept { return value == other; }
     constexpr bool operator==(promoted_type other) const noexcept { return value == other; }

src/util/xspan.cpp

@@ -33,7 +33,7 @@ XSPAN_NAMESPACE_BEGIN
 // debugging stats
 struct XSpanStats {
     upx_std_atomic(size_t) check_range_counter;
-    // these normally will be zero, but doctest checks will populate them
+    // these usually will be zero, but internal doctest checks will populate them; see dt_xspan.cpp
     upx_std_atomic(size_t) fail_nullptr;
     upx_std_atomic(size_t) fail_nullbase;
     upx_std_atomic(size_t) fail_not_same_base;
@@ -71,6 +71,7 @@ void xspan_fail_range_range() {
 }
 
 void xspan_check_range(const void *ptr, const void *base, ptrdiff_t size_in_bytes) {
+    // info: pointers are out of range deliberatly during internal doctest checks; see dt_xspan.cpp
     xspan_stats.check_range_counter += 1;
     if very_unlikely (ptr == nullptr)
         xspan_fail_range_nullptr();