src: retract libc qsort() requirements

.github/workflows/ci.yml

@@ -12,8 +12,8 @@ env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
   UPX_CMAKE_BUILD_FLAGS: --verbose
-  # 2023-08-31
+  # 2023-09-01
-  ZIG_DIST_VERSION: 0.12.0-dev.244+f4c9e19bc
+  ZIG_DIST_VERSION: 0.12.0-dev.252+5dc2db805
 
 jobs:
   job-rebuild-and-verify-stubs:

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

@@ -10,8 +10,8 @@ on:
 env:
   CMAKE_REQUIRED_QUIET: OFF
   DEBIAN_FRONTEND: noninteractive
-  # 2023-08-31
+  # 2023-09-01
-  ZIG_DIST_VERSION: 0.12.0-dev.244+f4c9e19bc
+  ZIG_DIST_VERSION: 0.12.0-dev.252+5dc2db805
 
 jobs:
   job-linux-zigcc: # uses cmake + make

src/check/dt_check.cpp

@@ -626,8 +626,11 @@ TEST_CASE("libc snprintf") {
     CHECK_EQ(strcmp(buf, "-7.0.0.0.0.0.0.0.7.0xffffffffffffffff"), 0);
 }
 
+#if 0
 TEST_CASE("libc qsort") {
     // runtime check that libc qsort() never compares identical objects
+    // UPDATE: while only poor implementations of qsort() would actually do
+    //   this, it is probably allowed by the standard; so skip this test case
     struct Elem {
         upx_uint16_t id;
         upx_uint16_t value;
@@ -637,25 +640,29 @@ TEST_CASE("libc qsort") {
             assert_noexcept(a->id != b->id); // check not IDENTICAL
             return a->value < b->value ? -1 : (a->value == b->value ? 0 : 1);
         }
-        static bool qsort_check(Elem *e, size_t n) {
+        static bool check_sort(upx_sort_func_t sort, Elem *e, size_t n) {
-            upx_uint32_t x = n + 5381;
+            upx_uint32_t x = 5381 + n + ((upx_uintptr_t) e & 0xff);
             for (size_t i = 0; i < n; i++) {
                 e[i].id = (upx_uint16_t) i;
-                x = x * 33 + (i & 255);
+                x = x * 33 + 1 + (i & 255);
                 e[i].value = (upx_uint16_t) x;
             }
-            qsort(e, n, sizeof(*e), Elem::compare);
+            sort(e, n, sizeof(*e), Elem::compare);
-            bool sorted_ok = true;
             for (size_t i = 1; i < n; i++)
-                sorted_ok &= e[i - 1].value <= e[i].value;
+                if very_unlikely (e[i - 1].value > e[i].value)
-            return sorted_ok;
+                    return false;
+            return true;
         }
     };
 
     constexpr size_t N = 4096;
     Elem e[N];
-    for (size_t n = 1; n <= N; n <<= 1)
+    for (size_t n = 0; n <= N; n = 2 * n + 1) {
-        CHECK(Elem::qsort_check(e, n));
+        CHECK(Elem::check_sort(qsort, e, n));
+        // CHECK(Elem::check_sort(upx_shellsort, e, n));
+        // CHECK(Elem::check_sort(upx_stable_sort, e, n));
+    }
 }
+#endif
 
 /* vim:set ts=4 sw=4 et: */

src/pefile.cpp

@@ -700,12 +700,13 @@ class PeFile::ImportLinker final : public ElfLinkerAMD64 {
     static int __acc_cdecl_qsort compare(const void *aa, const void *bb) {
         const Section *a = *(const Section *const *) aa;
         const Section *b = *(const Section *const *) bb;
+        if (a->sort_id == b->sort_id) // identical object, poor qsort() implementation
+            return 0;
         int rc = strcmp(a->name, b->name);
         if (rc != 0)
             return rc;
         // What could remain?
         // make sort order deterministic
-        assert_noexcept(a->sort_id != b->sort_id);
         return a->sort_id < b->sort_id ? -1 : 1;
     }
 
@@ -878,6 +879,8 @@ unsigned PeFile::processImports0(ord_mask_t ord_mask) // pass 1
         static int __acc_cdecl_qsort compare(const void *aa, const void *bb) {
             const udll *a = *(const udll *const *) aa;
             const udll *b = *(const udll *const *) bb;
+            if (a->original_position == b->original_position) // identical object, poor qsort()
+                return 0;
             if (a->isk32 != b->isk32)
                 return a->isk32 ? -1 : 1;
             if ((*a->lookupt != 0) != (*b->lookupt != 0))
@@ -898,7 +901,6 @@ unsigned PeFile::processImports0(ord_mask_t ord_mask) // pass 1
                 return (a->shname != nullptr) ? -1 : 1;
             // What could remain?
             // make sort order deterministic
-            assert_noexcept(a->original_position != b->original_position);
             return a->original_position < b->original_position ? -1 : 1;
         }
     };

src/util/util.cpp

@@ -280,7 +280,7 @@ void upx_memswap(void *a, void *b, size_t n) {
 // somewhat better memswap(), optimized for our use cases in sort functions
 static void memswap_no_overlap(char *a, char *b, size_t n) {
 #if defined(__clang__) && __clang_major__ < 15 && 1
-    // avoid a clang ICE; sigh
+    // work around a clang ICE (Internal Compiler Error); sigh
     upx_memswap(a, b, n);
 #else // clang bug
     alignas(16) char tmpbuf[16];
@@ -301,35 +301,34 @@ static void memswap_no_overlap(char *a, char *b, size_t n) {
         SWAP(4);
     if (n & 2)
         SWAP(2);
-    if (n & 1)
+    if (n & 1) {
-        SWAP(1);
+        char tmp = *a;
-    UNUSED(a); // avoid pedantic warning about final assignment
+        *a = *b;
-    UNUSED(b); // avoid pedantic warning about final assignment
+        *b = tmp;
+    }
 #undef SWAP
 #endif // clang bug
 }
 
 // simple Shell sort using Knuth's gap; NOT stable
-void upx_shellsort(void *array, size_t n, size_t element_size,
+void upx_shellsort(void *array, size_t n, size_t element_size, upx_compare_func_t compare) {
-                   int (*compare)(const void *, const void *)) {
     mem_size_assert(element_size, n); // check size
-    size_t gap = 1;
+    size_t gap = 0;
-    while (gap * 3 <= n) // cannot overflow
+    while (gap * 3 + 1 < n) // cannot overflow
         gap = gap * 3 + 1;
     for (; gap > 0; gap = (gap - 1) / 3) {
         const size_t gap_bytes = element_size * gap;
-        char *g = (char *) array + gap_bytes; // g := &array[gap]
+        char *const gbase = (char *) array + gap_bytes; // gbase := &array[gap]
-        char *ii = g;
+        char *ii = gbase;
         for (size_t i = gap; i < n; i += gap, ii += gap_bytes)
-            for (char *a = ii; a >= g && compare(a - gap_bytes, a) > 0; a -= gap_bytes)
+            for (char *a = ii; a >= gbase && compare(a - gap_bytes, a) > 0; a -= gap_bytes)
                 memswap_no_overlap(a - gap_bytes, a, element_size);
     }
 }
 
 // extremely simple (and beautiful) stable sort: Gnomesort
 // WARNING: O(n^2) and thus very inefficient for large n
-void upx_stable_sort(void *array, size_t n, size_t element_size,
+void upx_stable_sort(void *array, size_t n, size_t element_size, upx_compare_func_t compare) {
-                     int (*compare)(const void *, const void *)) {
     for (size_t i = 1; i < n; i++) {
         char *a = (char *) array + element_size * i;               // a := &array[i]
         if (i != 0 && compare(a - element_size, a) > 0) {          // if a[-1] > a[0] then
@@ -363,11 +362,9 @@ TEST_CASE("basic upx_stable_sort") {
 
 #if __cplusplus >= 202002L // use C++20 std::next_permutation() to test all permutations
 namespace {
-typedef int (*compare_func)(const void *, const void *);
+template <class ElementType, upx_compare_func_t CompareFunc>
-typedef void (*sort_func)(void *array, size_t n, size_t element_size, compare_func compare);
-template <class ElementType, compare_func CompareFunc>
 struct TestSortAllPermutations {
-    static noinline upx_uint64_t test(sort_func sort, size_t n) {
+    static noinline upx_uint64_t test(upx_sort_func_t sort, size_t n) {
         constexpr size_t N = 16;
         assert(n > 0 && n <= N);
         ElementType perm[N];

src/util/util.h

@@ -127,11 +127,12 @@ void *upx_calloc(size_t n, size_t element_size);
 
 void upx_memswap(void *a, void *b, size_t n);
 
-void upx_shellsort(void *array, size_t n, size_t element_size,
+typedef int(__acc_cdecl_qsort *upx_compare_func_t)(const void *, const void *);
-                   int (*compare)(const void *, const void *));
+typedef void (*upx_sort_func_t)(void *array, size_t n, size_t element_size, upx_compare_func_t);
 
-void upx_stable_sort(void *array, size_t n, size_t element_size,
+void upx_shellsort(void *array, size_t n, size_t element_size, upx_compare_func_t compare);
-                     int (*compare)(const void *, const void *));
+
+void upx_stable_sort(void *array, size_t n, size_t element_size, upx_compare_func_t compare);
 
 /*************************************************************************
 // misc. support functions