all: more assorted cleanups

.github/workflows/ci.yml

@@ -150,10 +150,14 @@ jobs:
       - name: 'Build cmake extra/gcc/release'
         if: ${{ matrix.gcc != '' }}
         run:   'make build/extra/gcc/release CC=${{ matrix.gcc }} CXX=${{ matrix.gxx }}'
+      - name: 'Build cmake xtarget/cross-darwin-arm64/debug'
+        run: |
+          CC="clang -target arm64-apple-darwin" CXX="clang++ -target arm64-apple-darwin" \
+          make UPX_XTARGET=cross-darwin-arm64 xtarget/debug
       - name: 'Build cmake xtarget/cross-darwin-arm64/release'
         run: |
-          export CC="clang -target arm64-apple-darwin" CXX="clang++ -target arm64-apple-darwin"
+          CC="clang -target arm64-apple-darwin" CXX="clang++ -target arm64-apple-darwin" \
-          make UPX_XTARGET=cross-darwin-arm64
+          make UPX_XTARGET=cross-darwin-arm64 xtarget/release
       - name: 'Make artifact'
         run: |
           N=upx-${GITHUB_REF_NAME}-${GITHUB_SHA:0:7}-${{ matrix.os }}
@@ -187,8 +191,9 @@ jobs:
       fail-fast: false
       matrix:
         include:
-          # windows-2019 used to work but got broken with the 20220821.1 runner-image update;
+          # windows-2019 used to work but got broken with the 20220821.1 runner-image
-          # as you cannot download that image this is painful to debug; disable for now
+          # update; we cannot download that image for inspection, and debugging the
+          # remote image is painful, so disable for now
           # see https://github.com/actions/runner-images.git
           ####- { os: windows-2019 }
           - { os: windows-2022 }

.github/workflows/close-stale-issues.yml

@@ -13,7 +13,7 @@
 name: 'GitHub - Close inactive issues'
 on:
   schedule:
-    - cron: "30 6 * * 4"
+    - cron: '30 6 * * 4'
   workflow_dispatch:
 jobs:
   close-issues:

.github/workflows/minimal-ci.yml

@@ -12,17 +12,18 @@ jobs:
     steps:
       - name: ${{ format('Install packages {0}', matrix.container) }}
         run: 'apk update && apk upgrade && apk add bash clang cmake g++ git make'
-      - name: 'Check out code'
+      - name: ${{ format('Check out code upx-{0}', github.ref_name) }}
         run: |
-          git clone --branch devel --depth 1 https://github.com/upx/upx
+          git clone --branch "$GITHUB_REF_NAME" --depth 1 https://github.com/upx/upx
           git -C upx submodule update --init
-          echo "artifact_name=upx-${GITHUB_REF_NAME}-${GITHUB_SHA:0:7}-minimal-ci-${{ matrix.container }}" | sed 's/:/-/g' >> $GITHUB_ENV
+          echo "artifact_name=upx-${GITHUB_REF_NAME}-${GITHUB_SHA:0:7}-minimal-ci-${{ matrix.container }}" | sed 's/[:,; ]/-/g' >> $GITHUB_ENV
-      - { name: 'Build clang', run: 'make -C upx build/extra/clang/release' }
+      - { name: 'Build clang', run: 'make -C upx UPX_XTARGET=clang-static CC="clang -static" CXX="clang++ -static"' }
-      - { name: 'Build gcc',   run: 'make -C upx build/extra/gcc/release' }
+      - { name: 'Build gcc',   run: 'make -C upx UPX_XTARGET=gcc-static   CC="gcc -static"   CXX="g++ -static"' }
+      - { name: 'Strip release binaries', run: 'strip -p --strip-unneeded upx/build/*/*/release/upx' }
       - name: ${{ format('Upload artifact {0}', env.artifact_name) }}
         uses: actions/upload-artifact@v3
         with:
           name: ${{ env.artifact_name }}
           path: 'upx*/build/*/*/*/upx'
-      - { name: 'Run basic tests clang', run: 'make -C upx/build/extra/clang/release test' }
+      - { name: 'Run basic tests clang', run: 'make -C upx/build/xtarget/clang-static/release test' }
-      - { name: 'Run basic tests gcc',   run: 'make -C upx/build/extra/gcc/release test' }
+      - { name: 'Run basic tests gcc',   run: 'make -C upx/build/xtarget/gcc-static/release test' }

CMakeLists.txt

@@ -123,10 +123,16 @@ endif()
 file(GLOB upx_SOURCES "src/*.cpp" "src/[cfu]*/*.cpp")
 list(SORT upx_SOURCES)
 add_executable(upx ${upx_SOURCES})
-#target_compile_features(upx PRIVATE cxx_std_17)
 set_property(TARGET upx PROPERTY CXX_STANDARD 17)
 target_link_libraries(upx upx_vendor_ucl upx_vendor_zlib)
 
+if(NOT MSVC)
+    # rather strict default compilation warnings
+    set(warn_strict
+        -Wall -Wextra -Wcast-align -Wcast-qual -Wmissing-declarations
+        -Wpointer-arith -Wshadow -Wvla -Wwrite-strings
+    )
+endif()
 if(UPX_CONFIG_DISABLE_WERROR)
     set(warn_Werror "")
     set(warn_WX "")
@@ -191,7 +197,7 @@ upx_sanitize_target(${t})
 if(MSVC)
     target_compile_options(${t} PRIVATE -J -W4 ${warn_WX})
 else()
-    target_compile_options(${t} PRIVATE -Wall -Wextra -Wcast-align -Wcast-qual -Wpointer-arith -Wvla -Wwrite-strings ${warn_Werror})
+    target_compile_options(${t} PRIVATE ${warn_strict} ${warn_Werror})
 endif()
 
 set(t upx_vendor_zlib)
@@ -213,7 +219,7 @@ target_compile_options(${t} PRIVATE -DDYNAMIC_BMI2=0 -DZSTD_DISABLE_ASM)
 if(MSVC)
     target_compile_options(${t} PRIVATE -J -W4 ${warn_WX})
 else()
-    target_compile_options(${t} PRIVATE -Wall -Wextra -Wcast-align -Wcast-qual -Wpointer-arith -Wvla -Wwrite-strings ${warn_Werror})
+    target_compile_options(${t} PRIVATE ${warn_strict} ${warn_Werror})
 endif()
 endif()
 
@@ -231,10 +237,7 @@ upx_sanitize_target(${t})
 if(MSVC)
     target_compile_options(${t} PRIVATE -EHsc -J -W4 ${warn_WX})
 else()
-    target_compile_options(${t} PRIVATE
+    target_compile_options(${t} PRIVATE ${warn_strict} ${warn_Werror})
-        -Wall -Wextra -Wcast-align -Wcast-qual -Wmissing-declarations -Wpointer-arith
-        -Wshadow -Wvla -Wwrite-strings ${warn_Werror}
-    )
 endif()
 if(NOT UPX_CONFIG_DISABLE_ZSTD)
     target_compile_definitions(${t} PRIVATE WITH_ZSTD=1)
@@ -333,6 +336,7 @@ if (CMAKE_BUILD_TYPE AND NOT CMAKE_BUILD_TYPE MATCHES "^(Debug|Release)$")
     message(WARNING "WARNING: unsupported CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}; please use \"Debug\" or \"Release\"")
 endif()
 
+if(NOT UPX_CONFIG_CMAKE_DISABLE_PLATFORM_CHECK)
 # extra sanity checks to detect incompatible C vs CXX settings
 if(NOT ",${CMAKE_C_PLATFORM_ID}," STREQUAL ",${CMAKE_CXX_PLATFORM_ID},")
     message(FATAL_ERROR "ERROR: CMAKE_C_PLATFORM_ID CMAKE_CXX_PLATFORM_ID mismatch")
@@ -340,5 +344,6 @@ endif()
 if(NOT ",${CMAKE_C_COMPILER_ABI}," STREQUAL ",${CMAKE_CXX_COMPILER_ABI},")
     message(FATAL_ERROR "ERROR: CMAKE_C_COMPILER_ABI CMAKE_CXX_COMPILER_ABI mismatch")
 endif()
+endif() # UPX_CONFIG_CMAKE_DISABLE_PLATFORM_CHECK
 
 # vim:set ft=cmake ts=4 sw=4 tw=0 et:

Makefile

@@ -39,6 +39,8 @@ build/release: PHONY
 	$(call run_build,$@,Release)
 
 # shortcuts
+all: build/debug build/release
+build: build/release
 debug: build/debug
 release: build/release
 
@@ -168,6 +170,7 @@ build/xtarget/$(UPX_XTARGET)/%: export CXX
 # shortcuts
 xtarget/debug:   build/xtarget/$(UPX_XTARGET)/debug
 xtarget/release: build/xtarget/$(UPX_XTARGET)/release
+xtarget: xtarget/release
 # set new default
 .DEFAULT_GOAL = xtarget/release
 

misc/cross-compile-upx-with-podman/10-create-image.sh

@@ -6,7 +6,7 @@ argv0=$0; argv0abs="$(readlink -fn "$argv0")"; argv0dir="$(dirname "$argv0abs")"
 # create the image from Dockerfile
 # using a rootless Podman container
 
-# NOTE: this image is based on rebuild-stubs-with-upx/upx-stubtools-20221212-v4,
+# NOTE: this image is based on rebuild-stubs-with-podman/upx-stubtools-20221212-v4,
 #   so you have to create that image first
 # WARNING: we install many packages, so the resulting image needs A LOT of disk space!
 image=upx-cross-compile-20230115-v2

misc/cross-compile-upx-with-podman/Dockerfile

@@ -1,4 +1,4 @@
-# NOTE: this image is based on rebuild-stubs-with-upx/upx-stubtools-20221212-v4,
+# NOTE: this image is based on rebuild-stubs-with-podman/upx-stubtools-20221212-v4,
 #   so you have to create that image first
 # WARNING: we install many packages, so the resulting image needs A LOT of disk space!
 FROM localhost/upx-stubtools-20221212-v4

misc/rebuild-stubs-with-podman/Dockerfile

@@ -40,7 +40,7 @@ RUN cd /root \
     && rm /root/bin-upx-20221212.tar.xz \
     && true
 
-# install pre-built binary UPX versions into /usr/local/bin; not required but convenient for testing
+# install official UPX release binaries into /usr/local/bin; not required but convenient for testing
 RUN cd /root \
     && wget -q https://github.com/upx/upx/releases/download/v3.91/upx-3.91-amd64_linux.tar.bz2 \
     && for v in 3.92 3.93 3.94 3.95 3.96 4.0.0 4.0.1; do wget -q https://github.com/upx/upx/releases/download/v${v}/upx-${v}-amd64_linux.tar.xz; done \
@@ -51,7 +51,7 @@ RUN cd /root \
 
 # create default user upx 2000:2000
 RUN useradd upx -U --uid 2000 --shell /bin/bash -m \
-    && mkdir -p /home/upx/.local/bin /home/upx/src/upx \
+    && mkdir -p /home/upx/.cache/zig /home/upx/.local/bin /home/upx/src/upx \
     && ln -s /usr/local/bin/bin-upx-20221212 /home/upx/.local/bin/bin-upx \
     && chown -R upx:upx /home/upx \
     && true

src/Makefile

@@ -57,13 +57,17 @@ endif
 
 # run the UPX testsuite
 # The expected (old) checksums are in $(top_srcdir)/.github/travis_testsuite_1-expected_sha256sums.sh
-# The   actual (new) checksums are in tmp-testsuite/testsuite_1/.sha256sums.recreate
+# The   actual (new) checksums are in ./tmp-testsuite/testsuite_1/.sha256sums.recreate
 ifneq ($(wildcard $(upx_testsuite_SRCDIR)/files/packed/.),)
 ifneq ($(wildcard $(top_srcdir)/.github/travis_testsuite_1.sh),)
-run-testsuite: export upx_exe                := $(top_srcdir)/build/release/upx
+run-testsuite: run-testsuite-release
-run-testsuite: export upx_testsuite_SRCDIR   := $(upx_testsuite_SRCDIR)
+run-testsuite-%: export upx_testsuite_SRCDIR   := $(upx_testsuite_SRCDIR)
-run-testsuite: export upx_testsuite_BUILDDIR := ./tmp-testsuite
+run-testsuite-%: export upx_testsuite_BUILDDIR := ./tmp-testsuite
-run-testsuite: $(top_srcdir)/build/release/upx PHONY
+run-testsuite-debug: export upx_exe := $(top_srcdir)/build/debug/upx
+run-testsuite-debug: PHONY $(top_srcdir)/build/debug/upx
+	time -p bash $(top_srcdir)/.github/travis_testsuite_1.sh
+run-testsuite-release: export upx_exe := $(top_srcdir)/build/release/upx
+run-testsuite-release: PHONY $(top_srcdir)/build/release/upx
 	time -p bash $(top_srcdir)/.github/travis_testsuite_1.sh
 endif
 endif

src/check/dt_check.cpp

@@ -147,8 +147,14 @@ template <class T>
 struct TestBELE {
     __acc_static_noinline bool test(void) {
         // POD checks
-        COMPILE_TIME_ASSERT(std::is_standard_layout<T>::value)
+        {
-        COMPILE_TIME_ASSERT(std::is_trivial<T>::value)
+            COMPILE_TIME_ASSERT(std::is_standard_layout<T>::value)
+            COMPILE_TIME_ASSERT(std::is_trivial<T>::value)
+            // extra checks, these are probably implied by std::is_trivial:
+            COMPILE_TIME_ASSERT(std::is_nothrow_default_constructible<T>::value)
+            COMPILE_TIME_ASSERT(std::is_trivially_copyable<T>::value)
+            COMPILE_TIME_ASSERT(std::is_trivially_default_constructible<T>::value)
+        }
         // alignment checks
         {
             COMPILE_TIME_ASSERT_ALIGNED1(T)

src/check/dt_xspan.cpp

@@ -88,14 +88,19 @@ TEST_CASE("basic xspan usage") {
         CHECK(x0 == z0p);
         CHECK(xp == z0s);
 
+        CHECK_NOTHROW(raw_bytes(a0, 0));
+        CHECK_THROWS(raw_bytes(a0, 1));
+        CHECK_THROWS(raw_index_bytes(a0, 0, 0));
         CHECK(raw_bytes(c0, 4) == buf);
         CHECK(raw_index_bytes(c0, 1, 3) == buf + 1);
         CHECK(raw_bytes(cp, 4) == buf);
         CHECK(raw_index_bytes(cp, 1, 3) == buf + 1);
         CHECK(raw_bytes(cs, 4) == buf);
         CHECK(raw_index_bytes(cs, 1, 3) == buf + 1);
+#if WITH_XSPAN >= 2
         CHECK_THROWS(raw_bytes(cs, 5));
         CHECK_THROWS(raw_index_bytes(cs, 1, 4));
+#endif
     }
 
     SUBCASE("XSPAN_x_VAR") {
@@ -128,14 +133,19 @@ TEST_CASE("basic xspan usage") {
         CHECK(x0 == z0p);
         CHECK(xp == z0s);
 
+        CHECK_NOTHROW(raw_bytes(a0, 0));
+        CHECK_THROWS(raw_bytes(a0, 1));
+        CHECK_THROWS(raw_index_bytes(a0, 0, 0));
         CHECK(raw_bytes(c0, 4) == buf);
         CHECK(raw_index_bytes(c0, 1, 3) == buf + 1);
         CHECK(raw_bytes(cp, 4) == buf);
         CHECK(raw_index_bytes(cp, 1, 3) == buf + 1);
         CHECK(raw_bytes(cs, 4) == buf);
         CHECK(raw_index_bytes(cs, 1, 3) == buf + 1);
+#if WITH_XSPAN >= 2
         CHECK_THROWS(raw_bytes(cs, 5));
         CHECK_THROWS(raw_index_bytes(cs, 1, 4));
+#endif
     }
 
     SUBCASE("xspan in class") {

src/conf.h

@@ -826,6 +826,7 @@ class MemBuffer;
 void *membuffer_get_void_ptr(MemBuffer &mb);
 unsigned membuffer_get_size(MemBuffer &mb);
 
+// xspan
 #include "util/xspan.h"
 
 // util/dt_check.cpp

src/options.cpp

@@ -27,8 +27,11 @@
 
 #include "conf.h"
 
+static options_t global_options;
+options_t *opt = &global_options; // also see class PackMaster
+
 /*************************************************************************
-// options
+// reset
 **************************************************************************/
 
 void options_t::reset() {
@@ -69,11 +72,8 @@ void options_t::reset() {
     o->win32_pe.keep_resource = "";
 }
 
-static options_t global_options;
-options_t *opt = &global_options;
-
 /*************************************************************************
-//
+// doctest checks
 **************************************************************************/
 
 TEST_CASE("options_t::reset") {
@@ -89,7 +89,7 @@ static inline void test_options(const char *(&a)[N]) {
 }
 
 TEST_CASE("getopt") {
-    options_t *saved_opt = opt;
+    options_t *const saved_opt = opt;
     options_t local_options;
     opt = &local_options;
     opt->reset();

src/options.h

@@ -117,8 +117,6 @@ struct options_t final {
         CPU_286 = 2,
         CPU_386 = 3,
         CPU_486 = 4,
-        CPU_586 = 5,
-        CPU_686 = 6
     };
     int cpu;
 
@@ -133,12 +131,6 @@ struct options_t final {
         bool force_stub;
         bool no_reloc;
     } dos_exe;
-    struct {
-        bool boot_only;
-        bool no_align;
-        bool do_8bit;
-        bool do_8mib;
-    } ps1_exe;
     struct {
         unsigned blocksize;
         bool force_execve;      // force the linux/386 execve format
@@ -151,6 +143,12 @@ struct options_t final {
         bool android_shlib;     // keep some ElfXX_Shdr for dlopen()
         bool force_pie;         // choose DF_1_PIE instead of is_shlib
     } o_unix;
+    struct {
+        bool boot_only;
+        bool no_align;
+        bool do_8bit;
+        bool do_8mib;
+    } ps1_exe;
     struct {
         bool le;
     } watcom_le;

src/packmast.h

@@ -52,15 +52,15 @@ public:
     static Packer *visitAllPackers(visit_func_t, InputFile *f, const options_t *, void *user);
 
 private:
-    InputFile *fi;
+    InputFile *fi = nullptr;
-    Packer *p;
+    Packer *p = nullptr;
 
     static Packer *getPacker(InputFile *f);
     static Packer *getUnpacker(InputFile *f);
 
     // setup local options for each file
     options_t local_options;
-    options_t *saved_opt;
+    options_t *saved_opt = nullptr;
 };
 
 #endif /* already included */

src/util/xspan.h

@@ -42,7 +42,7 @@
 #ifndef XSPAN_CONFIG_ENABLE_IMPLICIT_CONVERSION
 #define XSPAN_CONFIG_ENABLE_IMPLICIT_CONVERSION 0
 #endif
-// allow automatic conversion PtrOrSpanOrNull => PtrOrSpan => Span (with runtime checks)
+// allow automatic conversion PtrOrSpanOrNull => PtrOrSpan => Span (with run-time checks)
 // choose between compile-time safety vs. possible run-time errors
 #ifndef XSPAN_CONFIG_ENABLE_SPAN_CONVERSION
 #define XSPAN_CONFIG_ENABLE_SPAN_CONVERSION 1

src/util/xspan_impl_ptr.h

@@ -200,7 +200,9 @@ template <class T>
 inline typename Ptr<T>::pointer raw_index_bytes(const Ptr<T> &a, size_t index,
                                                 size_t size_in_bytes) {
     typedef typename Ptr<T>::element_type element_type;
-    return raw_bytes(a, mem_size(sizeof(element_type), index, size_in_bytes)) + index;
+    if very_unlikely (a.raw_ptr() == nullptr)
+        throwInternalError("raw_index_bytes unexpected NULL ptr");
+    return a.raw_bytes(mem_size(sizeof(element_type), index, size_in_bytes)) + index;
 }
 
 /*************************************************************************

src/util/xspan_impl_ptr_or_null.h

@@ -100,7 +100,9 @@ template <class T>
 inline typename PtrOrSpanOrNull<T>::pointer raw_index_bytes(const PtrOrSpanOrNull<T> &a,
                                                             size_t index, size_t size_in_bytes) {
     typedef typename PtrOrSpanOrNull<T>::element_type element_type;
-    return raw_bytes(a, mem_size(sizeof(element_type), index, size_in_bytes)) + index;
+    if very_unlikely (a.raw_ptr() == nullptr)
+        throwInternalError("raw_index_bytes unexpected NULL ptr");
+    return a.raw_bytes(mem_size(sizeof(element_type), index, size_in_bytes)) + index;
 }
 
 /*************************************************************************

src/util/xspan_impl_ptr_or_span.h

@@ -126,7 +126,7 @@ template <class T>
 inline typename PtrOrSpan<T>::pointer raw_index_bytes(const PtrOrSpan<T> &a, size_t index,
                                                       size_t size_in_bytes) {
     typedef typename PtrOrSpan<T>::element_type element_type;
-    return raw_bytes(a, mem_size(sizeof(element_type), index, size_in_bytes)) + index;
+    return a.raw_bytes(mem_size(sizeof(element_type), index, size_in_bytes)) + index;
 }
 
 /*************************************************************************

src/util/xspan_impl_span.h

@@ -126,7 +126,7 @@ template <class T>
 inline typename Span<T>::pointer raw_index_bytes(const Span<T> &a, size_t index,
                                                  size_t size_in_bytes) {
     typedef typename Span<T>::element_type element_type;
-    return raw_bytes(a, mem_size(sizeof(element_type), index, size_in_bytes)) + index;
+    return a.raw_bytes(mem_size(sizeof(element_type), index, size_in_bytes)) + index;
 }
 
 /*************************************************************************

src/work.cpp

@@ -27,7 +27,7 @@
 
 // work.cpp implements the central loop, and it uses class PackMaster to
 // dispatch. PackMaster by itself will instatiate a concrete subclass
-// of Packer which then does the actual work.
+// of class Packer which then does the actual work.
 
 #include "conf.h"
 #include "file.h"
@@ -171,7 +171,7 @@ void do_one_file(const char *iname, char *oname) {
         throwInternalError("invalid command");
 
     // copy time stamp
-    if (opt->preserve_timestamp && oname[0] && fo.isOpen()) {
+    if (oname[0] && opt->preserve_timestamp && fo.isOpen()) {
 #if (USE_FTIME)
         r = setftime(fo.getFd(), &fi_ftime);
         IGNORE_ERROR(r);
@@ -190,26 +190,24 @@ void do_one_file(const char *iname, char *oname) {
 
     // rename or delete files
     if (oname[0] && !opt->output_name) {
-        // FIXME: .exe or .cof etc.
+        if (opt->backup) {
-        if (!opt->backup) {
+            char bakname[ACC_FN_PATH_MAX + 1];
+            if (!makebakname(bakname, sizeof(bakname), iname))
+                throwIOException("could not create a backup file name");
+            FileBase::rename(iname, bakname);
+        } else {
 #if (HAVE_CHMOD)
             r = chmod(iname, 0777);
             IGNORE_ERROR(r);
 #endif
             FileBase::unlink(iname);
-        } else {
-            // make backup
-            char bakname[ACC_FN_PATH_MAX + 1];
-            if (!makebakname(bakname, sizeof(bakname), iname))
-                throwIOException("could not create a backup file name");
-            FileBase::rename(iname, bakname);
         }
         FileBase::rename(oname, iname);
     }
 
     // copy file attributes
     if (oname[0]) {
-        oname[0] = 0;
+        oname[0] = 0; // done with oname
         const char *name = opt->output_name ? opt->output_name : iname;
         UNUSED(name);
 #if (USE_UTIME)
@@ -222,6 +220,13 @@ void do_one_file(const char *iname, char *oname) {
             IGNORE_ERROR(r);
         }
 #endif
+#if (HAVE_CHOWN)
+        // copy the group ownership
+        if (opt->preserve_ownership) {
+            r = chown(name, -1, st.st_gid);
+            IGNORE_ERROR(r);
+        }
+#endif
 #if (HAVE_CHMOD)
         // copy permissions
         if (opt->preserve_mode) {
@@ -230,9 +235,9 @@ void do_one_file(const char *iname, char *oname) {
         }
 #endif
 #if (HAVE_CHOWN)
-        // copy the ownership
+        // copy the user ownership
         if (opt->preserve_ownership) {
-            r = chown(name, st.st_uid, st.st_gid);
+            r = chown(name, st.st_uid, -1);
             IGNORE_ERROR(r);
         }
 #endif