clang-tidy

.clang-tidy

@@ -19,6 +19,9 @@ Checks: >
     -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
     -clang-analyzer-security.insecureAPI.strcpy,
   clang-diagnostics-*',
+  performance-*,
+    -performance-unnecessary-value-param,
 FormatStyle: file
 HeaderFilterRegex: '.*'
+InheritParentConfig: false
 ...

.github/workflows/ci.yml

@@ -12,8 +12,8 @@ env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
   UPX_CMAKE_BUILD_FLAGS: --verbose
-  # 2023-07-26
+  # 2023-07-30
-  ZIG_DIST_VERSION: 0.11.0-dev.4246+71dfce31b
+  ZIG_DIST_VERSION: 0.11.0-dev.4320+6f0a613b6
 
 jobs:
   job-rebuild-and-verify-stubs:

.github/workflows/static-analyzer-clang-tidy.yml

@@ -24,7 +24,7 @@ jobs:
           git clone --branch "$GITHUB_REF_NAME" --depth 1 https://github.com/upx/upx
           git -C upx submodule update --init
       - name: 'Perform clang-tidy Analysis Debug'
-        run: 'make -C upx build/analyze/clang-tidy/debug'
+        run: 'make -k -C upx build/analyze/clang-tidy/debug'
       - name: 'Perform clang-tidy Analysis Release'
         if: success() || failure() # run this step even if the previous step failed
-        run: 'make -C upx build/analyze/clang-tidy/release'
+        run: 'make -k -C upx build/analyze/clang-tidy/release'

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

@@ -8,8 +8,8 @@ on:
 env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
-  # 2023-07-26
+  # 2023-07-30
-  ZIG_DIST_VERSION: 0.11.0-dev.4246+71dfce31b
+  ZIG_DIST_VERSION: 0.11.0-dev.4320+6f0a613b6
 
 jobs:
   job-linux-zigcc: # uses cmake + make

Makefile

@@ -160,7 +160,7 @@ build/extra/cross-darwin-x86_64/%: export CC  = clang -target x86_64-apple-darwi
 build/extra/cross-darwin-x86_64/%: export CXX = clang++ -target x86_64-apple-darwin
 
 #***********************************************************************
-# C++ static analyzers
+# C/C++ static analyzers
 #***********************************************************************
 
 # force building with clang Static Analyzer (scan-build)
@@ -171,10 +171,23 @@ build/analyze/clang-analyzer/%: export CCC_CC  ?= clang
 build/analyze/clang-analyzer/%: export CCC_CXX ?= clang++
 
 # run clang-tidy (uses file compile_commands.json from an existing clang build)
-build/analyze/clang-tidy/debug:   build/extra/clang/debug PHONY
+RUN_CLANG_TIDY = time python3 ./misc/analyze/clang-tidy/run-clang-tidy.py -p $<
-	python3 ./misc/scripts/run-clang-tidy.py -p $<
+RUN_CLANG_TIDY_WERROR = $(RUN_CLANG_TIDY) '-warnings-as-errors=*'
-build/analyze/clang-tidy/release: build/extra/clang/release PHONY
+build/analyze/clang-tidy-upx/debug build/analyze/clang-tidy-upx/release: build/extra/clang/$$(notdir $$@) PHONY
-	python3 ./misc/scripts/run-clang-tidy.py -p $<
+	$(RUN_CLANG_TIDY_WERROR) -config-file ./.clang-tidy '/src/.*\.cpp'
+build/analyze/clang-tidy-bzip2/debug build/analyze/clang-tidy-bzip2/release: build/extra/clang/$$(notdir $$@) PHONY
+	$(RUN_CLANG_TIDY)        -config-file ./misc/analyze/clang-tidy/clang-tidy-bzip2.yml /vendor/bzip2/
+build/analyze/clang-tidy-ucl/debug build/analyze/clang-tidy-ucl/release: build/extra/clang/$$(notdir $$@) PHONY
+	$(RUN_CLANG_TIDY_WERROR) -config-file ./misc/analyze/clang-tidy/clang-tidy-ucl.yml   /vendor/ucl/
+build/analyze/clang-tidy-zlib/debug build/analyze/clang-tidy-zlib/release: build/extra/clang/$$(notdir $$@) PHONY
+	$(RUN_CLANG_TIDY_WERROR) -config-file ./misc/analyze/clang-tidy/clang-tidy-zlib.yml  /vendor/zlib/
+build/analyze/clang-tidy-zstd/debug build/analyze/clang-tidy-zstd/release: build/extra/clang/$$(notdir $$@) PHONY
+	$(RUN_CLANG_TIDY)        -config-file ./misc/analyze/clang-tidy/clang-tidy-zstd.yml  /vendor/zstd/
+build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-upx/$$(notdir $$@)
+build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-bzip2/$$(notdir $$@)
+build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-ucl/$$(notdir $$@)
+build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-zlib/$$(notdir $$@)
+build/analyze/clang-tidy/debug build/analyze/clang-tidy/release: build/analyze/clang-tidy-zstd/$$(notdir $$@)
 
 # OLD names
 build/extra/scan-build/debug:   build/analyze/clang-analyzer/debug

misc/analyze/clang-tidy/clang-tidy-bzip2.yml

@@ -0,0 +1,21 @@
+# for clang-tidy-16 from https://clang.llvm.org/extra/clang-tidy/
+---
+Checks: >
+  -*,
+  bugprone-*,
+    -bugprone-branch-clone,
+    -bugprone-easily-swappable-parameters,
+    -bugprone-implicit-widening-of-multiplication-result,
+    -bugprone-macro-parentheses,
+    -bugprone-narrowing-conversions,
+    -bugprone-reserved-identifier,
+  clang-analyzer-*,
+    -clang-analyzer-core.UndefinedBinaryOperatorResult,
+    -clang-analyzer-core.uninitialized.Assign,
+    -clang-analyzer-security.insecureAPI.strcpy,
+  clang-diagnostics-*',
+  performance-*,
+FormatStyle: file
+HeaderFilterRegex: '.*'
+InheritParentConfig: false
+...

misc/analyze/clang-tidy/clang-tidy-ucl.yml

@@ -0,0 +1,20 @@
+# for clang-tidy-16 from https://clang.llvm.org/extra/clang-tidy/
+---
+Checks: >
+  -*,
+  bugprone-*,
+    -bugprone-assignment-in-if-condition,
+    -bugprone-easily-swappable-parameters,
+    -bugprone-implicit-widening-of-multiplication-result,
+    -bugprone-macro-parentheses,
+    -bugprone-reserved-identifier,
+    -bugprone-sizeof-expression,
+    -bugprone-suspicious-include,
+  clang-analyzer-*,
+    -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  clang-diagnostics-*',
+  performance-*,
+FormatStyle: file
+HeaderFilterRegex: '.*'
+InheritParentConfig: false
+...

misc/analyze/clang-tidy/clang-tidy-zlib.yml

@@ -0,0 +1,20 @@
+# for clang-tidy-16 from https://clang.llvm.org/extra/clang-tidy/
+---
+Checks: >
+  -*,
+  bugprone-*,
+    -bugprone-assignment-in-if-condition,
+    -bugprone-easily-swappable-parameters,
+    -bugprone-implicit-widening-of-multiplication-result,
+    -bugprone-macro-parentheses,
+    -bugprone-narrowing-conversions,
+    -bugprone-reserved-identifier,
+  clang-analyzer-*,
+    -clang-analyzer-optin.performance.Padding,
+    -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  clang-diagnostics-*',
+  performance-*,
+FormatStyle: file
+HeaderFilterRegex: '.*'
+InheritParentConfig: false
+...

misc/analyze/clang-tidy/clang-tidy-zstd.yml

@@ -0,0 +1,24 @@
+# for clang-tidy-16 from https://clang.llvm.org/extra/clang-tidy/
+---
+Checks: >
+  -*,
+  bugprone-*,
+    -bugprone-assignment-in-if-condition,
+    -bugprone-branch-clone,
+    -bugprone-easily-swappable-parameters,
+    -bugprone-implicit-widening-of-multiplication-result,
+    -bugprone-macro-parentheses,
+    -bugprone-misplaced-widening-cast,
+    -bugprone-narrowing-conversions,
+    -bugprone-reserved-identifier,
+  clang-analyzer-*,
+    -clang-analyzer-core.UndefinedBinaryOperatorResult,
+    -clang-analyzer-deadcode.DeadStores,
+    -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  clang-diagnostics-*',
+  performance-*,
+    -performance-no-int-to-ptr,
+FormatStyle: file
+HeaderFilterRegex: '.*'
+InheritParentConfig: false
+...

misc/analyze/clang-tidy/run-clang-tidy.py

@@ -391,6 +391,27 @@ def main():
         )
         tmpdir = tempfile.mkdtemp()
 
+    # Load the database and extract all files.
+    database = json.load(open(os.path.join(build_path, db_path)))
+    files = set(
+        [make_absolute(entry["file"], entry["directory"]) for entry in database]
+    )
+
+    # Build up a big regexy filter from all command line arguments.
+    file_name_re = re.compile("|".join(args.files))
+
+    # filter files with mo.group(0), and sort files
+    matched_files = []
+    for name in files:
+        mo = file_name_re.search(name)
+        if mo and mo.group(0):
+            matched_files.append(name)
+    files = sorted(matched_files)
+    if not files:
+        if tmpdir:
+            shutil.rmtree(tmpdir)
+        sys.exit(0)
+
     try:
         invocation = get_tidy_invocation(
             "",
@@ -422,20 +443,10 @@ def main():
         print("Unable to run clang-tidy.", file=sys.stderr)
         sys.exit(1)
 
-    # Load the database and extract all files.
-    database = json.load(open(os.path.join(build_path, db_path)))
-    files = set(
-        [make_absolute(entry["file"], entry["directory"]) for entry in database]
-    )
-    files = sorted(list(files))
-
     max_task = args.j
     if max_task == 0:
         max_task = multiprocessing.cpu_count()
 
-    # Build up a big regexy filter from all command line arguments.
-    file_name_re = re.compile("|".join(args.files))
-
     return_code = 0
     try:
         # Spin up a bunch of tidy-launching threads.

src/bele.h

@@ -29,11 +29,11 @@
 
 // BE - Big Endian
 // LE - Little Endian
-// NE - Native Endianness (aka host endianness)
+// NE - Native Endianness (aka Host Endianness aka CPU Endianness)
 // TE - Target Endianness (not used here, see various packers)
 
 #if 1
-// some platforms may provide their own system bswapXX() functions, so rename
+// some platforms may provide their own system bswapXX() functions, so rename to avoid conflicts
 #undef bswap16
 #undef bswap32
 #undef bswap64
@@ -112,7 +112,8 @@ static forceinline constexpr upx_uint64_t bswap64(upx_uint64_t v) noexcept {
 #endif
 
 static forceinline constexpr unsigned no_bswap16(unsigned v) noexcept {
-    return v & 0xffff; // needed so that this is equivalent to bswap16() above
+    // mask is needed so that for all v: bswap16(bswap16(v)) == no_bswap16(v)
+    return v & 0xffff;
 }
 static forceinline constexpr unsigned no_bswap32(unsigned v) noexcept { return v; }
 static forceinline constexpr upx_uint64_t no_bswap64(upx_uint64_t v) noexcept { return v; }

src/check/dt_check.cpp

@@ -28,6 +28,12 @@
 
 /*************************************************************************
 // upx_doctest_check()
+//
+// honors environment variables:
+//   UPX_DEBUG_DOCTEST_DISABLE
+//   UPX_DEBUG_DOCTEST_VERBOSE
+//
+// HINT: set "UPX_DEBUG_DOCTEST_DISABLE=1" for improved debugging experience
 **************************************************************************/
 
 int upx_doctest_check(int argc, char **argv) {
@@ -40,9 +46,10 @@ int upx_doctest_check(int argc, char **argv) {
     if (e && e[0] && strcmp(e, "0") != 0)
         return 0;
     bool minimal = true;   // don't show summary
-    bool duration = false; // show timings
+    bool duration = false; // don't show timings
-    bool success = false;  // show all tests
+    bool success = false;  // don't show all succeeding tests
 #if DEBUG
+    // default for debug builds: do show the [doctest] summary
     minimal = false;
 #endif
     e = getenv("UPX_DEBUG_DOCTEST_VERBOSE");
@@ -227,6 +234,7 @@ struct CheckIntegral {
         COMPILE_TIME_ASSERT(std::is_trivial<U>::value)
         // more checks, these are probably implied by std::is_trivial
         COMPILE_TIME_ASSERT(std::is_nothrow_default_constructible<U>::value)
+        COMPILE_TIME_ASSERT(std::is_nothrow_destructible<U>::value)
         COMPILE_TIME_ASSERT(std::is_trivially_copyable<U>::value)
         COMPILE_TIME_ASSERT(std::is_trivially_default_constructible<U>::value)
         // UPX extras
@@ -378,8 +386,14 @@ static noinline void check_basic_cxx_exception_handling(void (*func)(int)) noexc
 #include "../miniacc.h"
 
 void upx_compiler_sanity_check(void) noexcept {
-    // first check C++ exception handling to catch toolchain/qemu/wine/etc problems
+    const char *e = getenv("UPX_DEBUG_DOCTEST_DISABLE");
+    if (e && e[0] && strcmp(e, "0") != 0) {
+        // If UPX_DEBUG_DOCTEST_DISABLE is set then we don't want to throw any
+        // exceptions in order to improve debugging experience.
+    } else {
+        // check working C++ exception handling to catch toolchain/qemu/wine/etc problems
         check_basic_cxx_exception_handling(throwSomeValue);
+    }
 
 #define ACC_WANT_ACC_CHK_CH 1
 #undef ACCCHK_ASSERT
@@ -555,7 +569,7 @@ void upx_compiler_sanity_check(void) noexcept {
     assert_noexcept(TestIntegerWrap<int>::neg_eq(0));
     assert_noexcept(!TestIntegerWrap<int>::neg_eq(1));
     assert_noexcept(!TestIntegerWrap<int>::neg_eq(INT_MAX));
-    assert_noexcept(TestIntegerWrap<int>::neg_eq(INT_MIN)); // !!
+    assert_noexcept(TestIntegerWrap<int>::neg_eq(INT_MIN)); // special case
 }
 
 /*************************************************************************

src/check/dt_impl.cpp

@@ -52,9 +52,7 @@
 #pragma clang diagnostic ignored "-Wtautological-constant-compare"
 #endif
 
-// NOLINTBEGIN(bugprone-exception-escape)
 #include <doctest/doctest/parts/doctest.cpp>
-// NOLINTEND(bugprone-exception-escape)
 
 #endif // DOCTEST_CONFIG_DISABLE
 

src/check/dt_xspan.cpp

@@ -615,6 +615,8 @@ TEST_CASE("Span subspan") {
 TEST_CASE("Span constness") {
     static char buf[4] = {0, 1, 2, 3};
 
+    // NOLINTBEGIN(performance-unnecessary-copy-initialization)
+
     XSPAN_0(char) b0(buf, 4);
     XSPAN_P(char) bp(buf, 4);
     XSPAN_S(char) bs(buf, 4);
@@ -635,6 +637,8 @@ TEST_CASE("Span constness") {
     XSPAN_P(const char) xpc(bp);
     XSPAN_S(const char) xsc(bs);
 
+    // NOLINTEND(performance-unnecessary-copy-initialization)
+
     CHECK(ptr_diff_bytes(b0, buf) == 0);
     CHECK(ptr_diff_bytes(bp, buf) == 0);
     CHECK(ptr_diff_bytes(bs, buf) == 0);

src/p_mach.cpp

@@ -1663,8 +1663,8 @@ int PackMachBase<T>::canUnpack()
         unsigned const cmd = ptr->cmd;
         unsigned const cmdsize = ptr->cmdsize;
         if (is_bad_linker_command(cmd, cmdsize, headway, lc_seg, sizeof(Addr))) {
-                infoWarning("bad Mach_command[%u]{@0x%lx,+0x%x}: file_size=0x%lx  cmdsize=0x%lx",
+                infoWarning("bad Mach_command[%u]{@0x%zx,+0x%x}: file_size=0x%lx  cmdsize=0x%lx",
-                    j, (unsigned long) (sizeof(mhdri) + ((char const *)ptr - (char const *)rawmseg)), headway,
+                    j, (sizeof(mhdri) + ((char const *)ptr - (char const *)rawmseg)), headway,
                     (unsigned long) file_size, (unsigned long)ptr->cmdsize);
                 throwCantUnpack("file corrupted");
         }
@@ -1673,9 +1673,9 @@ int PackMachBase<T>::canUnpack()
             if ((unsigned long)file_size < segptr->filesize
             ||  (unsigned long)file_size < segptr->fileoff
             ||  (unsigned long)file_size < (segptr->filesize + segptr->fileoff)) {
-                infoWarning("bad Mach_segment_command[%u]{@0x%lx,+0x%x}: file_size=0x%lx  cmdsize=0x%lx"
+                infoWarning("bad Mach_segment_command[%u]{@0x%zx,+0x%x}: file_size=0x%lx  cmdsize=0x%lx"
                       "  filesize=0x%lx  fileoff=0x%lx",
-                    j, (unsigned long) (sizeof(mhdri) + ((char const *)ptr - (char const *)rawmseg)), headway,
+                    j, (sizeof(mhdri) + ((char const *)ptr - (char const *)rawmseg)), headway,
                     (unsigned long) file_size, (unsigned long)ptr->cmdsize,
                     (unsigned long)segptr->filesize, (unsigned long)segptr->fileoff);
                 throwCantUnpack("file corrupted");

src/packer_f.cpp

@@ -90,9 +90,9 @@ void Packer::addFilter32(int filter_id) {
                       "CALLTR13");
     }
     if (0x80 == (filter_id & 0xF0)) {
-        bool const x386 = (opt->cpu_x86 <= opt->CPU_386);
+        const bool x386 = (opt->cpu_x86 <= opt->CPU_386);
         const unsigned n_mru = ph.n_mru ? 1 + ph.n_mru : 0;
-        bool const mrupwr2 = (0 != n_mru) && 0 == ((n_mru - 1) & n_mru);
+        const bool mrupwr2 = (0 != n_mru) && 0 == ((n_mru - 1) & n_mru);
         const unsigned f_call = f80_call(filter_id);
         const unsigned f_jmp1 = f80_jmp1(filter_id);
         const unsigned f_jcc2 = f80_jcc2(filter_id);

src/util/membuffer.cpp

@@ -306,14 +306,10 @@ TEST_CASE("MemBuffer global overloads") {
     mb.clear();
     mb4.clear();
     CHECK(memcmp(mb, "\x00", 1) == 0);
-    // NOLINTNEXTLINE(bugprone-unused-return-value)
+    CHECK_THROWS(memcmp(mb, "\x00\x00", 2)); // NOLINT(bugprone-unused-return-value)
-    CHECK_THROWS(memcmp(mb, "\x00\x00", 2));
+    CHECK_THROWS(memcmp("\x00\x00", mb, 2)); // NOLINT(bugprone-unused-return-value)
-    // NOLINTNEXTLINE(bugprone-unused-return-value)
+    CHECK_THROWS(memcmp(mb, mb4, 2));        // NOLINT(bugprone-unused-return-value)
-    CHECK_THROWS(memcmp("\x00\x00", mb, 2));
+    CHECK_THROWS(memcmp(mb4, mb, 2));        // NOLINT(bugprone-unused-return-value)
-    // NOLINTNEXTLINE(bugprone-unused-return-value)
-    CHECK_THROWS(memcmp(mb, mb4, 2));
-    // NOLINTNEXTLINE(bugprone-unused-return-value)
-    CHECK_THROWS(memcmp(mb4, mb, 2));
     CHECK_NOTHROW(memset(mb, 255, 1));
     CHECK_THROWS(memset(mb, 254, 2));
     CHECK(mb[0] == 255);

src/util/util.h

@@ -141,7 +141,7 @@ void upx_stable_sort(void *array, size_t n, size_t element_size,
 // this works
 #define OwningPointer(T) T *
 
-#elif 1
+#elif !(DEBUG)
 
 // this also works
 template <class T>
@@ -150,7 +150,7 @@ using OwningPointer = T *;
 
 #else
 
-// also works: a simple class with just a number of no-ops
+// also works: a trivial class with just a number of no-ops
 template <class T>
 struct OwningPointer final {
     static_assert(std::is_class_v<T>); // UPX convention

src/util/xspan_impl_common.h

@@ -113,8 +113,8 @@ forceinline ~CSelf() noexcept {}
 #endif
     noinline void invalidate() {
         assertInvariants();
-        // poison the pointer
+        // poison the pointer: point to non-null invalid address
-        ptr = (pointer) (upx_uintptr_t) 16; // point to non-null invalid address
+        ptr = (pointer) (void *) (upx_uintptr_t) 16; // NOLINT(performance-no-int-to-ptr)
         // ptr = (pointer) (void *) &ptr; // point to self
         base = ptr;
         size_in_bytes = 0;

src/util/xspan_impl_ptr.h

@@ -77,8 +77,8 @@ public:
 #endif
     noinline void invalidate() {
         assertInvariants();
-        // poison the pointer
+        // poison the pointer: point to non-null invalid address
-        ptr = (pointer) (upx_uintptr_t) 16; // point to non-null invalid address
+        ptr = (pointer) (void *) (upx_uintptr_t) 16; // NOLINT(performance-no-int-to-ptr)
         // ptr = (pointer) (void *) &ptr; // point to self
         assertInvariants();
     }