dictBuilder: break COVER sort ties by position, not by address

lib/dictBuilder/cover.c

@@ -310,7 +310,7 @@ static int COVER_cmp8(COVER_ctx_t *ctx, const void *lp, const void *rp) {
 }
 
 /**
- * Same as COVER_cmp() except ties are broken by pointer value
+ * Same as COVER_cmp() except ties are broken by the position each element holds
  */
 #if (ZDICT_QSORT == ZDICT_QSORT_MSVC) || (ZDICT_QSORT == ZDICT_QSORT_APPLE)
 static int WIN_CDECL COVER_strict_cmp(void* g_coverCtx, const void* lp, const void* rp) {
@@ -321,7 +321,9 @@ static int COVER_strict_cmp(const void *lp, const void *rp) {
 #endif
   int result = COVER_cmp((COVER_ctx_t*)g_coverCtx, lp, rp);
   if (result == 0) {
-    result = lp < rp ? -1 : 1;
+    U32 const lpos = *(const U32 *)lp;
+    U32 const rpos = *(const U32 *)rp;
+    result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0;
   }
   return result;
 }
@@ -337,7 +339,9 @@ static int COVER_strict_cmp8(const void *lp, const void *rp) {
 #endif
   int result = COVER_cmp8((COVER_ctx_t*)g_coverCtx, lp, rp);
   if (result == 0) {
-    result = lp < rp ? -1 : 1;
+    U32 const lpos = *(const U32 *)lp;
+    U32 const rpos = *(const U32 *)rp;
+    result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0;
   }
   return result;
 }
@@ -692,7 +696,7 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer,
   {
     /* suffix is a partial suffix array.
      * It only sorts suffixes by their first parameters.d bytes.
-     * The sort is stable, so each dmer group is sorted by position in input.
+     * Ties are broken by position, so each group is sorted by input position.
      */
     U32 i;
     for (i = 0; i < ctx->suffixSize; ++i) {

tests/fuzzer.c

@@ -3120,6 +3120,45 @@ static int basicUnitTests(U32 const seed, double compressibility)
         }
         DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize);
 
+        DISPLAYLEVEL(3, "test%3i : COVER dictBuilder orders equal dmers by position : ", testNb++);
+        /* COVER sorts its suffix array with qsort(), breaking ties between
+         * equal dmers by sample position; COVER_group() relies on that order
+         * to count how many samples contain a dmer. A tie-break that depends on
+         * the sort's internal state instead (the former address comparison)
+         * still produces a dictionary, just a different one. Low-entropy
+         * samples make ties frequent, so train on them and compare against the
+         * dictionary the position order produces, for both comparator paths. */
+        {   size_t const dictCapacity = 16 KB;
+            void* const coverDict = malloc(dictCapacity);
+            unsigned const dValues[2] = { 8, 16 };  /* COVER_cmp8() and COVER_cmp() */
+            XXH64_hash_t const expectedHash[2] = { 0x77EB0C1D595FF7A3ULL, 0xE02EC0FD3A5BBF6DULL };
+            unsigned dIdx;
+            if (coverDict==NULL) goto _output_error;
+            RDG_genBuffer(decodedBuffer, totalSampleSize, 0.5, 0.05, 1);  /* low-entropy: many repeated dmers */
+            { U32 u; for (u=0; u<nbSamples; u++) samplesSizes[u] = sampleUnitSize; }
+            for (dIdx = 0; dIdx < 2; dIdx++) {
+                ZDICT_cover_params_t coverParams;
+                size_t coverDictSize;
+                XXH64_hash_t hash;
+                memset(&coverParams, 0, sizeof(coverParams));
+                coverParams.k = 256;
+                coverParams.d = dValues[dIdx];
+                coverDictSize = ZDICT_trainFromBuffer_cover(coverDict, dictCapacity,
+                                                            decodedBuffer, samplesSizes, nbSamples, coverParams);
+                if (ZDICT_isError(coverDictSize)) goto _output_error;
+                hash = XXH64(coverDict, coverDictSize, 0);
+                if (hash != expectedHash[dIdx]) {
+                    DISPLAYLEVEL(1, "error : COVER dictionary for d=%u hashes to %08X%08X, expected %08X%08X \n",
+                                    coverParams.d,
+                                    (unsigned)(hash >> 32), (unsigned)hash,
+                                    (unsigned)(expectedHash[dIdx] >> 32), (unsigned)expectedHash[dIdx]);
+                    goto _output_error;
+                }
+            }
+            free(coverDict);
+        }
+        DISPLAYLEVEL(3, "OK \n");
+
         DISPLAYLEVEL(3, "test%3i : check dictID : ", testNb++);
         dictID = ZDICT_getDictID(dictBuffer, dictSize);
         if (dictID==0) goto _output_error;