src: make sort order deterministic, next try

We cannot compare pointers as they may point to qsort-local objects.
And we must make sure that cmp(a,b) always agrees with cmp(b,a).

src/linker.cpp

@@ -329,6 +329,7 @@ ElfLinker::Section *ElfLinker::addSection(const char *sname, const void *sdata,
     assert(sname[strlen(sname) - 1] != ':');
     assert(findSection(sname, false) == nullptr);
     Section *sec = new Section(sname, sdata, slen, p2align);
+    sec->sort_id = nsections;
     sections[nsections++] = sec;
     return sec;
 }

src/linker.h

@@ -119,6 +119,7 @@ struct ElfLinker::Section : private noncopyable {
     void *input = nullptr;
     byte *output = nullptr;
     unsigned size = 0;
+    unsigned sort_id = 0; // for qsort()
     upx_uint64_t offset = 0;
     unsigned p2align = 0; // log2
     Section *next = nullptr;

src/p_mach.cpp

@@ -562,7 +562,8 @@ PackMachBase<T>::compare_segment_command(void const *const aa, void const *const
     unsigned const xb = b->cmd - lc_seg;
            if (xa < xb)        return -1;  // LC_SEGMENT first
            if (xa > xb)        return  1;
-           if (0 != xa)        return  0;  // not LC_SEGMENT
+// mfx @jreiser: we do not want to return 0; please check
+////       if (0 != xa)        return  0;  // not LC_SEGMENT
     // Ascending by .fileoff so that find_SEGMENT_gap works
     if (a->fileoff < b->fileoff)
                                return -1;
@@ -572,10 +573,24 @@ PackMachBase<T>::compare_segment_command(void const *const aa, void const *const
     if (a->vmaddr < b->vmaddr) return -1;
     if (a->vmaddr > b->vmaddr) return  1;
     // Descending by .vmsize
+// mfx @jreiser: I think this may violate cmp(a,b) == -cmp(b,a); please check
+//    if ((a->vmsize != 0) != (b->vmsize != 0))
+//        return (a->vmsize != 0) ? -1 : 1;
     if (a->vmsize)             return -1;  // 'a' is first
     if (b->vmsize)             return  1;  // 'a' is last
     // What could remain?
-    return aa < bb ? -1 : 1; // make sort order deterministic/stable
+#define CMP(field) \
+    if (a->field != b->field) return a->field < b->field ? -1 : 1
+    // try to make sort order deterministic and just compare more fields
+    CMP(vmsize);
+    CMP(cmdsize);
+    CMP(filesize);
+    CMP(maxprot);
+    CMP(initprot);
+    CMP(nsects);
+    CMP(flags);
+#undef CMP
+    return 0;
 }
 
 // At 2013-02-03 part of the source for codesign was

src/p_vmlinx.cpp

@@ -99,7 +99,17 @@ PackVmlinuxBase<T>::compare_Phdr(void const *aa, void const *bb)
     if (a->p_paddr < b->p_paddr) return -1;  // ascending by .p_paddr
     if (a->p_paddr > b->p_paddr) return  1;
     // What could remain?
-    return aa < bb ? -1 : 1; // make sort order deterministic/stable
+    // try to make sort order deterministic and just compare more fields
+#define CMP(field) \
+    if (a->field != b->field) return a->field < b->field ? -1 : 1
+    CMP(p_offset);
+    CMP(p_vaddr);
+    CMP(p_filesz);
+    CMP(p_memsz);
+    CMP(p_flags);
+    CMP(p_align);
+#undef CMP
+    return 0;
 }
 
 template <class T>

src/pefile.cpp

@@ -697,14 +697,15 @@ class PeFile::ImportLinker final : public ElfLinkerAMD64 {
             infoWarning("empty import: %s", dll);
     }
 
-    static int __acc_cdecl_qsort compare(const void *p1, const void *p2) {
+    static int __acc_cdecl_qsort compare(const void *aa, const void *bb) {
-        const Section *s1 = *(const Section *const *) p1;
+        const Section *a = *(const Section *const *) aa;
-        const Section *s2 = *(const Section *const *) p2;
+        const Section *b = *(const Section *const *) bb;
-        int rc = strcmp(s1->name, s2->name);
+        int rc = strcmp(a->name, b->name);
         if (rc != 0)
             return rc;
         // What could remain?
-        return p1 < p2 ? -1 : 1; // make sort order deterministic/stable
+        assert_noexcept(a->sort_id != b->sort_id);
+        return a->sort_id < b->sort_id ? -1 : 1; // make sort order deterministic
     }
 
     virtual void alignCode(unsigned len) override { alignWithByte(len, 0); }
@@ -873,29 +874,31 @@ unsigned PeFile::processImports0(ord_mask_t ord_mask) // pass 1
         unsigned original_position;
         bool isk32;
 
-        static int __acc_cdecl_qsort compare(const void *p1, const void *p2) {
+        static int __acc_cdecl_qsort compare(const void *aa, const void *bb) {
-            const udll *u1 = *(const udll *const *) p1;
+            const udll *a = *(const udll *const *) aa;
-            const udll *u2 = *(const udll *const *) p2;
+            const udll *b = *(const udll *const *) bb;
-            if (u1->isk32 != u2->isk32)
+            if (a->isk32 != b->isk32)
-                return u1->isk32 ? -1 : 1;
+                return a->isk32 ? -1 : 1;
-            if ((*u1->lookupt != 0) != (*u2->lookupt != 0))
+            if ((*a->lookupt != 0) != (*b->lookupt != 0))
-                return (*u1->lookupt != 0) ? -1 : 1;
+                return (*a->lookupt != 0) ? -1 : 1;
-            int rc = strcasecmp(u1->name, u2->name);
+            int rc = strcasecmp(a->name, b->name);
             if (rc != 0)
                 return rc;
-            if ((u1->ordinal != 0) != (u2->ordinal != 0))
+            if ((a->ordinal != 0) != (b->ordinal != 0))
-                return (u1->ordinal != 0) ? -1 : 1;
+                return (a->ordinal != 0) ? -1 : 1;
-            if (u1->shname && u2->shname) {
+            if (a->shname && b->shname) {
-                rc = (int) (upx_safe_strlen(u1->shname) - upx_safe_strlen(u2->shname));
+                rc = (int) (upx_safe_strlen(a->shname) - upx_safe_strlen(b->shname));
                 if (rc != 0)
                     return rc;
-                rc = strcmp(u1->shname, u2->shname);
+                rc = strcmp(a->shname, b->shname);
                 if (rc != 0)
                     return rc;
-            } else if ((u1->shname != nullptr) != (u2->shname != nullptr))
+            } else if ((a->shname != nullptr) != (b->shname != nullptr))
-                return (u1->shname != nullptr) ? -1 : 1;
+                return (a->shname != nullptr) ? -1 : 1;
             // What could remain?
-            return p1 < p2 ? -1 : 1; // make sort order deterministic/stable
+            // make sort order deterministic
+            assert_noexcept(a->original_position != b->original_position);
+            return a->original_position < b->original_position ? -1 : 1;
         }
     };