From: Nathan Moin Vaziri Date: Mon, 22 Jun 2026 21:21:23 +0000 (-0700) Subject: Rebuild stale hash chains on deflateParams change X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b0e408e72d7f40fbed321caad956c15331d898b8;p=thirdparty%2Fzlib-ng.git Rebuild stale hash chains on deflateParams change 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. --- diff --git a/deflate.c b/deflate.c index 164d9b9b3..cdc992a31 100644 --- 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; } diff --git a/deflate.h b/deflate.h index 832514c91..a68249cd9 100644 --- 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. diff --git a/deflate_p.h b/deflate_p.h index 6085e62c9..18c8b6780 100644 --- a/deflate_p.h +++ b/deflate_p.h @@ -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"); diff --git a/deflate_stored.c b/deflate_stored.c index 9c9e3fecb..253a67e94 100644 --- a/deflate_stored.c +++ b/deflate_stored.c @@ -13,13 +13,6 @@ * 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); } diff --git a/test/test_deflate_params.cc b/test/test_deflate_params.cc index 9fadea85f..9afb7813d 100644 --- a/test/test_deflate_params.cc +++ b/test/test_deflate_params.cc @@ -12,7 +12,6 @@ #include #include #include -#include #include "deflate.h" @@ -24,13 +23,20 @@ #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 a12ed385c..b9ba590b7 100644 --- 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