]> git.ipfire.org Git - thirdparty/zlib-ng.git/commitdiff
Rebuild stale hash chains on deflateParams change
authorNathan Moin Vaziri <nathan@nathanm.com>
Mon, 22 Jun 2026 21:21:23 +0000 (14:21 -0700)
committerHans Kristian Rosbach <hk-github@circlestorm.org>
Wed, 15 Jul 2026 10:56:06 +0000 (12:56 +0200)
deflateParams can leave the chains stale for the new strategy several ways:
the previous strategy didn't maintain them (stored, huffman, or rle), or
crossing the level-9 boundary switched the hash function from integer to
rolling. In each case the new strategy silently dropped match history until
the window refilled.

Replace the stored-only slide/clear fixup with a single staleness check that
rebuilds the chains from the current window with the new hash whenever the new
strategy reads it. The rebuild reconstructs the chains unconditionally, so
deflate_stored no longer needs to defer and count hash slides; drop that
counter.

deflate.c
deflate.h
deflate_p.h
deflate_stored.c
test/test_deflate_params.cc
trees.c

index 164d9b9b36efacfbf9e915f4732adac362a23710..cdc992a3137484c348764b131b2f21c707cecaa7 100644 (file)
--- a/deflate.c
+++ b/deflate.c
@@ -619,18 +619,23 @@ int32_t Z_EXPORT PREFIX(deflateParams)(PREFIX3(stream) *strm, int32_t level, int
         if (strm->avail_in || ((int)s->strstart - s->block_start) + s->lookahead || !DEFLATE_DONE(strm, flush))
             return Z_BUF_ERROR;
     }
-    if (s->level != level) {
-        if (s->level == 0 && s->matches != 0) {
-            if (s->matches == 1) {
-                FUNCTABLE_CALL(slide_hash)(s);
-            } else {
-                CLEAR_HASH(s);
-            }
-            s->matches = 0;
-        }
 
-        lm_set_level(s, level);
+    int hashless = level == 0 || strategy == Z_HUFFMAN_ONLY || strategy == Z_RLE;
+    int was_hashless = s->level == 0 || s->strategy == Z_HUFFMAN_ONLY || s->strategy == Z_RLE;
+
+    /* Stale if the hash usage flipped (to/from huffman/rle/stored) or the hash
+     * function changed at level 9. */
+    int stale_chain = (hashless != was_hashless) || (level >= 9) != (s->level >= 9);
+
+    /* Rebuild the hash chains when fill_window is called. */
+    if (stale_chain && !hashless) {
+        CLEAR_HASH(s);
+        s->ins_h = 0;
+        s->insert = MIN(s->strstart, s->w_size);
     }
+
+    if (s->level != level)
+        lm_set_level(s, level);
     s->strategy = strategy;
     return Z_OK;
 }
index 832514c9125eed18aae0409df78eb58770007f92..a68249cd966c8ed1d61d3e0bd3960088a41bdf92 100644 (file)
--- a/deflate.h
+++ b/deflate.h
@@ -227,7 +227,7 @@ struct ALIGNED_(64) internal_state {
     int strategy;               /* favor or force Huffman coding*/
     unsigned int good_match;    /* Use a faster search when the previous match is longer than this */
     int nice_match;             /* Stop searching when current match exceeds this */
-    unsigned int matches;       /* number of string matches in current block */
+    int32_t padding1;           /* padding */
     unsigned int insert;        /* bytes at end of window left to insert */
 
     uint64_t bi_buf;            /* Output buffer.
index 6085e62c9b667c96975a082e58cd9221bd271bcd..18c8b6780158294f0bfb2f4a9cec3b6b8060ca81 100644 (file)
@@ -104,7 +104,6 @@ static inline int zng_tr_tally_dist(deflate_state* s, uint32_t dist, uint32_t le
 #  endif
     s->sym_next = sym_next + 3;
 #endif
-    s->matches++;
     dist--;
     Assert(dist < MAX_DIST(s) && (uint16_t)d_code(dist) < (uint16_t)D_CODES,
         "zng_tr_tally: bad match");
index 9c9e3fecb86be7a614763e5e3430f2fc1016249f..253a67e94bd9033583a9448b30be826fe21c07c2 100644 (file)
  * Copy without compression as much as possible from the input stream, return
  * the current block state.
  *
- * In case deflateParams() is used to later switch to a non-zero compression
- * level, s->matches (otherwise unused when storing) keeps track of the number
- * of hash table slides to perform. If s->matches is 1, then one hash table
- * slide will be done when switching. If s->matches is 2, the maximum value
- * allowed here, then the hash table will be cleared, since two or more slides
- * is the same as a clear.
- *
  * deflate_stored() is written to minimize the number of times an input byte is
  * copied. It is most efficient with large input and output buffers, which
  * maximizes the opportunities to have a single copy from next_in to next_out.
@@ -115,7 +108,6 @@ Z_INTERNAL block_state deflate_stored(deflate_state *s, int flush) {
          * therefore s->block_start == s->strstart.
          */
         if (used >= w_size) {    /* supplant the previous history */
-            s->matches = 2;         /* clear hash */
             memcpy(window, s->strm->next_in - w_size, w_size);
             s->strstart = w_size;
             s->insert = s->strstart;
@@ -124,8 +116,6 @@ Z_INTERNAL block_state deflate_stored(deflate_state *s, int flush) {
                 /* Slide the window down. */
                 s->strstart -= w_size;
                 memcpy(window, window + w_size, s->strstart);
-                if (s->matches < 2)
-                    s->matches++;   /* add a pending slide_hash() */
                 s->insert = MIN(s->insert, s->strstart);
             }
             memcpy(window + s->strstart, s->strm->next_in - used, used);
@@ -151,8 +141,6 @@ Z_INTERNAL block_state deflate_stored(deflate_state *s, int flush) {
         s->block_start -= (int)w_size;
         s->strstart -= w_size;
         memcpy(window, window + w_size, s->strstart);
-        if (s->matches < 2)
-            s->matches++;           /* add a pending slide_hash() */
         have += w_size;          /* more space now */
         s->insert = MIN(s->insert, s->strstart);
     }
index 9fadea85f9733848393a571b9cfa9703033eaa36..9afb7813dd273071d17b5ee4490c4aecbec48192 100644 (file)
@@ -12,7 +12,6 @@
 #include <stdlib.h>
 #include <string.h>
 #include <inttypes.h>
-#include <time.h>
 
 #include "deflate.h"
 
 #define UNCOMPR_BUFFER_SIZE (64 * 1024)
 #define UNCOMPR_RAND_SIZE (8 * 1024)
 
+/* Fill a buffer with deterministic, effectively incompressible bytes. */
+static void fill_random(uint8_t *buf, uint32_t len) {
+    uint32_t r = 0xa5a5a5a5u;
+    for (uint32_t i = 0; i < len; i++) {
+        r = r * 1664525u + 1013904223u;
+        buf[i] = (uint8_t)(r >> 24);
+    }
+}
+
 TEST(deflate, params) {
     PREFIX3(stream) c_stream, d_stream;
     uint8_t *compr, *uncompr;
     uint32_t compr_len, uncompr_len;
     uint32_t diff;
-    int32_t i;
-    time_t now;
     int err;
 #ifndef ZLIB_COMPAT
     int level = -1;
@@ -57,9 +63,7 @@ TEST(deflate, params) {
     compr_len = COMPR_BUFFER_SIZE;
     uncompr_len = UNCOMPR_BUFFER_SIZE;
 
-    srand((unsigned)time(&now));
-    for (i = 0; i < UNCOMPR_RAND_SIZE; i++)
-        uncompr[i] = (uint8_t)(rand() % 256);
+    fill_random(uncompr, UNCOMPR_RAND_SIZE);
 
     err = PREFIX(deflateInit)(&c_stream, Z_BEST_SPEED);
     EXPECT_EQ(err, Z_OK);
@@ -141,3 +145,77 @@ TEST(deflate, params) {
     free(compr);
     free(uncompr);
 }
+
+/* deflateParams must preserve match history when the params change mid-stream.
+ * The second half of the input is an exact copy of the first and sits within
+ * the 32K window, so preserved history turns it into cross-boundary matches.
+ * If the chains were dropped at the switch the copy is re-emitted and the
+ * stream nearly doubles. */
+static void deflate_params_history(int from_level, int from_strategy, int to_level, int to_strategy) {
+    const uint32_t half = 16 * 1024;
+    const uint32_t len = 2 * half;
+
+    uint8_t *uncompr = (uint8_t *)calloc(1, len);
+    uint8_t *compr = (uint8_t *)calloc(1, 2 * len);
+    uint8_t *check = (uint8_t *)calloc(1, len);
+    ASSERT_TRUE(uncompr != NULL && compr != NULL && check != NULL);
+
+    fill_random(uncompr, half);
+    memcpy(uncompr + half, uncompr, half);
+
+    PREFIX3(stream) c_stream;
+    memset(&c_stream, 0, sizeof(c_stream));
+    EXPECT_EQ(PREFIX(deflateInit2)(&c_stream, from_level, Z_DEFLATED, 15, 8, from_strategy), Z_OK);
+
+    c_stream.next_out = compr;
+    c_stream.avail_out = 2 * len;
+    c_stream.next_in = uncompr;
+    c_stream.avail_in = half;
+    EXPECT_EQ(PREFIX(deflate)(&c_stream, Z_NO_FLUSH), Z_OK);
+    EXPECT_EQ(c_stream.avail_in, 0u);
+
+    EXPECT_EQ(PREFIX(deflateParams)(&c_stream, to_level, to_strategy), Z_OK);
+
+    c_stream.next_in = uncompr + half;
+    c_stream.avail_in = half;
+    EXPECT_EQ(PREFIX(deflate)(&c_stream, Z_FINISH), Z_STREAM_END);
+
+    uint32_t compr_len = (uint32_t)(c_stream.next_out - compr);
+    EXPECT_EQ(PREFIX(deflateEnd)(&c_stream), Z_OK);
+
+    PREFIX3(stream) d_stream;
+    memset(&d_stream, 0, sizeof(d_stream));
+    EXPECT_EQ(PREFIX(inflateInit)(&d_stream), Z_OK);
+    d_stream.next_in = compr;
+    d_stream.avail_in = compr_len;
+    d_stream.next_out = check;
+    d_stream.avail_out = len;
+    EXPECT_EQ(PREFIX(inflate)(&d_stream, Z_FINISH), Z_STREAM_END);
+    EXPECT_EQ(d_stream.total_out, len);
+    EXPECT_EQ(memcmp(uncompr, check, len), 0);
+    EXPECT_EQ(PREFIX(inflateEnd)(&d_stream), Z_OK);
+
+    EXPECT_LT(compr_len, half + half / 2);
+
+    free(uncompr);
+    free(compr);
+    free(check);
+}
+
+TEST(deflate, params_history_level9_down) {
+    deflate_params_history(9, Z_DEFAULT_STRATEGY, 6, Z_DEFAULT_STRATEGY);
+}
+
+TEST(deflate, params_history_level9_up) {
+    deflate_params_history(6, Z_DEFAULT_STRATEGY, 9, Z_DEFAULT_STRATEGY);
+}
+
+/* Switching from a hashless strategy (Z_RLE/Z_HUFFMAN_ONLY) back to a hashing one
+ * at the same level must also rebuild, since those strategies never insert. */
+TEST(deflate, params_history_rle) {
+    deflate_params_history(6, Z_RLE, 6, Z_DEFAULT_STRATEGY);
+}
+
+TEST(deflate, params_history_huffman) {
+    deflate_params_history(6, Z_HUFFMAN_ONLY, 6, Z_DEFAULT_STRATEGY);
+}
diff --git a/trees.c b/trees.c
index a12ed385c2cc8b3c87aba0c8a8ae9522ccde3d85..b9ba590b724e577b5854be5fad0be32837d177da 100644 (file)
--- a/trees.c
+++ b/trees.c
@@ -117,7 +117,7 @@ static void init_block(deflate_state *s) {
 
     s->dyn_ltree[END_BLOCK].Freq = 1;
     s->opt_len = s->static_len = 0;
-    s->sym_next = s->matches = 0;
+    s->sym_next = 0;
 }
 
 #define SMALLEST 1