From 35dd9cfd223e78c1da9dc29151ef039f8a52dfab Mon Sep 17 00:00:00 2001 From: "Markus F.X.J. Oberhumer" Date: Sat, 2 Sep 2023 16:15:57 +0200 Subject: [PATCH] src: retract libc qsort() requirements --- .github/workflows/ci.yml | 4 +-- .github/workflows/weekly-ci-cc-zigcc.yml | 4 +-- src/check/dt_check.cpp | 25 +++++++++++------- src/pefile.cpp | 6 +++-- src/util/util.cpp | 33 +++++++++++------------- src/util/util.h | 9 ++++--- 6 files changed, 44 insertions(+), 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f0d7f41c..df12bb38 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,8 +12,8 @@ env: CMAKE_REQUIRED_QUIET: OFF DEBIAN_FRONTEND: noninteractive UPX_CMAKE_BUILD_FLAGS: --verbose - # 2023-08-31 - ZIG_DIST_VERSION: 0.12.0-dev.244+f4c9e19bc + # 2023-09-01 + ZIG_DIST_VERSION: 0.12.0-dev.252+5dc2db805 jobs: job-rebuild-and-verify-stubs: diff --git a/.github/workflows/weekly-ci-cc-zigcc.yml b/.github/workflows/weekly-ci-cc-zigcc.yml index e2e2df71..9adea1f4 100644 --- a/.github/workflows/weekly-ci-cc-zigcc.yml +++ b/.github/workflows/weekly-ci-cc-zigcc.yml @@ -10,8 +10,8 @@ on: env: CMAKE_REQUIRED_QUIET: OFF DEBIAN_FRONTEND: noninteractive - # 2023-08-31 - ZIG_DIST_VERSION: 0.12.0-dev.244+f4c9e19bc + # 2023-09-01 + ZIG_DIST_VERSION: 0.12.0-dev.252+5dc2db805 jobs: job-linux-zigcc: # uses cmake + make diff --git a/src/check/dt_check.cpp b/src/check/dt_check.cpp index fc4486d8..876a93e7 100644 --- a/src/check/dt_check.cpp +++ b/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) { - upx_uint32_t x = n + 5381; + static bool check_sort(upx_sort_func_t sort, Elem *e, size_t n) { + 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); - bool sorted_ok = true; + sort(e, n, sizeof(*e), Elem::compare); for (size_t i = 1; i < n; i++) - sorted_ok &= e[i - 1].value <= e[i].value; - return sorted_ok; + if very_unlikely (e[i - 1].value > e[i].value) + return false; + return true; } }; constexpr size_t N = 4096; Elem e[N]; - for (size_t n = 1; n <= N; n <<= 1) - CHECK(Elem::qsort_check(e, n)); + for (size_t n = 0; n <= N; n = 2 * n + 1) { + 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: */ diff --git a/src/pefile.cpp b/src/pefile.cpp index 0dd745be..260afd64 100644 --- a/src/pefile.cpp +++ b/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; } }; diff --git a/src/util/util.cpp b/src/util/util.cpp index 060fbeb5..c33bee26 100644 --- a/src/util/util.cpp +++ b/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) - SWAP(1); - UNUSED(a); // avoid pedantic warning about final assignment - UNUSED(b); // avoid pedantic warning about final assignment + if (n & 1) { + char tmp = *a; + *a = *b; + *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, - int (*compare)(const void *, const void *)) { +void upx_shellsort(void *array, size_t n, size_t element_size, upx_compare_func_t compare) { mem_size_assert(element_size, n); // check size - size_t gap = 1; - while (gap * 3 <= n) // cannot overflow + size_t gap = 0; + 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 *ii = g; + char *const gbase = (char *) array + gap_bytes; // gbase := &array[gap] + 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, - int (*compare)(const void *, const void *)) { +void upx_stable_sort(void *array, size_t n, size_t element_size, upx_compare_func_t compare) { 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 *); -typedef void (*sort_func)(void *array, size_t n, size_t element_size, compare_func compare); -template +template 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]; diff --git a/src/util/util.h b/src/util/util.h index 91fb206f..f3eac5a2 100644 --- a/src/util/util.h +++ b/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, - int (*compare)(const void *, const void *)); +typedef int(__acc_cdecl_qsort *upx_compare_func_t)(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, - int (*compare)(const void *, const void *)); +void upx_shellsort(void *array, size_t n, size_t element_size, upx_compare_func_t compare); + +void upx_stable_sort(void *array, size_t n, size_t element_size, upx_compare_func_t compare); /************************************************************************* // misc. support functions